From 35172e140236d5197a4d7d19f46d627dd6c119c1 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Wed, 17 Apr 2019 17:54:51 +0100 Subject: [PATCH] Update the types consumers of Index and Range (#24036) * Update the types consumers of Index and Range The C# compiler now can automatically support the indexers with Index and Range parameters on any type meet the conditions: - The type exposes Length or Count property which returning integer. - The type has indexer of integer parameter. - The type has a Slice method which takes 2 integers The change here is to remove the methods and indexers of the types which we previously added it as the compiler will automatically provide those. * Remove the JIT failing test --- src/System.Private.CoreLib/shared/System/Memory.cs | 29 ---------- .../shared/System/ReadOnlyMemory.cs | 29 ---------- .../shared/System/ReadOnlySpan.Fast.cs | 39 -------------- .../shared/System/Span.Fast.cs | 34 ------------ .../shared/System/String.Manipulation.cs | 14 ----- src/System.Private.CoreLib/shared/System/String.cs | 13 ----- .../src/System/Utf8String.Manipulation.cs | 10 ++-- .../src/System/Utf8String.cs | 19 ------- .../JitBlue/GitHub_20958/GitHub_20958.cs | 61 ---------------------- .../JitBlue/GitHub_20958/GitHub_20958.csproj | 17 ------ 10 files changed, 5 insertions(+), 260 deletions(-) delete mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_20958/GitHub_20958.cs delete mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_20958/GitHub_20958.csproj diff --git a/src/System.Private.CoreLib/shared/System/Memory.cs b/src/System.Private.CoreLib/shared/System/Memory.cs index e61ada7..e3d0347 100644 --- a/src/System.Private.CoreLib/shared/System/Memory.cs +++ b/src/System.Private.CoreLib/shared/System/Memory.cs @@ -282,35 +282,6 @@ namespace System } /// - /// Forms a slice out of the given memory, beginning at 'startIndex' - /// - /// The index at which to begin this slice. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public Memory Slice(Index startIndex) - { - int actualIndex = startIndex.GetOffset(_length); - return Slice(actualIndex); - } - - /// - /// Forms a slice out of the given memory using the range start and end indexes. - /// - /// The range used to slice the memory using its start and end indexes. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public Memory Slice(Range range) - { - (int start, int length) = range.GetOffsetAndLength(_length); - // It is expected for _index + start to be negative if the memory is already pre-pinned. - return new Memory(_object, _index + start, length); - } - - /// - /// Forms a slice out of the given memory using the range start and end indexes. - /// - /// The range used to slice the memory using its start and end indexes. - public Memory this[Range range] => Slice(range); - - /// /// Returns a span from the memory. /// public unsafe Span Span diff --git a/src/System.Private.CoreLib/shared/System/ReadOnlyMemory.cs b/src/System.Private.CoreLib/shared/System/ReadOnlyMemory.cs index b7814cf..67c85eb 100644 --- a/src/System.Private.CoreLib/shared/System/ReadOnlyMemory.cs +++ b/src/System.Private.CoreLib/shared/System/ReadOnlyMemory.cs @@ -211,35 +211,6 @@ namespace System } /// - /// Forms a slice out of the given memory, beginning at 'startIndex' - /// - /// The index at which to begin this slice. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public ReadOnlyMemory Slice(Index startIndex) - { - int actualIndex = startIndex.GetOffset(_length); - return Slice(actualIndex); - } - - /// - /// Forms a slice out of the given memory using the range start and end indexes. - /// - /// The range used to slice the memory using its start and end indexes. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public ReadOnlyMemory Slice(Range range) - { - (int start, int length) = range.GetOffsetAndLength(_length); - // It is expected for _index + start to be negative if the memory is already pre-pinned. - return new ReadOnlyMemory(_object, _index + start, length); - } - - /// - /// Forms a slice out of the given memory using the range start and end indexes. - /// - /// The range used to slice the memory using its start and end indexes. - public ReadOnlyMemory this[Range range] => Slice(range); - - /// /// Returns a span from the memory. /// public unsafe ReadOnlySpan Span diff --git a/src/System.Private.CoreLib/shared/System/ReadOnlySpan.Fast.cs b/src/System.Private.CoreLib/shared/System/ReadOnlySpan.Fast.cs index 00337a5..df49337 100644 --- a/src/System.Private.CoreLib/shared/System/ReadOnlySpan.Fast.cs +++ b/src/System.Private.CoreLib/shared/System/ReadOnlySpan.Fast.cs @@ -153,18 +153,6 @@ namespace System #endif } - public ref readonly T this[Index index] - { - get - { - // Evaluate the actual index first because it helps performance - int actualIndex = index.GetOffset(_length); - return ref this [actualIndex]; - } - } - - public ReadOnlySpan this[Range range] => Slice(range); - /// /// Returns a reference to the 0th element of the Span. If the Span is empty, returns null reference. /// It can be used for pinning and is required to support the use of span within a fixed statement. @@ -293,33 +281,6 @@ namespace System } /// - /// Forms a slice out of the given read-only span, beginning at 'startIndex' - /// - /// The index at which to begin this slice. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public ReadOnlySpan Slice(Index startIndex) - { - int actualIndex; - if (startIndex.IsFromEnd) - actualIndex = _length - startIndex.Value; - else - actualIndex = startIndex.Value; - - return Slice(actualIndex); - } - - /// - /// Forms a slice out of the given read-only span, beginning at range start index to the range end - /// - /// The range which has the start and end indexes used to slice the span. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public ReadOnlySpan Slice(Range range) - { - (int start, int length) = range.GetOffsetAndLength(_length); - return new ReadOnlySpan(ref Unsafe.Add(ref _pointer.Value, start), length); - } - - /// /// Copies the contents of this read-only span into a new array. This heap /// allocates, so should generally be avoided, however it is sometimes /// necessary to bridge the gap with APIs written in terms of arrays. diff --git a/src/System.Private.CoreLib/shared/System/Span.Fast.cs b/src/System.Private.CoreLib/shared/System/Span.Fast.cs index adc1f39..c884989 100644 --- a/src/System.Private.CoreLib/shared/System/Span.Fast.cs +++ b/src/System.Private.CoreLib/shared/System/Span.Fast.cs @@ -159,18 +159,6 @@ namespace System #endif } - public ref T this[Index index] - { - get - { - // Evaluate the actual index first because it helps performance - int actualIndex = index.GetOffset(_length); - return ref this [actualIndex]; - } - } - - public Span this[Range range] => Slice(range); - /// /// Returns a reference to the 0th element of the Span. If the Span is empty, returns null reference. /// It can be used for pinning and is required to support the use of span within a fixed statement. @@ -377,28 +365,6 @@ namespace System } /// - /// Forms a slice out of the given span, beginning at 'startIndex' - /// - /// The index at which to begin this slice. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public Span Slice(Index startIndex) - { - int actualIndex = startIndex.GetOffset(_length); - return Slice(actualIndex); - } - - /// - /// Forms a slice out of the given span, beginning at range start index to the range end - /// - /// The range which has the start and end indexes used to slice the span. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public Span Slice(Range range) - { - (int start, int length) = range.GetOffsetAndLength(_length); - return new Span(ref Unsafe.Add(ref _pointer.Value, start), length); - } - - /// /// Copies the contents of this span into a new array. This heap /// allocates, so should generally be avoided, however it is sometimes /// necessary to bridge the gap with APIs written in terms of arrays. diff --git a/src/System.Private.CoreLib/shared/System/String.Manipulation.cs b/src/System.Private.CoreLib/shared/System/String.Manipulation.cs index dc7f015..278a3b6 100644 --- a/src/System.Private.CoreLib/shared/System/String.Manipulation.cs +++ b/src/System.Private.CoreLib/shared/System/String.Manipulation.cs @@ -1680,20 +1680,6 @@ namespace System return InternalSubString(startIndex, length); } - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public string Substring(Index startIndex) - { - int actualIndex = startIndex.GetOffset(Length); - return Substring(actualIndex); - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public string Substring(Range range) - { - (int start, int length) = range.GetOffsetAndLength(Length); - return Substring(start, length); - } - private unsafe string InternalSubString(int startIndex, int length) { Debug.Assert(startIndex >= 0 && startIndex <= this.Length, "StartIndex is out of range!"); diff --git a/src/System.Private.CoreLib/shared/System/String.cs b/src/System.Private.CoreLib/shared/System/String.cs index 3ae88bb..3701434 100644 --- a/src/System.Private.CoreLib/shared/System/String.cs +++ b/src/System.Private.CoreLib/shared/System/String.cs @@ -448,19 +448,6 @@ namespace System return (value == null || 0u >= (uint)value.Length) ? true : false; } - [System.Runtime.CompilerServices.IndexerName("Chars")] - public char this[Index index] - { - get - { - int actualIndex = index.GetOffset(Length); - return this[actualIndex]; - } - } - - [System.Runtime.CompilerServices.IndexerName("Chars")] - public string this[Range range] => Substring(range); - public static bool IsNullOrWhiteSpace(string? value) { if (value == null) return true; diff --git a/src/System.Private.CoreLib/src/System/Utf8String.Manipulation.cs b/src/System.Private.CoreLib/src/System/Utf8String.Manipulation.cs index 9ca72f1..6fcdf17 100644 --- a/src/System.Private.CoreLib/src/System/Utf8String.Manipulation.cs +++ b/src/System.Private.CoreLib/src/System/Utf8String.Manipulation.cs @@ -76,12 +76,12 @@ namespace System return InternalSubstring(startIndex, length); } + // Slice intended to be used by the compiler only to provide indexer with range parameter functionality. + // Developers should be using Substring method instead. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public Utf8String Substring(Range range) - { - (int start, int length) = range.GetOffsetAndLength(Length); - return Substring(start, length); - } + [ComponentModel.EditorBrowsable(ComponentModel.EditorBrowsableState.Never)] + public Utf8String Slice(int startIndex, int length) => Substring(startIndex, length); + [StackTraceHidden] [MethodImpl(MethodImplOptions.AggressiveInlining)] diff --git a/src/System.Private.CoreLib/src/System/Utf8String.cs b/src/System.Private.CoreLib/src/System/Utf8String.cs index 8551830..f488c9b 100644 --- a/src/System.Private.CoreLib/src/System/Utf8String.cs +++ b/src/System.Private.CoreLib/src/System/Utf8String.cs @@ -84,25 +84,6 @@ namespace System } } - /// - /// Gets the at the specified position. - /// - public Char8 this[Index index] - { - get - { - // Just like String, we don't allow indexing into the null terminator itself. - - int actualIndex = index.GetOffset(Length); - return this[actualIndex]; - } - } - - /// - /// Gets a substring of this based on the provided . - /// - public Utf8String this[Range range] => Substring(range); - /* * METHODS */ diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_20958/GitHub_20958.cs b/tests/src/JIT/Regression/JitBlue/GitHub_20958/GitHub_20958.cs deleted file mode 100644 index 45730f5..0000000 --- a/tests/src/JIT/Regression/JitBlue/GitHub_20958/GitHub_20958.cs +++ /dev/null @@ -1,61 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System; - -// This test was extracted from the Indexer tests in the corefx System.Memory.Tests. -// The JIT was trying to expand the form of indexer that takes a range, but was not -// correctly expanding it, as it was expecting only the scalar index form. - -public class GitHub_20958 -{ - - public static int IndexerWithRangeTest() - { - int returnVal = 100; - - ReadOnlySpan span = "Hello".AsSpan(); - ReadOnlySpan sliced = span[new Range(new Index(1, fromEnd: false), new Index(1, fromEnd: true))]; - if (span.Slice(1, 3) != sliced) - { - returnVal = -1; - } - try - { - ReadOnlySpan s = "Hello".AsSpan()[new Range(new Index(1, fromEnd: true), new Index(1, fromEnd: false))]; - returnVal = -1; - } - catch (ArgumentOutOfRangeException) - { - } - catch (Exception e) - { - returnVal = -1; - } - Span span1 = new Span(new char[] { 'H', 'e', 'l', 'l', 'o' }); - Span sliced1 = span1[new Range(new Index(2, fromEnd: false), new Index(1, fromEnd: true))]; - if (span1.Slice(2, 2) != sliced1) - { - returnVal = -1; - } - try - { - Span s = new Span(new char[] { 'H', 'i' })[new Range(new Index(0, fromEnd: true), new Index(1, fromEnd: false))]; - returnVal = -1; - } - catch (ArgumentOutOfRangeException) - { - } - catch (Exception e) - { - returnVal = -1; - } - return returnVal; - } - - public static int Main() - { - return IndexerWithRangeTest(); - } -} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_20958/GitHub_20958.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_20958/GitHub_20958.csproj deleted file mode 100644 index d86ed9f..0000000 --- a/tests/src/JIT/Regression/JitBlue/GitHub_20958/GitHub_20958.csproj +++ /dev/null @@ -1,17 +0,0 @@ - - - - - Release - AnyCPU - $(MSBuildProjectName) - Exe - - True - - - - - - - \ No newline at end of file -- 2.7.4