-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix FrozenDictionary/Set handling of ValueTuple keys (#84280)
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<ValueTuple<...>>`, 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).
- Loading branch information
1 parent
149e43e
commit e579ccb
Showing
12 changed files
with
189 additions
and
190 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
73 changes: 0 additions & 73 deletions
73
...tions.Immutable/src/System/Collections/Frozen/SmallComparableValueTypeFrozenDictionary.cs
This file was deleted.
Oops, something went wrong.
79 changes: 0 additions & 79 deletions
79
....Collections.Immutable/src/System/Collections/Frozen/SmallComparableValueTypeFrozenSet.cs
This file was deleted.
Oops, something went wrong.
50 changes: 50 additions & 0 deletions
50
....Immutable/src/System/Collections/Frozen/SmallValueTypeDefaultComparerFrozenDictionary.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| { | ||
| /// <summary>Provides a frozen dictionary to use when the key is a value type, the default comparer is used, and the item count is small.</summary> | ||
| internal sealed class SmallValueTypeDefaultComparerFrozenDictionary<TKey, TValue> : FrozenDictionary<TKey, TValue> | ||
| where TKey : notnull | ||
| { | ||
| private readonly TKey[] _keys; | ||
| private readonly TValue[] _values; | ||
|
|
||
| internal SmallValueTypeDefaultComparerFrozenDictionary(Dictionary<TKey, TValue> source) : base(EqualityComparer<TKey>.Default) | ||
| { | ||
| Debug.Assert(default(TKey) is not null); | ||
| Debug.Assert(typeof(TKey).IsValueType); | ||
|
|
||
| Debug.Assert(source.Count != 0); | ||
| Debug.Assert(ReferenceEquals(source.Comparer, EqualityComparer<TKey>.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<TKey>.Default.Equals(keys[i], key)) | ||
| { | ||
| return ref _values[i]; | ||
| } | ||
| } | ||
|
|
||
| return ref Unsafe.NullRef<TValue>(); | ||
| } | ||
| } | ||
| } |
56 changes: 56 additions & 0 deletions
56
...ections.Immutable/src/System/Collections/Frozen/SmallValueTypeDefaultComparerFrozenSet.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| { | ||
| /// <summary>Provides a frozen set to use when the item is a value type, the default comparer is used, and the item count is small.</summary> | ||
| internal sealed class SmallValueTypeDefaultComparerFrozenSet<T> : FrozenSetInternalBase<T, SmallValueTypeDefaultComparerFrozenSet<T>.GSW> | ||
| { | ||
| private readonly T[] _items; | ||
|
|
||
| internal SmallValueTypeDefaultComparerFrozenSet(HashSet<T> source) : base(EqualityComparer<T>.Default) | ||
| { | ||
| Debug.Assert(default(T) is not null); | ||
| Debug.Assert(typeof(T).IsValueType); | ||
|
|
||
| Debug.Assert(source.Count != 0); | ||
| Debug.Assert(ReferenceEquals(source.Comparer, EqualityComparer<T>.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<T>.Default.Equals(item, items[i])) | ||
| { | ||
| return i; | ||
| } | ||
| } | ||
|
|
||
| return -1; | ||
| } | ||
|
|
||
| internal struct GSW : IGenericSpecializedWrapper | ||
| { | ||
| private SmallValueTypeDefaultComparerFrozenSet<T> _set; | ||
| public void Store(FrozenSet<T> set) => _set = (SmallValueTypeDefaultComparerFrozenSet<T>)set; | ||
|
|
||
| public int Count => _set.Count; | ||
| public IEqualityComparer<T> Comparer => _set.Comparer; | ||
| public int FindItemIndex(T item) => _set.FindItemIndex(item); | ||
| public Enumerator GetEnumerator() => _set.GetEnumerator(); | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.