From 0051e43fc7cd14a8b3c326c2a7880145efd345af Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Fri, 27 Dec 2019 23:54:58 -0800 Subject: [PATCH 1/2] Add asserts to catch unintentional use of generic overloads Delete unnecessary ifdefs --- .../ConfiguredValueTaskAwaitable.cs | 2 - .../CompilerServices/ValueTaskAwaiter.cs | 2 - .../src/System/SpanHelpers.T.cs | 39 ++++++++++++++++++- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConfiguredValueTaskAwaitable.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConfiguredValueTaskAwaitable.cs index 493269218dffa8..78d70e7703c64b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConfiguredValueTaskAwaitable.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConfiguredValueTaskAwaitable.cs @@ -8,9 +8,7 @@ using System.Threading.Tasks; using System.Threading.Tasks.Sources; -#if !NETSTANDARD2_0 using Internal.Runtime.CompilerServices; -#endif namespace System.Runtime.CompilerServices { diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ValueTaskAwaiter.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ValueTaskAwaiter.cs index b3817d33904172..07f9441518584a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ValueTaskAwaiter.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ValueTaskAwaiter.cs @@ -7,9 +7,7 @@ using System.Threading.Tasks; using System.Threading.Tasks.Sources; -#if !NETSTANDARD2_0 using Internal.Runtime.CompilerServices; -#endif namespace System.Runtime.CompilerServices { diff --git a/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.T.cs b/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.T.cs index d5efbd5fae2535..a40b1162d6b6df 100644 --- a/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.T.cs +++ b/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.T.cs @@ -3,9 +3,8 @@ // See the LICENSE file in the project root for more information. using System.Diagnostics; -#if !NETSTANDARD2_0 + using Internal.Runtime.CompilerServices; -#endif namespace System { @@ -13,6 +12,9 @@ internal static partial class SpanHelpers // .T { public static int IndexOf(ref T searchSpace, int searchSpaceLength, ref T value, int valueLength) where T : IEquatable { + // The optimized implementation should be used for these types + Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(searchSpaceLength >= 0); Debug.Assert(valueLength >= 0); @@ -49,6 +51,9 @@ public static int IndexOf(ref T searchSpace, int searchSpaceLength, ref T val // Adapted from IndexOf(...) public static unsafe bool Contains(ref T searchSpace, T value, int length) where T : IEquatable { + // The optimized implementation should be used for these types + Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(length >= 0); IntPtr index = (IntPtr)0; // Use IntPtr for arithmetic to avoid unnecessary 64->32->64 truncations @@ -119,6 +124,9 @@ public static unsafe bool Contains(ref T searchSpace, T value, int length) wh public static unsafe int IndexOf(ref T searchSpace, T value, int length) where T : IEquatable { + // The optimized implementation should be used for these types + Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(length >= 0); IntPtr index = (IntPtr)0; // Use IntPtr for arithmetic to avoid unnecessary 64->32->64 truncations @@ -206,6 +214,9 @@ public static unsafe int IndexOf(ref T searchSpace, T value, int length) wher public static int IndexOfAny(ref T searchSpace, T value0, T value1, int length) where T : IEquatable { + // The optimized implementation should be used for these types + Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(length >= 0); T lookUp; @@ -310,6 +321,9 @@ public static int IndexOfAny(ref T searchSpace, T value0, T value1, int lengt public static int IndexOfAny(ref T searchSpace, T value0, T value1, T value2, int length) where T : IEquatable { + // The optimized implementation should be used for these types + Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(length >= 0); T lookUp; @@ -413,6 +427,9 @@ public static int IndexOfAny(ref T searchSpace, T value0, T value1, T value2, public static int IndexOfAny(ref T searchSpace, int searchSpaceLength, ref T value, int valueLength) where T : IEquatable { + // The optimized implementation should be used for these types + Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(searchSpaceLength >= 0); Debug.Assert(valueLength >= 0); @@ -472,6 +489,9 @@ public static int LastIndexOf(ref T searchSpace, int searchSpaceLength, ref T public static int LastIndexOf(ref T searchSpace, T value, int length) where T : IEquatable { + // The optimized implementation should be used for these types + Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(length >= 0); if (default(T)! != null || (object)value != null) // TODO-NULLABLE: default(T) == null warning (https://github.com/dotnet/roslyn/issues/34757) @@ -553,6 +573,9 @@ public static int LastIndexOf(ref T searchSpace, T value, int length) where T public static int LastIndexOfAny(ref T searchSpace, T value0, T value1, int length) where T : IEquatable { + // The optimized implementation should be used for these types + Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(length >= 0); T lookUp; @@ -656,6 +679,9 @@ public static int LastIndexOfAny(ref T searchSpace, T value0, T value1, int l public static int LastIndexOfAny(ref T searchSpace, T value0, T value1, T value2, int length) where T : IEquatable { + // The optimized implementation should be used for these types + Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(length >= 0); T lookUp; @@ -759,6 +785,9 @@ public static int LastIndexOfAny(ref T searchSpace, T value0, T value1, T val public static int LastIndexOfAny(ref T searchSpace, int searchSpaceLength, ref T value, int valueLength) where T : IEquatable { + // The optimized implementation should be used for these types + Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(searchSpaceLength >= 0); Debug.Assert(valueLength >= 0); @@ -777,6 +806,9 @@ public static int LastIndexOfAny(ref T searchSpace, int searchSpaceLength, re public static bool SequenceEqual(ref T first, ref T second, int length) where T : IEquatable { + // The optimized implementation should be used for these types + Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(length >= 0); if (Unsafe.AreSame(ref first, ref second)) @@ -869,6 +901,9 @@ public static bool SequenceEqual(ref T first, ref T second, int length) where public static int SequenceCompareTo(ref T first, int firstLength, ref T second, int secondLength) where T : IComparable { + // The optimized implementation should be used for these types + Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(firstLength >= 0); Debug.Assert(secondLength >= 0); From 9deab3f2b6b4c4e580e598ea2b45858ec0ec1b5d Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sat, 28 Dec 2019 08:21:08 -0800 Subject: [PATCH 2/2] CR feedback, bugfix --- .../src/System/RuntimeType.CoreCLR.cs | 2 +- .../src/System/SpanHelpers.T.cs | 23 ++++++++++--------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs index 0b7da5b14c47b3..7c92adc25ed33e 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs @@ -4288,7 +4288,7 @@ internal bool Equals(MdUtf8String s) } else { - return SpanHelpers.SequenceEqual(ref *s.m_pStringHeap, ref *m_pStringHeap, m_StringHeapByteLength); + return SpanHelpers.SequenceEqual(ref *s.m_pStringHeap, ref *m_pStringHeap, (uint)m_StringHeapByteLength); } } diff --git a/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.T.cs b/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.T.cs index a40b1162d6b6df..87918217de1db6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.T.cs +++ b/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.T.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Diagnostics; +using System.Runtime.CompilerServices; using Internal.Runtime.CompilerServices; @@ -13,7 +14,7 @@ internal static partial class SpanHelpers // .T public static int IndexOf(ref T searchSpace, int searchSpaceLength, ref T value, int valueLength) where T : IEquatable { // The optimized implementation should be used for these types - Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(!RuntimeHelpers.IsBitwiseEquatable() || !(Unsafe.SizeOf() == sizeof(byte) || Unsafe.SizeOf() == sizeof(char))); Debug.Assert(searchSpaceLength >= 0); Debug.Assert(valueLength >= 0); @@ -52,7 +53,7 @@ public static int IndexOf(ref T searchSpace, int searchSpaceLength, ref T val public static unsafe bool Contains(ref T searchSpace, T value, int length) where T : IEquatable { // The optimized implementation should be used for these types - Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(!RuntimeHelpers.IsBitwiseEquatable() || !(Unsafe.SizeOf() == sizeof(byte) || Unsafe.SizeOf() == sizeof(char))); Debug.Assert(length >= 0); @@ -125,7 +126,7 @@ public static unsafe bool Contains(ref T searchSpace, T value, int length) wh public static unsafe int IndexOf(ref T searchSpace, T value, int length) where T : IEquatable { // The optimized implementation should be used for these types - Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(!RuntimeHelpers.IsBitwiseEquatable() || !(Unsafe.SizeOf() == sizeof(byte) || Unsafe.SizeOf() == sizeof(char))); Debug.Assert(length >= 0); @@ -215,7 +216,7 @@ public static unsafe int IndexOf(ref T searchSpace, T value, int length) wher public static int IndexOfAny(ref T searchSpace, T value0, T value1, int length) where T : IEquatable { // The optimized implementation should be used for these types - Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(!RuntimeHelpers.IsBitwiseEquatable() || !(Unsafe.SizeOf() == sizeof(byte) || Unsafe.SizeOf() == sizeof(char))); Debug.Assert(length >= 0); @@ -322,7 +323,7 @@ public static int IndexOfAny(ref T searchSpace, T value0, T value1, int lengt public static int IndexOfAny(ref T searchSpace, T value0, T value1, T value2, int length) where T : IEquatable { // The optimized implementation should be used for these types - Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(!RuntimeHelpers.IsBitwiseEquatable() || !(Unsafe.SizeOf() == sizeof(byte) || Unsafe.SizeOf() == sizeof(char))); Debug.Assert(length >= 0); @@ -428,7 +429,7 @@ public static int IndexOfAny(ref T searchSpace, T value0, T value1, T value2, public static int IndexOfAny(ref T searchSpace, int searchSpaceLength, ref T value, int valueLength) where T : IEquatable { // The optimized implementation should be used for these types - Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(!RuntimeHelpers.IsBitwiseEquatable() || !(Unsafe.SizeOf() == sizeof(byte) || Unsafe.SizeOf() == sizeof(char))); Debug.Assert(searchSpaceLength >= 0); Debug.Assert(valueLength >= 0); @@ -490,7 +491,7 @@ public static int LastIndexOf(ref T searchSpace, int searchSpaceLength, ref T public static int LastIndexOf(ref T searchSpace, T value, int length) where T : IEquatable { // The optimized implementation should be used for these types - Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(!RuntimeHelpers.IsBitwiseEquatable() || !(Unsafe.SizeOf() == sizeof(byte) || Unsafe.SizeOf() == sizeof(char))); Debug.Assert(length >= 0); @@ -574,7 +575,7 @@ public static int LastIndexOf(ref T searchSpace, T value, int length) where T public static int LastIndexOfAny(ref T searchSpace, T value0, T value1, int length) where T : IEquatable { // The optimized implementation should be used for these types - Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(!RuntimeHelpers.IsBitwiseEquatable() || !(Unsafe.SizeOf() == sizeof(byte) || Unsafe.SizeOf() == sizeof(char))); Debug.Assert(length >= 0); @@ -680,7 +681,7 @@ public static int LastIndexOfAny(ref T searchSpace, T value0, T value1, int l public static int LastIndexOfAny(ref T searchSpace, T value0, T value1, T value2, int length) where T : IEquatable { // The optimized implementation should be used for these types - Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(!RuntimeHelpers.IsBitwiseEquatable() || !(Unsafe.SizeOf() == sizeof(byte) || Unsafe.SizeOf() == sizeof(char))); Debug.Assert(length >= 0); @@ -786,7 +787,7 @@ public static int LastIndexOfAny(ref T searchSpace, T value0, T value1, T val public static int LastIndexOfAny(ref T searchSpace, int searchSpaceLength, ref T value, int valueLength) where T : IEquatable { // The optimized implementation should be used for these types - Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(!RuntimeHelpers.IsBitwiseEquatable() || !(Unsafe.SizeOf() == sizeof(byte) || Unsafe.SizeOf() == sizeof(char))); Debug.Assert(searchSpaceLength >= 0); Debug.Assert(valueLength >= 0); @@ -807,7 +808,7 @@ public static int LastIndexOfAny(ref T searchSpace, int searchSpaceLength, re public static bool SequenceEqual(ref T first, ref T second, int length) where T : IEquatable { // The optimized implementation should be used for these types - Debug.Assert(typeof(T) != typeof(byte) && typeof(T) != typeof(char)); + Debug.Assert(!RuntimeHelpers.IsBitwiseEquatable() || !(Unsafe.SizeOf() == sizeof(byte) || Unsafe.SizeOf() == sizeof(char))); Debug.Assert(length >= 0);