Skip to content

Commit

Permalink
Reduce private points of contact between frozen colelctions and Immut…
Browse files Browse the repository at this point in the history
…ableArray<T>. (#78675)

This change is intended to make it easier to use the frozen collections in environments/conditions
when only the public API of ImmutableArray is available. There are two things which are changed:

1. Uses of the internal ImmutableArray ctor are replaced by the one-line ImmutableArrayFactory.Create
stub, which is easy to replace in above mentioned environments/conditions.

2. Uses of the internal ImmutableArray.array property are trivially eliminated by using the public API
surface.

Co-authored-by: Martin Taillefer <mataille@microsoft.com>
  • Loading branch information
geeknoid and Martin Taillefer authored Nov 22, 2022
1 parent fe6eb87 commit dc1efbf
Show file tree
Hide file tree
Showing 14 changed files with 46 additions and 25 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<IsPackable>true</IsPackable>
Expand All @@ -24,6 +24,7 @@ The System.Collections.Immutable library is built-in as part of the shared frame
<Compile Include="System\Collections\Frozen\FrozenHashTable.cs" />
<Compile Include="System\Collections\Frozen\FrozenSet.cs" />
<Compile Include="System\Collections\Frozen\FrozenSetInternalBase.cs" />
<Compile Include="System\Collections\Frozen\ImmutableArrayFactory.cs" />
<Compile Include="System\Collections\Frozen\Int32FrozenDictionary.cs" />
<Compile Include="System\Collections\Frozen\Int32FrozenSet.cs" />
<Compile Include="System\Collections\Frozen\ItemsFrozenSet.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ internal sealed class EmptyFrozenDictionary<TKey, TValue> : FrozenDictionary<TKe
internal EmptyFrozenDictionary() : base(EqualityComparer<TKey>.Default) { }

/// <inheritdoc />
private protected override ImmutableArray<TKey> KeysCore => ImmutableArray<TKey>.Empty;
private protected override TKey[] KeysCore => Array.Empty<TKey>();

/// <inheritdoc />
private protected override ImmutableArray<TValue> ValuesCore => ImmutableArray<TValue>.Empty;
private protected override TValue[] ValuesCore => Array.Empty<TValue>();

/// <inheritdoc />
private protected override Enumerator GetEnumeratorCore() => new Enumerator(Array.Empty<TKey>(), Array.Empty<TValue>());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ internal sealed class EmptyFrozenSet<T> : FrozenSet<T>
internal EmptyFrozenSet() : base(EqualityComparer<T>.Default) { }

/// <inheritdoc />
private protected override ImmutableArray<T> ItemsCore => ImmutableArray<T>.Empty;
private protected override T[] ItemsCore => Array.Empty<T>();

/// <inheritdoc />
private protected override int CountCore => 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,10 @@ public abstract class FrozenDictionary<TKey, TValue> : IDictionary<TKey, TValue>
/// <remarks>
/// The order of the keys in the dictionary is unspecified, but it is the same order as the associated values returned by the <see cref="Values"/> property.
/// </remarks>
public ImmutableArray<TKey> Keys => KeysCore;
public ImmutableArray<TKey> Keys => ImmutableArrayFactory.Create(KeysCore);

/// <inheritdoc cref="Keys" />
private protected abstract ImmutableArray<TKey> KeysCore { get; }
private protected abstract TKey[] KeysCore { get; }

/// <inheritdoc />
ICollection<TKey> IDictionary<TKey, TValue>.Keys =>
Expand All @@ -191,10 +191,10 @@ public abstract class FrozenDictionary<TKey, TValue> : IDictionary<TKey, TValue>
/// <remarks>
/// The order of the values in the dictionary is unspecified, but it is the same order as the associated keys returned by the <see cref="Keys"/> property.
/// </remarks>
public ImmutableArray<TValue> Values => ValuesCore;
public ImmutableArray<TValue> Values => ImmutableArrayFactory.Create(ValuesCore);

/// <inheritdoc cref="Values" />
private protected abstract ImmutableArray<TValue> ValuesCore { get; }
private protected abstract TValue[] ValuesCore { get; }

ICollection<TValue> IDictionary<TKey, TValue>.Values =>
Values is { Length: > 0 } values ? values : Array.Empty<TValue>();
Expand Down Expand Up @@ -230,8 +230,8 @@ public void CopyTo(Span<KeyValuePair<TKey, TValue>> destination)
ThrowHelper.ThrowIfDestinationTooSmall();
}

TKey[] keys = Keys.array!;
TValue[] values = Values.array!;
TKey[] keys = KeysCore;
TValue[] values = ValuesCore;
Debug.Assert(keys.Length == values.Length);

for (int i = 0; i < keys.Length; i++)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,10 @@ public abstract class FrozenSet<T> : ISet<T>,

/// <summary>Gets a collection containing the values in the set.</summary>
/// <remarks>The order of the values in the set is unspecified.</remarks>
public ImmutableArray<T> Items => ItemsCore;
public ImmutableArray<T> Items => ImmutableArrayFactory.Create(ItemsCore);

/// <inheritdoc cref="Items" />
private protected abstract ImmutableArray<T> ItemsCore { get; }
private protected abstract T[] ItemsCore { get; }

/// <summary>Gets the number of values contained in the set.</summary>
public int Count => CountCore;
Expand Down Expand Up @@ -160,7 +160,8 @@ void ICollection.CopyTo(Array array, int index)
throw new ArgumentException(SR.Arg_RankMultiDimNotSupported, nameof(array));
}

Array.Copy(Items.array!, 0, array!, index, Items.Length);
T[] items = ItemsCore;
Array.Copy(items, 0, array!, index, items.Length);
}

/// <inheritdoc />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// 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.Immutable;

namespace System.Collections.Frozen
{
/// <summary>
/// Stubs to isolate the frozen collection code from the internal details of ImmutableArray
/// </summary>
/// <remarks>
/// This is intended to make it easier to use the frozen collections in environments/conditions
/// when only the public API of ImmutableArray is available.
/// </remarks>
internal static class ImmutableArrayFactory
{
public static ImmutableArray<T> Create<T>(T[] array) => new ImmutableArray<T>(array);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ internal Int32FrozenDictionary(Dictionary<int, TValue> source) : base(EqualityCo
}

/// <inheritdoc />
private protected override ImmutableArray<int> KeysCore => new ImmutableArray<int>(_hashTable.HashCodes);
private protected override int[] KeysCore => _hashTable.HashCodes;

/// <inheritdoc />
private protected override ImmutableArray<TValue> ValuesCore => new ImmutableArray<TValue>(_values);
private protected override TValue[] ValuesCore => _values;

/// <inheritdoc />
private protected override Enumerator GetEnumeratorCore() => new Enumerator(_hashTable.HashCodes, _values);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal Int32FrozenSet(HashSet<int> source) : base(EqualityComparer<int>.Defaul
}

/// <inheritdoc />
private protected override ImmutableArray<int> ItemsCore => new ImmutableArray<int>(_hashTable.HashCodes);
private protected override int[] ItemsCore => _hashTable.HashCodes;

/// <inheritdoc />
private protected override Enumerator GetEnumeratorCore() => new Enumerator(_hashTable.HashCodes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ protected ItemsFrozenSet(HashSet<T> source, IEqualityComparer<T> comparer) : bas
}

/// <inheritdoc />
private protected sealed override ImmutableArray<T> ItemsCore => new ImmutableArray<T>(_items);
private protected sealed override T[] ItemsCore => _items;

/// <inheritdoc />
private protected sealed override Enumerator GetEnumeratorCore() => new Enumerator(_items);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ protected KeysAndValuesFrozenDictionary(Dictionary<TKey, TValue> source, IEquali
}

/// <inheritdoc />
private protected sealed override ImmutableArray<TKey> KeysCore => new ImmutableArray<TKey>(_keys);
private protected sealed override TKey[] KeysCore => _keys;

/// <inheritdoc />
private protected sealed override ImmutableArray<TValue> ValuesCore => new ImmutableArray<TValue>(_values);
private protected sealed override TValue[] ValuesCore => _values;

/// <inheritdoc />
private protected sealed override Enumerator GetEnumeratorCore() => new Enumerator(_keys, _values);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,10 @@ private LengthBucketsFrozenDictionary(
}

/// <inheritdoc />
private protected override ImmutableArray<string> KeysCore => new ImmutableArray<string>(_keys);
private protected override string[] KeysCore => _keys;

/// <inheritdoc />
private protected override ImmutableArray<TValue> ValuesCore => new ImmutableArray<TValue>(_values);
private protected override TValue[] ValuesCore => _values;

/// <inheritdoc />
private protected override Enumerator GetEnumeratorCore() => new Enumerator(_keys, _values);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ private LengthBucketsFrozenSet(string[] items, KeyValuePair<string, int>[][] len
}

/// <inheritdoc />
private protected override ImmutableArray<string> ItemsCore => new ImmutableArray<string>(_items);
private protected override string[] ItemsCore => _items;

/// <inheritdoc />
private protected override Enumerator GetEnumeratorCore() => new Enumerator(_items);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ internal OrdinalStringFrozenDictionary(Dictionary<string, TValue> source, IEqual
}

/// <inheritdoc />
private protected override ImmutableArray<string> KeysCore => new ImmutableArray<string>(_keys);
private protected override string[] KeysCore => _keys;

/// <inheritdoc />
private protected override ImmutableArray<TValue> ValuesCore => new ImmutableArray<TValue>(_values);
private protected override TValue[] ValuesCore => _values;

/// <inheritdoc />
private protected override Enumerator GetEnumeratorCore() => new Enumerator(_keys, _values);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ internal OrdinalStringFrozenSet(HashSet<string> source, IEqualityComparer<string
}

/// <inheritdoc />
private protected override ImmutableArray<string> ItemsCore => new ImmutableArray<string>(_items);
private protected override string[] ItemsCore => _items;

/// <inheritdoc />
private protected override Enumerator GetEnumeratorCore() => new Enumerator(_items);
Expand Down

0 comments on commit dc1efbf

Please sign in to comment.