From 7be97855bb91a9c30beda1c7065b740ee393a1e9 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Mon, 3 Apr 2023 23:26:54 -0400 Subject: [PATCH] Fix FrozenDictionary/Set handling of ValueTuple keys For small collections of comparable value types, we were using an implementation that sorted the keys in order to a) quickly rule out values outside of the known contained range, and b) stop searching when we hit a value that was too small. However, this can break for some well-known types. In particular, `ValueTuple<...>` implements `IComparable>`, but it might throw an exception if you actually try to use its `IComparable<>` implementation if any of the `T` types in the tuple are themselves not comparable. Since we have no good way then to dynamically select an implementation based on whether it implements `IComparable<>`, I've simply removed those checks / calls from the implementation. Testing this also highlighted that our existing shared set tests don't like being given non-comparable types, as they use SortedSet which itself suffers from effectively the same issue (but there you can choose to not use SortedSet if it doesn't work for your data, and "sort"ing is part of the name). --- .../System/Collections/ISet.Generic.Tests.cs | 15 ++-- .../System/Collections/TestBase.Generic.cs | 2 +- .../src/System.Collections.Immutable.csproj | 4 +- .../System/Collections/Frozen/Constants.cs | 2 +- .../Collections/Frozen/FrozenDictionary.cs | 5 +- .../System/Collections/Frozen/FrozenSet.cs | 5 +- ...mallComparableValueTypeFrozenDictionary.cs | 73 ----------------- .../SmallComparableValueTypeFrozenSet.cs | 79 ------------------- ...alueTypeDefaultComparerFrozenDictionary.cs | 50 ++++++++++++ .../SmallValueTypeDefaultComparerFrozenSet.cs | 56 +++++++++++++ .../tests/Frozen/FrozenDictionaryTests.cs | 61 +++++++++++--- .../tests/Frozen/FrozenSetTests.cs | 27 ++++--- 12 files changed, 189 insertions(+), 190 deletions(-) delete mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallComparableValueTypeFrozenDictionary.cs delete mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallComparableValueTypeFrozenSet.cs create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallValueTypeDefaultComparerFrozenDictionary.cs create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallValueTypeDefaultComparerFrozenSet.cs diff --git a/src/libraries/Common/tests/System/Collections/ISet.Generic.Tests.cs b/src/libraries/Common/tests/System/Collections/ISet.Generic.Tests.cs index 9c0b36df38eeb..303001e8bd144 100644 --- a/src/libraries/Common/tests/System/Collections/ISet.Generic.Tests.cs +++ b/src/libraries/Common/tests/System/Collections/ISet.Generic.Tests.cs @@ -325,6 +325,9 @@ public void ISet_Generic_NullEnumerableArgument(int count) } } + public static IEnumerable SetTestData() => + new[] { EnumerableType.HashSet, EnumerableType.List }.SelectMany(GetEnumerableTestData); + [Theory] [MemberData(nameof(EnumerableTestData))] public void ISet_Generic_ExceptWith(EnumerableType enumerableType, int setLength, int enumerableLength, int numberOfMatchingElements, int numberOfDuplicateElements) @@ -350,7 +353,7 @@ public void ISet_Generic_IntersectWith(EnumerableType enumerableType, int setLen } [Theory] - [MemberData(nameof(EnumerableTestData))] + [MemberData(nameof(SetTestData))] public void ISet_Generic_IsProperSubsetOf(EnumerableType enumerableType, int setLength, int enumerableLength, int numberOfMatchingElements, int numberOfDuplicateElements) { ISet set = GenericISetFactory(setLength); @@ -359,7 +362,7 @@ public void ISet_Generic_IsProperSubsetOf(EnumerableType enumerableType, int set } [Theory] - [MemberData(nameof(EnumerableTestData))] + [MemberData(nameof(SetTestData))] public void ISet_Generic_IsProperSupersetOf(EnumerableType enumerableType, int setLength, int enumerableLength, int numberOfMatchingElements, int numberOfDuplicateElements) { ISet set = GenericISetFactory(setLength); @@ -368,7 +371,7 @@ public void ISet_Generic_IsProperSupersetOf(EnumerableType enumerableType, int s } [Theory] - [MemberData(nameof(EnumerableTestData))] + [MemberData(nameof(SetTestData))] public void ISet_Generic_IsSubsetOf(EnumerableType enumerableType, int setLength, int enumerableLength, int numberOfMatchingElements, int numberOfDuplicateElements) { ISet set = GenericISetFactory(setLength); @@ -377,7 +380,7 @@ public void ISet_Generic_IsSubsetOf(EnumerableType enumerableType, int setLength } [Theory] - [MemberData(nameof(EnumerableTestData))] + [MemberData(nameof(SetTestData))] public void ISet_Generic_IsSupersetOf(EnumerableType enumerableType, int setLength, int enumerableLength, int numberOfMatchingElements, int numberOfDuplicateElements) { ISet set = GenericISetFactory(setLength); @@ -386,7 +389,7 @@ public void ISet_Generic_IsSupersetOf(EnumerableType enumerableType, int setLeng } [Theory] - [MemberData(nameof(EnumerableTestData))] + [MemberData(nameof(SetTestData))] public void ISet_Generic_Overlaps(EnumerableType enumerableType, int setLength, int enumerableLength, int numberOfMatchingElements, int numberOfDuplicateElements) { ISet set = GenericISetFactory(setLength); @@ -395,7 +398,7 @@ public void ISet_Generic_Overlaps(EnumerableType enumerableType, int setLength, } [Theory] - [MemberData(nameof(EnumerableTestData))] + [MemberData(nameof(SetTestData))] public void ISet_Generic_SetEquals(EnumerableType enumerableType, int setLength, int enumerableLength, int numberOfMatchingElements, int numberOfDuplicateElements) { ISet set = GenericISetFactory(setLength); diff --git a/src/libraries/Common/tests/System/Collections/TestBase.Generic.cs b/src/libraries/Common/tests/System/Collections/TestBase.Generic.cs index 3769da939c0e6..25cdb1cb5ac9c 100644 --- a/src/libraries/Common/tests/System/Collections/TestBase.Generic.cs +++ b/src/libraries/Common/tests/System/Collections/TestBase.Generic.cs @@ -47,7 +47,7 @@ public static IEnumerable EnumerableTestData() => public static IEnumerable ListTestData() => GetEnumerableTestData(EnumerableType.List); - private static IEnumerable GetEnumerableTestData(EnumerableType enumerableType) + protected static IEnumerable GetEnumerableTestData(EnumerableType enumerableType) { foreach (object[] collectionSizeArray in ValidCollectionSizes()) { diff --git a/src/libraries/System.Collections.Immutable/src/System.Collections.Immutable.csproj b/src/libraries/System.Collections.Immutable/src/System.Collections.Immutable.csproj index 83a02151b0a73..5613158ff7855 100644 --- a/src/libraries/System.Collections.Immutable/src/System.Collections.Immutable.csproj +++ b/src/libraries/System.Collections.Immutable/src/System.Collections.Immutable.csproj @@ -29,8 +29,8 @@ The System.Collections.Immutable library is built-in as part of the shared frame - - + + diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Constants.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Constants.cs index 7e1a953a4cf17..b65f051f6b992 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Constants.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Constants.cs @@ -27,6 +27,6 @@ internal static class Constants /// SmallValueTypeDefaultComparerFrozenDictionary/Set to the /// hashing ValueTypeDefaultComparerFrozenDictionary/Set. /// - public const int MaxItemsInSmallComparableValueTypeFrozenCollection = 10; + public const int MaxItemsInSmallValueTypeFrozenCollection = 10; } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.cs index e0b8af841bb00..eca5a39bbb591 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.cs @@ -200,10 +200,9 @@ private static FrozenDictionary ChooseImplementationOptimizedForRe // the Equals/GetHashCode methods to be devirtualized and possibly inlined. if (ReferenceEquals(comparer, EqualityComparer.Default)) { - if (default(TKey) is IComparable && - source.Count <= Constants.MaxItemsInSmallComparableValueTypeFrozenCollection) + if (source.Count <= Constants.MaxItemsInSmallValueTypeFrozenCollection) { - return (FrozenDictionary)(object)new SmallComparableValueTypeFrozenDictionary(source); + return (FrozenDictionary)(object)new SmallValueTypeDefaultComparerFrozenDictionary(source); } if (typeof(TKey) == typeof(int)) diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.cs index e267afb00320d..69638e8acd106 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.cs @@ -131,10 +131,9 @@ private static FrozenSet ChooseImplementationOptimizedForReading(HashSet.Default)) { - if (default(T) is IComparable && - source.Count <= Constants.MaxItemsInSmallComparableValueTypeFrozenCollection) + if (source.Count <= Constants.MaxItemsInSmallValueTypeFrozenCollection) { - return (FrozenSet)(object)new SmallComparableValueTypeFrozenSet(source); + return (FrozenSet)(object)new SmallValueTypeDefaultComparerFrozenSet(source); } if (typeof(T) == typeof(int)) diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallComparableValueTypeFrozenDictionary.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallComparableValueTypeFrozenDictionary.cs deleted file mode 100644 index 6b55e99192156..0000000000000 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallComparableValueTypeFrozenDictionary.cs +++ /dev/null @@ -1,73 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Collections.Generic; -using System.Diagnostics; -using System.Linq; -using System.Runtime.CompilerServices; - -namespace System.Collections.Frozen -{ - /// Provides a frozen dictionary to use when the key is a comparable value type, the default comparer is used, and the item count is small. - /// - /// No hashing involved, just a linear scan through the keys. This implementation is close in nature to that of , - /// except that this implementation sorts the keys in order to a) extract a max that it can compare against at the beginning of each match in order to - /// immediately rule out keys too large to be contained, and b) early-exits from the linear scan when a comparison determines the key is too - /// small to be contained. - /// - internal sealed class SmallComparableValueTypeFrozenDictionary : FrozenDictionary - where TKey : notnull - { - private readonly TKey[] _keys; - private readonly TValue[] _values; - private readonly TKey _max; - - internal SmallComparableValueTypeFrozenDictionary(Dictionary source) : base(EqualityComparer.Default) - { - // TKey is logically constrained to `where TKey : struct, IComparable`, but we can't actually write that - // constraint currently and still have this be used from the calling context that has an unconstrained TKey. - // So, we assert it here instead. The implementation relies on {Equality}Comparer.Default to sort things out. - Debug.Assert(default(TKey) is IComparable); - Debug.Assert(default(TKey) is not null); - Debug.Assert(typeof(TKey).IsValueType); - - Debug.Assert(source.Count != 0); - Debug.Assert(ReferenceEquals(source.Comparer, EqualityComparer.Default)); - - _keys = source.Keys.ToArray(); - _values = source.Values.ToArray(); - Array.Sort(_keys, _values); - - _max = _keys[_keys.Length - 1]; - } - - private protected override TKey[] KeysCore => _keys; - private protected override TValue[] ValuesCore => _values; - private protected override Enumerator GetEnumeratorCore() => new Enumerator(_keys, _values); - private protected override int CountCore => _keys.Length; - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private protected override ref readonly TValue GetValueRefOrNullRefCore(TKey key) - { - if (Comparer.Default.Compare(key, _max) <= 0) - { - TKey[] keys = _keys; - for (int i = 0; i < keys.Length; i++) - { - int c = Comparer.Default.Compare(key, keys[i]); - if (c <= 0) - { - if (c == 0) - { - return ref _values[i]; - } - - break; - } - } - } - - return ref Unsafe.NullRef(); - } - } -} diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallComparableValueTypeFrozenSet.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallComparableValueTypeFrozenSet.cs deleted file mode 100644 index 99a30dca3dca5..0000000000000 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallComparableValueTypeFrozenSet.cs +++ /dev/null @@ -1,79 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Collections.Generic; -using System.Collections.Immutable; -using System.Diagnostics; -using System.Linq; - -namespace System.Collections.Frozen -{ - /// Provides a frozen set to use when the item is a comparable value type, the default comparer is used, and the item count is small. - /// - /// No hashing involved, just a linear scan through the items. This implementation is close in nature to that of , - /// except that this implementation sorts the values in order to a) extract a max that it can compare against at the beginning of each match in order to - /// immediately rule out values too large to be contained, and b) early-exits from the linear scan when a comparison determines the value is too - /// small to be contained. - /// - internal sealed class SmallComparableValueTypeFrozenSet : FrozenSetInternalBase.GSW> - { - private readonly T[] _items; - private readonly T _max; - - internal SmallComparableValueTypeFrozenSet(HashSet source) : base(EqualityComparer.Default) - { - // T is logically constrained to `where T : struct, IComparable`, but we can't actually write that - // constraint currently and still have this be used from the calling context that has an unconstrained T. - // So, we assert it here instead. The implementation relies on {Equality}Comparer.Default to sort things out. - Debug.Assert(default(T) is IComparable); - Debug.Assert(default(T) is not null); - Debug.Assert(typeof(T).IsValueType); - - Debug.Assert(source.Count != 0); - Debug.Assert(ReferenceEquals(source.Comparer, EqualityComparer.Default)); - - _items = source.ToArray(); - Array.Sort(_items); - - _max = _items[_items.Length - 1]; - } - - private protected override T[] ItemsCore => _items; - private protected override Enumerator GetEnumeratorCore() => new Enumerator(_items); - private protected override int CountCore => _items.Length; - - private protected override int FindItemIndex(T item) - { - if (Comparer.Default.Compare(item, _max) <= 0) - { - T[] items = _items; - for (int i = 0; i < items.Length; i++) - { - int c = Comparer.Default.Compare(item, items[i]); - if (c <= 0) - { - if (c == 0) - { - return i; - } - - break; - } - } - } - - return -1; - } - - internal struct GSW : IGenericSpecializedWrapper - { - private SmallComparableValueTypeFrozenSet _set; - public void Store(FrozenSet set) => _set = (SmallComparableValueTypeFrozenSet)set; - - public int Count => _set.Count; - public IEqualityComparer Comparer => _set.Comparer; - public int FindItemIndex(T item) => _set.FindItemIndex(item); - public Enumerator GetEnumerator() => _set.GetEnumerator(); - } - } -} diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallValueTypeDefaultComparerFrozenDictionary.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallValueTypeDefaultComparerFrozenDictionary.cs new file mode 100644 index 0000000000000..d9b4fcc46176a --- /dev/null +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallValueTypeDefaultComparerFrozenDictionary.cs @@ -0,0 +1,50 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Runtime.CompilerServices; + +namespace System.Collections.Frozen +{ + /// Provides a frozen dictionary to use when the key is a value type, the default comparer is used, and the item count is small. + internal sealed class SmallValueTypeDefaultComparerFrozenDictionary : FrozenDictionary + where TKey : notnull + { + private readonly TKey[] _keys; + private readonly TValue[] _values; + + internal SmallValueTypeDefaultComparerFrozenDictionary(Dictionary source) : base(EqualityComparer.Default) + { + Debug.Assert(default(TKey) is not null); + Debug.Assert(typeof(TKey).IsValueType); + + Debug.Assert(source.Count != 0); + Debug.Assert(ReferenceEquals(source.Comparer, EqualityComparer.Default)); + + _keys = source.Keys.ToArray(); + _values = source.Values.ToArray(); + } + + private protected override TKey[] KeysCore => _keys; + private protected override TValue[] ValuesCore => _values; + private protected override Enumerator GetEnumeratorCore() => new Enumerator(_keys, _values); + private protected override int CountCore => _keys.Length; + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private protected override ref readonly TValue GetValueRefOrNullRefCore(TKey key) + { + TKey[] keys = _keys; + for (int i = 0; i < keys.Length; i++) + { + if (EqualityComparer.Default.Equals(keys[i], key)) + { + return ref _values[i]; + } + } + + return ref Unsafe.NullRef(); + } + } +} diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallValueTypeDefaultComparerFrozenSet.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallValueTypeDefaultComparerFrozenSet.cs new file mode 100644 index 0000000000000..58931454e1c27 --- /dev/null +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallValueTypeDefaultComparerFrozenSet.cs @@ -0,0 +1,56 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Diagnostics; +using System.Linq; + +namespace System.Collections.Frozen +{ + /// Provides a frozen set to use when the item is a value type, the default comparer is used, and the item count is small. + internal sealed class SmallValueTypeDefaultComparerFrozenSet : FrozenSetInternalBase.GSW> + { + private readonly T[] _items; + + internal SmallValueTypeDefaultComparerFrozenSet(HashSet source) : base(EqualityComparer.Default) + { + Debug.Assert(default(T) is not null); + Debug.Assert(typeof(T).IsValueType); + + Debug.Assert(source.Count != 0); + Debug.Assert(ReferenceEquals(source.Comparer, EqualityComparer.Default)); + + _items = source.ToArray(); + } + + private protected override T[] ItemsCore => _items; + private protected override Enumerator GetEnumeratorCore() => new Enumerator(_items); + private protected override int CountCore => _items.Length; + + private protected override int FindItemIndex(T item) + { + T[] items = _items; + for (int i = 0; i < items.Length; i++) + { + if (EqualityComparer.Default.Equals(item, items[i])) + { + return i; + } + } + + return -1; + } + + internal struct GSW : IGenericSpecializedWrapper + { + private SmallValueTypeDefaultComparerFrozenSet _set; + public void Store(FrozenSet set) => _set = (SmallValueTypeDefaultComparerFrozenSet)set; + + public int Count => _set.Count; + public IEqualityComparer Comparer => _set.Comparer; + public int FindItemIndex(T item) => _set.FindItemIndex(item); + public Enumerator GetEnumerator() => _set.GetEnumerator(); + } + } +} diff --git a/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryTests.cs b/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryTests.cs index 9d384683f5c97..2b3490e0f8407 100644 --- a/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryTests.cs +++ b/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryTests.cs @@ -359,10 +359,8 @@ public void IReadOnlyDictionary_Generic_Values_ContainsAllCorrectValues(int coun public abstract class FrozenDictionary_Generic_Tests_string_string : FrozenDictionary_Generic_Tests { - protected override KeyValuePair CreateT(int seed) - { - return new KeyValuePair(CreateTKey(seed), CreateTKey(seed + 500)); - } + protected override KeyValuePair CreateT(int seed) => + new KeyValuePair(CreateTKey(seed), CreateTKey(seed + 500)); protected override string CreateTKey(int seed) { @@ -458,10 +456,8 @@ public class FrozenDictionary_Generic_Tests_int_int_ReadingUnoptimized : FrozenD public class FrozenDictionary_Generic_Tests_SimpleClass_SimpleClass : FrozenDictionary_Generic_Tests { - protected override KeyValuePair CreateT(int seed) - { - return new KeyValuePair(CreateTKey(seed), CreateTValue(seed + 500)); - } + protected override KeyValuePair CreateT(int seed) => + new KeyValuePair(CreateTKey(seed), CreateTValue(seed + 500)); protected override SimpleClass CreateTKey(int seed) { @@ -486,10 +482,8 @@ public int CompareTo(SimpleClass? other) => public class FrozenDictionary_Generic_Tests_SimpleStruct_int : FrozenDictionary_Generic_Tests { - protected override KeyValuePair CreateT(int seed) - { - return new KeyValuePair(CreateTKey(seed), CreateTValue(seed + 500)); - } + protected override KeyValuePair CreateT(int seed) => + new KeyValuePair(CreateTKey(seed), CreateTValue(seed + 500)); protected override SimpleStruct CreateTKey(int seed) => new SimpleStruct { Value = seed + 1 }; @@ -500,6 +494,37 @@ protected override KeyValuePair CreateT(int seed) protected override bool AllowVeryLargeSizes => false; // hash code contention leads to longer running times } + public class FrozenDictionary_Generic_Tests_SimpleNonComparableStruct_int : FrozenDictionary_Generic_Tests + { + protected override KeyValuePair CreateT(int seed) => + new KeyValuePair(CreateTKey(seed), CreateTValue(seed + 500)); + + protected override SimpleNonComparableStruct CreateTKey(int seed) => new SimpleNonComparableStruct { Value = seed + 1 }; + + protected override int CreateTValue(int seed) => seed; + + protected override bool DefaultValueAllowed => true; + + protected override bool AllowVeryLargeSizes => false; // hash code contention leads to longer running times + } + + public class FrozenDictionary_Generic_Tests_ValueTupleSimpleNonComparableStruct_int : FrozenDictionary_Generic_Tests, int> + { + protected override KeyValuePair, int> CreateT(int seed) => + new KeyValuePair, int>(CreateTKey(seed), CreateTValue(seed + 500)); + + protected override ValueTuple CreateTKey(int seed) => + new ValueTuple( + new SimpleNonComparableStruct { Value = seed + 1 }, + new SimpleNonComparableStruct { Value = seed + 1 }); + + protected override int CreateTValue(int seed) => seed; + + protected override bool DefaultValueAllowed => true; + + protected override bool AllowVeryLargeSizes => false; // hash code contention leads to longer running times + } + public struct SimpleStruct : IEquatable, IComparable { public int Value { get; set; } @@ -514,6 +539,18 @@ public override bool Equals([NotNullWhen(true)] object? obj) => obj is SimpleStruct other && Equals(other); } + public struct SimpleNonComparableStruct : IEquatable + { + public int Value { get; set; } + + public bool Equals(SimpleNonComparableStruct other) => Value == other.Value; + + public override int GetHashCode() => 0; // to force hashcode contention in implementation + + public override bool Equals([NotNullWhen(true)] object? obj) => + obj is SimpleNonComparableStruct other && Equals(other); + } + public sealed class NonDefaultEqualityComparer : IEqualityComparer { public static NonDefaultEqualityComparer Instance { get; } = new(); diff --git a/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenSetTests.cs b/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenSetTests.cs index 48b9462cfc1b1..925a45edf44a4 100644 --- a/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenSetTests.cs +++ b/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenSetTests.cs @@ -266,8 +266,6 @@ public void ComparingWithOtherSets(int size) Assert.True(frozen.SetEquals(new HashSet(source, otherComparer))); Assert.True(frozen.SetEquals(FrozenSet.ToFrozenSet(source, otherComparer))); Assert.True(frozen.SetEquals(source.ToImmutableHashSet(otherComparer))); - Assert.True(frozen.SetEquals(new SortedSet(source))); - Assert.True(frozen.SetEquals(source.ToImmutableSortedSet())); Assert.False(frozen.SetEquals(new HashSet(InvertingComparer.Instance))); Assert.False(frozen.SetEquals(FrozenSet.ToFrozenSet(source, InvertingComparer.Instance))); Assert.False(frozen.SetEquals(source.ToImmutableHashSet(InvertingComparer.Instance))); @@ -280,8 +278,6 @@ public void ComparingWithOtherSets(int size) Assert.True(frozen.IsSubsetOf(new HashSet(source, otherComparer))); Assert.True(frozen.IsSubsetOf(FrozenSet.ToFrozenSet(source, otherComparer))); Assert.True(frozen.IsSubsetOf(source.ToImmutableHashSet(otherComparer))); - Assert.True(frozen.IsSubsetOf(new SortedSet(source))); - Assert.True(frozen.IsSubsetOf(source.ToImmutableSortedSet())); Assert.False(frozen.IsSubsetOf(new HashSet(InvertingComparer.Instance))); Assert.False(frozen.IsSubsetOf(FrozenSet.ToFrozenSet(source, InvertingComparer.Instance))); Assert.False(frozen.IsSubsetOf(source.ToImmutableHashSet(InvertingComparer.Instance))); @@ -294,8 +290,6 @@ public void ComparingWithOtherSets(int size) Assert.True(frozen.IsSupersetOf(new HashSet(source, otherComparer))); Assert.True(frozen.IsSupersetOf(FrozenSet.ToFrozenSet(source, otherComparer))); Assert.True(frozen.IsSupersetOf(source.ToImmutableHashSet(otherComparer))); - Assert.True(frozen.IsSupersetOf(new SortedSet(source))); - Assert.True(frozen.IsSupersetOf(source.ToImmutableSortedSet())); Assert.True(frozen.IsSupersetOf(new HashSet(InvertingComparer.Instance))); Assert.True(frozen.IsSupersetOf(FrozenSet.ToFrozenSet(source, InvertingComparer.Instance))); Assert.True(frozen.IsSupersetOf(source.ToImmutableHashSet(InvertingComparer.Instance))); @@ -308,8 +302,6 @@ public void ComparingWithOtherSets(int size) Assert.False(frozen.IsProperSubsetOf(new HashSet(source, otherComparer))); Assert.False(frozen.IsProperSubsetOf(FrozenSet.ToFrozenSet(source, otherComparer))); Assert.False(frozen.IsProperSubsetOf(source.ToImmutableHashSet(otherComparer))); - Assert.False(frozen.IsProperSubsetOf(new SortedSet(source))); - Assert.False(frozen.IsProperSubsetOf(source.ToImmutableSortedSet())); Assert.False(frozen.IsProperSubsetOf(new HashSet(InvertingComparer.Instance))); Assert.False(frozen.IsProperSubsetOf(FrozenSet.ToFrozenSet(source, InvertingComparer.Instance))); Assert.False(frozen.IsProperSubsetOf(source.ToImmutableHashSet(InvertingComparer.Instance))); @@ -323,8 +315,6 @@ public void ComparingWithOtherSets(int size) Assert.False(frozen.IsProperSupersetOf(new HashSet(source, otherComparer))); Assert.False(frozen.IsProperSupersetOf(FrozenSet.ToFrozenSet(source, otherComparer))); Assert.False(frozen.IsProperSupersetOf(source.ToImmutableHashSet(otherComparer))); - Assert.False(frozen.IsProperSupersetOf(new SortedSet(source))); - Assert.False(frozen.IsProperSupersetOf(source.ToImmutableSortedSet())); Assert.True(frozen.IsProperSupersetOf(new HashSet(InvertingComparer.Instance))); Assert.True(frozen.IsProperSupersetOf(FrozenSet.ToFrozenSet(source, InvertingComparer.Instance))); Assert.True(frozen.IsProperSupersetOf(source.ToImmutableHashSet(InvertingComparer.Instance))); @@ -466,6 +456,23 @@ public class FrozenSet_Generic_Tests_SimpleStruct : FrozenSet_Generic_Tests false; // hash code contention leads to longer running times } + public class FrozenSet_Generic_Tests_SimpleNonComparableStruct : FrozenSet_Generic_Tests + { + protected override SimpleNonComparableStruct CreateT(int seed) => new SimpleNonComparableStruct { Value = seed + 1 }; + + protected override bool TestLargeSizes => false; // hash code contention leads to longer running times + } + + public class FrozenSet_Generic_Tests_ValueTupleSimpleNonComparableStruct : FrozenSet_Generic_Tests> + { + protected override ValueTuple CreateT(int seed) => + new ValueTuple( + new SimpleNonComparableStruct { Value = seed + 1 }, + new SimpleNonComparableStruct { Value = seed + 1 }); + + protected override bool TestLargeSizes => false; // hash code contention leads to longer running times + } + public class FrozenSet_NonGeneric_Tests : ICollection_NonGeneric_Tests { protected override ICollection NonGenericICollectionFactory() =>