Skip to content

Fix missing comparer when creating FrozenDictionary #83651

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 19, 2023
Merged

Fix missing comparer when creating FrozenDictionary #83651

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ....Collections.Immutable/src/System/Collections/Frozen/WrappedDictionaryFrozenDictionary.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ internal sealed class WrappedDictionaryFrozenDictionary<TKey, TValue> :
private TValue[]? _values;

internal WrappedDictionaryFrozenDictionary(Dictionary<TKey, TValue> source, bool sourceIsCopy) : base(source.Comparer) =>
_source = sourceIsCopy ? source : new Dictionary<TKey, TValue>(source);
_source = sourceIsCopy ? source : new Dictionary<TKey, TValue>(source, source.Comparer);

/// <inheritdoc />
private protected sealed override TKey[] KeysCore =>
Expand Down
127 changes: 75 additions & 52 deletions src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,21 @@ public abstract class FrozenDictionary_Generic_Tests<TKey, TValue> : IDictionary
protected override bool Enumerator_Current_UndefinedOperation_Throws => true;
protected override Type ICollection_Generic_CopyTo_IndexLargerThanArrayCount_ThrowType => typeof(ArgumentOutOfRangeException);

protected override IDictionary<TKey, TValue> GenericIDictionaryFactory(int count) =>
GenericIDictionaryFactory(count, optimizeForReading: true);
public virtual bool OptimizeForReading => true;

protected virtual bool AllowVeryLargeSizes => true;

protected virtual IDictionary<TKey, TValue> GenericIDictionaryFactory(int count, bool optimizeForReading)
public virtual TKey GetEqualKey(TKey key) => key;

protected override IDictionary<TKey, TValue> GenericIDictionaryFactory(int count)
{
var d = new Dictionary<TKey, TValue>();
for (int i = 0; i < count; i++)
{
d.Add(CreateTKey(i), CreateTValue(i));
}
return optimizeForReading ?
d.ToFrozenDictionary(GetKeyIEqualityComparer(), true) :
return OptimizeForReading ?
d.ToFrozenDictionary(GetKeyIEqualityComparer(), optimizeForReading: true) :
d.ToFrozenDictionary(GetKeyIEqualityComparer());
}

Expand All @@ -46,13 +47,12 @@ protected virtual IDictionary<TKey, TValue> GenericIDictionaryFactory(int count,
protected override EnumerableOrder Order => EnumerableOrder.Unspecified;

[Theory]
[InlineData(100_000, false)]
[InlineData(100_000, true)]
public virtual void CreateVeryLargeDictionary_Success(int largeCount, bool optimizeForReading)
[InlineData(100_000)]
public virtual void CreateVeryLargeDictionary_Success(int largeCount)
{
if (AllowVeryLargeSizes)
{
GenericIDictionaryFactory(largeCount, optimizeForReading);
GenericIDictionaryFactory(largeCount);
}
}

Expand Down Expand Up @@ -87,26 +87,22 @@ public void EmptySource_ProducedFrozenDictionaryEmpty()
Assert.Same(FrozenDictionary<TKey, TValue>.Empty, Enumerable.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(comparer));
Assert.Same(FrozenDictionary<TKey, TValue>.Empty, Array.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(comparer));
Assert.Same(FrozenDictionary<TKey, TValue>.Empty, new List<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(comparer));
foreach (bool optimizeForReading in new[] { false, true })
{
Assert.Same(FrozenDictionary<TKey, TValue>.Empty, new Dictionary<TKey, TValue>().ToFrozenDictionary(comparer, optimizeForReading));
Assert.Same(FrozenDictionary<TKey, TValue>.Empty, Enumerable.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(comparer));
Assert.Same(FrozenDictionary<TKey, TValue>.Empty, Array.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(comparer));
Assert.Same(FrozenDictionary<TKey, TValue>.Empty, new List<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(comparer));
}

Assert.Same(FrozenDictionary<TKey, TValue>.Empty, new Dictionary<TKey, TValue>().ToFrozenDictionary(comparer, OptimizeForReading));
Assert.Same(FrozenDictionary<TKey, TValue>.Empty, Enumerable.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(comparer, OptimizeForReading));
Assert.Same(FrozenDictionary<TKey, TValue>.Empty, Array.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(comparer, OptimizeForReading));
Assert.Same(FrozenDictionary<TKey, TValue>.Empty, new List<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(comparer, OptimizeForReading));
}

Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, new Dictionary<TKey, TValue>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance));
Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, Enumerable.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance));
Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, Array.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance));
Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, new List<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance));
foreach (bool optimizeForReading in new[] { false, true })
{
Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, new Dictionary<TKey, TValue>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance, optimizeForReading));
Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, Enumerable.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance, optimizeForReading));
Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, Array.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance, optimizeForReading));
Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, new List<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance, optimizeForReading));
}

Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, new Dictionary<TKey, TValue>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance, OptimizeForReading));
Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, Enumerable.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance, OptimizeForReading));
Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, Array.Empty<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance, OptimizeForReading));
Assert.NotSame(FrozenDictionary<TKey, TValue>.Empty, new List<KeyValuePair<TKey, TValue>>().ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance, OptimizeForReading));
}

[Fact]
Expand Down Expand Up @@ -170,14 +166,12 @@ public void FrozenDictionary_ToFrozenDictionary_Idempotent()
Assert.NotSame(frozen, frozen.ToFrozenDictionary(NonDefaultEqualityComparer<TKey>.Instance));
}

[Theory]
[InlineData(false)]
[InlineData(true)]
public void ToFrozenDictionary_BoolArg_UsesDefaultComparer(bool optimizeForReading)
[Fact]
public void ToFrozenDictionary_BoolArg_UsesDefaultComparer()
{
Dictionary<TKey, TValue> source = Enumerable.Range(0, 4).ToDictionary(CreateTKey, CreateTValue);

FrozenDictionary<TKey, TValue> frozen1 = source.ToFrozenDictionary(optimizeForReading);
FrozenDictionary<TKey, TValue> frozen1 = source.ToFrozenDictionary(OptimizeForReading);

Assert.Same(EqualityComparer<TKey>.Default, frozen1.Comparer);
}
Expand Down Expand Up @@ -215,20 +209,19 @@ public static IEnumerable<object[]> LookupItems_AllItemsFoundAsExpected_MemberDa
from size in new[] { 0, 1, 2, 10, 999, 1024 }
from comparer in new IEqualityComparer<TKey>[] { null, EqualityComparer<TKey>.Default, NonDefaultEqualityComparer<TKey>.Instance }
from specifySameComparer in new[] { false, true }
from optimizeForReading in new[] { false, true }
select new object[] { size, comparer, specifySameComparer, optimizeForReading };
select new object[] { size, comparer, specifySameComparer };

[Theory]
[MemberData(nameof(LookupItems_AllItemsFoundAsExpected_MemberData))]
public void LookupItems_AllItemsFoundAsExpected(int size, IEqualityComparer<TKey> comparer, bool specifySameComparer, bool optimizeForReading)
public void LookupItems_AllItemsFoundAsExpected(int size, IEqualityComparer<TKey> comparer, bool specifySameComparer)
{
Dictionary<TKey, TValue> original =
Enumerable.Range(0, size)
.Select(i => new KeyValuePair<TKey, TValue>(CreateTKey(i), CreateTValue(i)))
.ToDictionary(p => p.Key, p => p.Value, comparer);
KeyValuePair<TKey, TValue>[] originalPairs = original.ToArray();

FrozenDictionary<TKey, TValue> frozen = (specifySameComparer, optimizeForReading) switch
FrozenDictionary<TKey, TValue> frozen = (specifySameComparer, OptimizeForReading) switch
{
(false, false) => original.ToFrozenDictionary(),
(false, true) => original.ToFrozenDictionary(null, true),
Expand Down Expand Up @@ -275,6 +268,31 @@ comparer is null ||
}
}

[Theory]
[InlineData(false)]
[InlineData(true)]
public void EqualButPossiblyDifferentKeys_Found(bool fromDictionary)
{
Dictionary<TKey, TValue> original =
Enumerable.Range(0, 50)
.Select(i => new KeyValuePair<TKey, TValue>(CreateTKey(i), CreateTValue(i)))
.ToDictionary(p => p.Key, p => p.Value, GetKeyIEqualityComparer());

FrozenDictionary<TKey, TValue> frozen = fromDictionary ?
original.ToFrozenDictionary(GetKeyIEqualityComparer()) :
original.Select(k => k).ToFrozenDictionary(GetKeyIEqualityComparer());

foreach (TKey key in original.Keys)
{
Assert.True(original.ContainsKey(key));
Assert.True(frozen.ContainsKey(key));

TKey equalKey = GetEqualKey(key);
Assert.True(original.ContainsKey(equalKey));
Assert.True(frozen.ContainsKey(equalKey));
}
}

[Fact]
public void MultipleValuesSameKey_LastInSourceWins()
{
Expand All @@ -301,15 +319,12 @@ from value in values
}

[Theory]
[InlineData(0, false)]
[InlineData(1, false)]
[InlineData(75, false)]
[InlineData(0, true)]
[InlineData(1, true)]
[InlineData(75, true)]
public void IReadOnlyDictionary_Generic_Keys_ContainsAllCorrectKeys(int count, bool optimizeForReading)
[InlineData(0)]
[InlineData(1)]
[InlineData(75)]
public void IReadOnlyDictionary_Generic_Keys_ContainsAllCorrectKeys(int count)
{
IDictionary<TKey, TValue> dictionary = GenericIDictionaryFactory(count, optimizeForReading);
IDictionary<TKey, TValue> dictionary = GenericIDictionaryFactory(count);
IEnumerable<TKey> expected = dictionary.Select((pair) => pair.Key);

IReadOnlyDictionary<TKey, TValue> rod = (IReadOnlyDictionary<TKey, TValue>)dictionary;
Expand All @@ -318,15 +333,12 @@ public void IReadOnlyDictionary_Generic_Keys_ContainsAllCorrectKeys(int count, b
}

[Theory]
[InlineData(0, false)]
[InlineData(1, false)]
[InlineData(75, false)]
[InlineData(0, true)]
[InlineData(1, true)]
[InlineData(75, true)]
public void IReadOnlyDictionary_Generic_Values_ContainsAllCorrectValues(int count, bool optimizeForReading)
[InlineData(0)]
[InlineData(1)]
[InlineData(75)]
public void IReadOnlyDictionary_Generic_Values_ContainsAllCorrectValues(int count)
{
IDictionary<TKey, TValue> dictionary = GenericIDictionaryFactory(count, optimizeForReading);
IDictionary<TKey, TValue> dictionary = GenericIDictionaryFactory(count);
IEnumerable<TValue> expected = dictionary.Select((pair) => pair.Value);

IReadOnlyDictionary<TKey, TValue> rod = (IReadOnlyDictionary<TKey, TValue>)dictionary;
Expand Down Expand Up @@ -374,9 +386,16 @@ public class FrozenDictionary_Generic_Tests_string_string_Ordinal : FrozenDictio
public override IEqualityComparer<string> GetKeyIEqualityComparer() => StringComparer.Ordinal;
}

public class FrozenDictionary_Generic_Tests_string_string_OrdinalIgnoreCase_ReadingUnoptimized : FrozenDictionary_Generic_Tests_string_string_OrdinalIgnoreCase
{
public override bool OptimizeForReading => false;
}

public class FrozenDictionary_Generic_Tests_string_string_OrdinalIgnoreCase : FrozenDictionary_Generic_Tests_string_string
{
public override IEqualityComparer<string> GetKeyIEqualityComparer() => StringComparer.OrdinalIgnoreCase;

public override string GetEqualKey(string key) => key.ToLowerInvariant();
}

public class FrozenDictionary_Generic_Tests_string_string_NonDefault : FrozenDictionary_Generic_Tests_string_string
Expand Down Expand Up @@ -407,13 +426,12 @@ protected override ulong CreateTKey(int seed)

[OuterLoop("Takes several seconds")]
[Theory]
[InlineData(8_000_000, false)]
[InlineData(8_000_000, true)]
public void CreateHugeDictionary_Success(int largeCount, bool optimizeForReading)
[InlineData(8_000_000)]
public void CreateHugeDictionary_Success(int largeCount)
{
if (AllowVeryLargeSizes)
{
GenericIDictionaryFactory(largeCount, optimizeForReading);
GenericIDictionaryFactory(largeCount);
}
}
}
Expand All @@ -433,6 +451,11 @@ protected override KeyValuePair<int, int> CreateT(int seed)
protected override int CreateTValue(int seed) => CreateTKey(seed);
}

public class FrozenDictionary_Generic_Tests_int_int_ReadingUnoptimized : FrozenDictionary_Generic_Tests_int_int
{
public override bool OptimizeForReading => false;
}

public class FrozenDictionary_Generic_Tests_SimpleClass_SimpleClass : FrozenDictionary_Generic_Tests<SimpleClass, SimpleClass>
{
protected override KeyValuePair<SimpleClass, SimpleClass> CreateT(int seed)
Expand Down
Loading