From fff8f5ab763f3c1eea95ab201bfde9bd4b6e62fb Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Tue, 11 Feb 2020 18:34:58 -0800 Subject: [PATCH] Replace default(T) != null with typeof(T).IsValueType (#32098) Only affects call sites which weren't intending to special-case Nullable --- .../src/System/Collections/Concurrent/PartitionerStatic.cs | 5 +---- .../src/System/Collections/Immutable/ImmutableExtensions.cs | 4 ++++ .../src/System/Collections/Generic/HashSet.cs | 8 ++++---- .../src/System/Linq/Parallel/Scheduling/Scheduling.cs | 5 +---- .../src/System/Collections/Generic/Dictionary.cs | 12 ++++++------ src/libraries/System.Private.CoreLib/src/System/Memory.cs | 6 +++--- .../System.Private.CoreLib/src/System/MemoryExtensions.cs | 6 +++--- .../src/System/Runtime/CompilerServices/RuntimeHelpers.cs | 2 +- .../src/System/Runtime/InteropServices/MemoryMarshal.cs | 2 +- src/libraries/System.Private.CoreLib/src/System/Span.cs | 4 ++-- 10 files changed, 26 insertions(+), 28 deletions(-) diff --git a/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/PartitionerStatic.cs b/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/PartitionerStatic.cs index 06df026..3d8371f 100644 --- a/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/PartitionerStatic.cs +++ b/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/PartitionerStatic.cs @@ -1671,10 +1671,7 @@ namespace System.Collections.Concurrent { int chunkSize; - // Because of the lack of typeof(T).IsValueType we need two pieces of information - // to determine this. default(T) will return a non null for Value Types, except those - // using Nullable<>, that is why we need a second condition. - if (default(TSource) != null || Nullable.GetUnderlyingType(typeof(TSource)) != null) + if (typeof(TSource).IsValueType) { // Marshal.SizeOf fails for value types that don't have explicit layouts. We // just fall back to some arbitrary constant in that case. Is there a better way? diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableExtensions.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableExtensions.cs index c4f908b..2c2f6cf 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableExtensions.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableExtensions.cs @@ -17,6 +17,9 @@ namespace System.Collections.Immutable { internal static bool IsValueType() { +#if NETCOREAPP + return typeof(T).IsValueType; +#else if (default(T) != null) { return true; @@ -29,6 +32,7 @@ namespace System.Collections.Immutable } return false; +#endif } #if EqualsStructurally diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/HashSet.cs b/src/libraries/System.Collections/src/System/Collections/Generic/HashSet.cs index 8b2486b..db68e78 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/HashSet.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/HashSet.cs @@ -266,7 +266,7 @@ namespace System.Collections.Generic { int hashCode = item == null ? 0 : InternalGetHashCode(item.GetHashCode()); - if (default(T) != null) + if (typeof(T).IsValueType) { // see note at "HashSet" level describing why "- 1" appears in for loop for (int i = buckets[hashCode % buckets.Length] - 1; i >= 0; i = slots[i].next) @@ -368,7 +368,7 @@ namespace System.Collections.Generic hashCode = item == null ? 0 : InternalGetHashCode(item.GetHashCode()); bucket = hashCode % _buckets!.Length; - if (default(T) != null) + if (typeof(T).IsValueType) { for (i = _buckets[bucket] - 1; i >= 0; last = i, i = slots[i].next) { @@ -1341,7 +1341,7 @@ namespace System.Collections.Generic hashCode = value == null ? 0 : InternalGetHashCode(value.GetHashCode()); bucket = hashCode % _buckets!.Length; - if (default(T) != null) + if (typeof(T).IsValueType) { for (int i = _buckets[bucket] - 1; i >= 0; i = slots[i].next) { @@ -1577,7 +1577,7 @@ namespace System.Collections.Generic { int hashCode = item == null ? 0 : InternalGetHashCode(item.GetHashCode()); - if (default(T) != null) + if (typeof(T).IsValueType) { // see note at "HashSet" level describing why "- 1" appears in for loop for (int i = buckets[hashCode % buckets.Length] - 1; i >= 0; i = slots[i].next) diff --git a/src/libraries/System.Linq.Parallel/src/System/Linq/Parallel/Scheduling/Scheduling.cs b/src/libraries/System.Linq.Parallel/src/System/Linq/Parallel/Scheduling/Scheduling.cs index c75997d..02205ea 100644 --- a/src/libraries/System.Linq.Parallel/src/System/Linq/Parallel/Scheduling/Scheduling.cs +++ b/src/libraries/System.Linq.Parallel/src/System/Linq/Parallel/Scheduling/Scheduling.cs @@ -75,10 +75,7 @@ namespace System.Linq.Parallel { int chunkSize; - // Because of the lack of typeof(T).IsValueType we need two pieces of information - // to determine this. default(T) will return a non null for Value Types, except those - // using Nullable<>, that is why we need a second condition. - if (default(T) != null || Nullable.GetUnderlyingType(typeof(T)) != null) + if (typeof(T).IsValueType) { // Marshal.SizeOf fails for value types that don't have explicit layouts. We // just fall back to some arbitrary constant in that case. Is there a better way? diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs index 968cbcc..0ea5ce6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs @@ -250,7 +250,7 @@ namespace System.Collections.Generic } else { - if (default(TValue) != null) + if (typeof(TValue).IsValueType) { // ValueType: Devirtualize with EqualityComparer.Default intrinsic for (int i = 0; i < _count; i++) @@ -344,7 +344,7 @@ namespace System.Collections.Generic int i = GetBucket(hashCode); Entry[]? entries = _entries; uint collisionCount = 0; - if (default(TKey) != null) + if (typeof(TKey).IsValueType) { // ValueType: Devirtualize with EqualityComparer.Default intrinsic @@ -495,7 +495,7 @@ namespace System.Collections.Generic if (comparer == null) { - if (default(TKey) != null) + if (typeof(TKey).IsValueType) { // ValueType: Devirtualize with EqualityComparer.Default intrinsic while (true) @@ -658,7 +658,7 @@ namespace System.Collections.Generic _version++; // Value types never rehash - if (default(TKey) == null && collisionCount > HashHelpers.HashCollisionThreshold && comparer is NonRandomizedStringEqualityComparer) + if (!typeof(TKey).IsValueType && collisionCount > HashHelpers.HashCollisionThreshold && comparer is NonRandomizedStringEqualityComparer) { // If we hit the collision threshold we'll need to switch to the comparer which is using randomized string hashing // i.e. EqualityComparer.Default. @@ -720,7 +720,7 @@ namespace System.Collections.Generic private void Resize(int newSize, bool forceNewHashCodes) { // Value types never rehash - Debug.Assert(!forceNewHashCodes || default(TKey) == null); + Debug.Assert(!forceNewHashCodes || !typeof(TKey).IsValueType); Debug.Assert(_entries != null, "_entries should be non-null"); Debug.Assert(newSize >= _entries.Length); @@ -729,7 +729,7 @@ namespace System.Collections.Generic int count = _count; Array.Copy(_entries, entries, count); - if (default(TKey) == null && forceNewHashCodes) + if (!typeof(TKey).IsValueType && forceNewHashCodes) { for (int i = 0; i < count; i++) { diff --git a/src/libraries/System.Private.CoreLib/src/System/Memory.cs b/src/libraries/System.Private.CoreLib/src/System/Memory.cs index f10b1f1..cb8f631 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Memory.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Memory.cs @@ -53,7 +53,7 @@ namespace System this = default; return; // returns default } - if (default(T) == null && array.GetType() != typeof(T[])) + if (!typeof(T).IsValueType && array.GetType() != typeof(T[])) ThrowHelper.ThrowArrayTypeMismatchException(); _object = array; @@ -71,7 +71,7 @@ namespace System this = default; return; // returns default } - if (default(T) == null && array.GetType() != typeof(T[])) + if (!typeof(T).IsValueType && array.GetType() != typeof(T[])) ThrowHelper.ThrowArrayTypeMismatchException(); if ((uint)start > (uint)array.Length) ThrowHelper.ThrowArgumentOutOfRangeException(); @@ -103,7 +103,7 @@ namespace System this = default; return; // returns default } - if (default(T) == null && array.GetType() != typeof(T[])) + if (!typeof(T).IsValueType && array.GetType() != typeof(T[])) ThrowHelper.ThrowArrayTypeMismatchException(); #if TARGET_64BIT // See comment in Span.Slice for how this works. diff --git a/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs b/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs index d17a179..2a66efa 100644 --- a/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs +++ b/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs @@ -34,7 +34,7 @@ namespace System ThrowHelper.ThrowArgumentOutOfRangeException(); return default; } - if (default(T) == null && array.GetType() != typeof(T[])) + if (!typeof(T).IsValueType && array.GetType() != typeof(T[])) ThrowHelper.ThrowArrayTypeMismatchException(); if ((uint)start > (uint)array.Length) ThrowHelper.ThrowArgumentOutOfRangeException(); @@ -56,7 +56,7 @@ namespace System return default; } - if (default(T) == null && array.GetType() != typeof(T[])) + if (!typeof(T).IsValueType && array.GetType() != typeof(T[])) ThrowHelper.ThrowArrayTypeMismatchException(); int actualIndex = startIndex.GetOffset(array.Length); @@ -83,7 +83,7 @@ namespace System return default; } - if (default(T) == null && array.GetType() != typeof(T[])) + if (!typeof(T).IsValueType && array.GetType() != typeof(T[])) ThrowHelper.ThrowArrayTypeMismatchException(); (int start, int length) = range.GetOffsetAndLength(array.Length); diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs index 8860428..3f06b3d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs @@ -27,7 +27,7 @@ namespace System.Runtime.CompilerServices (int offset, int length) = range.GetOffsetAndLength(array.Length); - if (default(T) != null || typeof(T[]) == array.GetType()) + if (typeof(T).IsValueType || typeof(T[]) == array.GetType()) { // We know the type of the array to be exactly T[]. diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs index 16d431c..a03ae91 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs @@ -522,7 +522,7 @@ namespace System.Runtime.InteropServices ThrowHelper.ThrowArgumentOutOfRangeException(); return default; } - if (default(T) == null && array.GetType() != typeof(T[])) + if (!typeof(T).IsValueType && array.GetType() != typeof(T[])) ThrowHelper.ThrowArrayTypeMismatchException(); if ((uint)start > (uint)array.Length || (uint)length > (uint)(array.Length - start)) ThrowHelper.ThrowArgumentOutOfRangeException(); diff --git a/src/libraries/System.Private.CoreLib/src/System/Span.cs b/src/libraries/System.Private.CoreLib/src/System/Span.cs index 8a4f397..106b091 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Span.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Span.cs @@ -50,7 +50,7 @@ namespace System this = default; return; // returns default } - if (default(T) == null && array.GetType() != typeof(T[])) + if (!typeof(T).IsValueType && array.GetType() != typeof(T[])) ThrowHelper.ThrowArrayTypeMismatchException(); _pointer = new ByReference(ref MemoryMarshal.GetArrayDataReference(array)); @@ -79,7 +79,7 @@ namespace System this = default; return; // returns default } - if (default(T) == null && array.GetType() != typeof(T[])) + if (!typeof(T).IsValueType && array.GetType() != typeof(T[])) ThrowHelper.ThrowArrayTypeMismatchException(); #if TARGET_64BIT // See comment in Span.Slice for how this works. -- 2.7.4