Skip to content

Commit

Permalink
Update ICollection<T> usage to IReadOnlyCollection<T> where applicable (
Browse files Browse the repository at this point in the history
dotnet#101469)

Anywhere we're casting to `ICollection<T>` just to use its `Count`, we can instead now cast to `IReadOnlyCollection<T>`, as the former inherits the latter as of .NET 9. This expands the set of types that can light-up with the check; in a couple of places it also lets us remove what would then be a duplicative check. We can do the same for `IList<T>` and `IReadOnlyList<T>`.

While dispatch via the DIM could result in an extra interface dispatch for types that weren't previously implementing the read-only interface, a) our collection types already implement both, and b) these cases all represent fast paths where the extra savings from the faster path should more than make up for additional call overheads. I audited for anywhere we were missing explicit implementations and added a few corner-cases in.
  • Loading branch information
stephentoub authored and michaelgsharp committed May 8, 2024
1 parent 717e940 commit ec48347
Show file tree
Hide file tree
Showing 25 changed files with 121 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,22 @@ internal static bool CompareTags(List<KeyValuePair<string, object?>>? sortedTags
int size = count / (sizeof(ulong) * 8) + 1;
BitMapper bitMapper = new BitMapper(size <= 100 ? stackalloc ulong[size] : new ulong[size]);

#if NET9_0_OR_GREATER // ICollection<T> : IReadOnlyCollection<T> on .NET 9+
if (tags2 is IReadOnlyCollection<KeyValuePair<string, object?>> tagsCol)
#else
if (tags2 is ICollection<KeyValuePair<string, object?>> tagsCol)
#endif
{
if (tagsCol.Count != count)
{
return false;
}

#if NET9_0_OR_GREATER // IList<T> : IReadOnlyList<T> on .NET 9+
if (tagsCol is IReadOnlyList<KeyValuePair<string, object?>> secondList)
#else
if (tagsCol is IList<KeyValuePair<string, object?>> secondList)
#endif
{
for (int i = 0; i < count; i++)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ private protected override bool IsProperSubsetOfCore(IEnumerable<T> other)
{
Debug.Assert(_thisSet.Count != 0, "EmptyFrozenSet should have been used.");

#if NET9_0_OR_GREATER // ICollection<T> : IReadOnlyCollection<T> on .NET 9+
if (other is IReadOnlyCollection<T> otherAsCollection)
#else
if (other is ICollection<T> otherAsCollection)
#endif
{
int otherCount = otherAsCollection.Count;

Expand All @@ -59,7 +63,11 @@ private protected override bool IsProperSupersetOfCore(IEnumerable<T> other)
{
Debug.Assert(_thisSet.Count != 0, "EmptyFrozenSet should have been used.");

#if NET9_0_OR_GREATER // ICollection<T> : IReadOnlyCollection<T> on .NET 9+
if (other is IReadOnlyCollection<T> otherAsCollection)
#else
if (other is ICollection<T> otherAsCollection)
#endif
{
int otherCount = otherAsCollection.Count;

Expand Down Expand Up @@ -103,7 +111,11 @@ private protected override bool IsSupersetOfCore(IEnumerable<T> other)
Debug.Assert(_thisSet.Count != 0, "EmptyFrozenSet should have been used.");

// Try to compute the answer based purely on counts.
#if NET9_0_OR_GREATER // ICollection<T> : IReadOnlyCollection<T> on .NET 9+
if (other is IReadOnlyCollection<T> otherAsCollection)
#else
if (other is ICollection<T> otherAsCollection)
#endif
{
int otherCount = otherAsCollection.Count;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,13 @@ internal static bool TryGetCount<T>(this IEnumerable sequence, out int count)
return true;
}

#if !NET9_0_OR_GREATER // ICollection<T> : IReadOnlyCollection<T> on .NET 9+
if (sequence is ICollection<T> collectionOfT)
{
count = collectionOfT.Count;
return true;
}
#endif

if (sequence is IReadOnlyCollection<T> readOnlyCollection)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ public void EnqueueRange(IEnumerable<TElement> elements, TPriority priority)
ArgumentNullException.ThrowIfNull(elements);

int count;
if (elements is ICollection<TElement> collection &&
if (elements is IReadOnlyCollection<TElement> collection &&
(count = collection.Count) > _nodes.Length - _size)
{
Grow(checked(_size + count));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1323,7 +1323,7 @@ public bool Overlaps(IEnumerable<T> other)
if (Count == 0)
return false;

if (other is ICollection<T> c && c.Count == 0)
if (other is IReadOnlyCollection<T> c && c.Count == 0)
return false;

SortedSet<T>? asSorted = other as SortedSet<T>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace System.Dynamic
/// <summary>
/// Represents an object with members that can be dynamically added and removed at runtime.
/// </summary>
public sealed class ExpandoObject : IDynamicMetaObjectProvider, IDictionary<string, object?>, INotifyPropertyChanged
public sealed class ExpandoObject : IDynamicMetaObjectProvider, IDictionary<string, object?>, IReadOnlyDictionary<string, object?>, INotifyPropertyChanged
{
private static readonly MethodInfo s_expandoTryGetValue =
typeof(RuntimeOps).GetMethod(nameof(RuntimeOps.ExpandoTryGetValue))!;
Expand Down Expand Up @@ -618,6 +618,10 @@ System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()

ICollection<object?> IDictionary<string, object?>.Values => new ValueCollection(this);

IEnumerable<string> IReadOnlyDictionary<string, object?>.Keys => new KeyCollection(this);

IEnumerable<object?> IReadOnlyDictionary<string, object?>.Values => new ValueCollection(this);

object? IDictionary<string, object?>.this[string key]
{
get
Expand All @@ -636,6 +640,18 @@ System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
}
}

object? IReadOnlyDictionary<string, object?>.this[string key]
{
get
{
if (!TryGetValueForKey(key, out object? value))
{
throw System.Linq.Expressions.Error.KeyDoesNotExistInExpando(key);
}
return value;
}
}

void IDictionary<string, object?>.Add(string key, object? value)
{
this.TryAddMember(key, value);
Expand All @@ -650,6 +666,15 @@ System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
return index >= 0 && data[index] != Uninitialized;
}

bool IReadOnlyDictionary<string, object?>.ContainsKey(string key)
{
ArgumentNullException.ThrowIfNull(key);

ExpandoData data = _data;
int index = data.Class.GetValueIndexCaseSensitive(key);
return index >= 0 && data[index] != Uninitialized;
}

bool IDictionary<string, object?>.Remove(string key)
{
ArgumentNullException.ThrowIfNull(key);
Expand All @@ -662,6 +687,11 @@ System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
return TryGetValueForKey(key, out value);
}

bool IReadOnlyDictionary<string, object?>.TryGetValue(string key, out object? value)
{
return TryGetValueForKey(key, out value);
}

#endregion

#region ICollection<KeyValuePair<string, object>> Members
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1852,7 +1852,7 @@ public static int Count<TSource>(this ParallelQuery<TSource> source)
// If the data source is a collection, we can just return the count right away.
if (source is ParallelEnumerableWrapper<TSource> sourceAsWrapper)
{
if (sourceAsWrapper.WrappedEnumerable is ICollection<TSource> sourceAsCollection)
if (sourceAsWrapper.WrappedEnumerable is IReadOnlyCollection<TSource> sourceAsCollection)
{
return sourceAsCollection.Count;
}
Expand Down Expand Up @@ -1923,7 +1923,7 @@ public static long LongCount<TSource>(this ParallelQuery<TSource> source)
// If the data source is a collection, we can just return the count right away.
if (source is ParallelEnumerableWrapper<TSource> sourceAsWrapper)
{
if (sourceAsWrapper.WrappedEnumerable is ICollection<TSource> sourceAsCollection)
if (sourceAsWrapper.WrappedEnumerable is IReadOnlyCollection<TSource> sourceAsCollection)
{
return sourceAsCollection.Count;
}
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Linq/src/System/Linq/AnyAll.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public static bool Any<TSource>(this IEnumerable<TSource> source)
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source);
}

if (source is ICollection<TSource> gc)
if (source is IReadOnlyCollection<TSource> gc)
{
return gc.Count != 0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public override int GetCount(bool onlyIfCheap)
return count == -1 ? -1 : count + 1;
}

return !onlyIfCheap || _source is ICollection<TSource> ? _source.Count() + 1 : -1;
return !onlyIfCheap || _source is IReadOnlyCollection<TSource> ? _source.Count() + 1 : -1;
}

public override TSource? TryGetFirst(out bool found)
Expand Down Expand Up @@ -276,7 +276,7 @@ public override int GetCount(bool onlyIfCheap)
return count == -1 ? -1 : count + _appendCount + _prependCount;
}

return !onlyIfCheap || _source is ICollection<TSource> ? _source.Count() + _appendCount + _prependCount : -1;
return !onlyIfCheap || _source is IReadOnlyCollection<TSource> ? _source.Count() + _appendCount + _prependCount : -1;
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Linq/src/System/Linq/Count.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public static int Count<TSource>(this IEnumerable<TSource> source)
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source);
}

if (source is ICollection<TSource> collectionoft)
if (source is IReadOnlyCollection<TSource> collectionoft)
{
return collectionoft.Count;
}
Expand Down Expand Up @@ -101,7 +101,7 @@ public static bool TryGetNonEnumeratedCount<TSource>(this IEnumerable<TSource> s
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source);
}

if (source is ICollection<TSource> collectionoft)
if (source is IReadOnlyCollection<TSource> collectionoft)
{
count = collectionoft.Count;
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public override List<TSource> ToList()
public override int GetCount(bool onlyIfCheap)
{
int count;
if (!onlyIfCheap || _source is ICollection<TSource> || _source is ICollection)
if (!onlyIfCheap || _source is IReadOnlyCollection<TSource> || _source is ICollection)
{
count = _source.Count();
}
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Linq/src/System/Linq/ElementAt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public static TSource ElementAt<TSource>(this IEnumerable<TSource> source, int i
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source);
}

if (source is IList<TSource> list)
if (source is IReadOnlyList<TSource> list)
{
return list[index];
}
Expand Down Expand Up @@ -115,7 +115,7 @@ public static TSource ElementAt<TSource>(this IEnumerable<TSource> source, Index

private static TSource? TryGetElementAt<TSource>(this IEnumerable<TSource> source, int index, out bool found)
{
if (source is IList<TSource> list)
if (source is IReadOnlyList<TSource> list)
{
return (found = (uint)index < (uint)list.Count) ?
list[index] :
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Linq/src/System/Linq/First.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public static TSource FirstOrDefault<TSource>(this IEnumerable<TSource> source,

private static TSource? TryGetFirstNonIterator<TSource>(IEnumerable<TSource> source, out bool found)
{
if (source is IList<TSource> list)
if (source is IReadOnlyList<TSource> list)
{
if (list.Count > 0)
{
Expand Down
17 changes: 16 additions & 1 deletion src/libraries/System.Linq/src/System/Linq/Grouping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ public interface IGrouping<out TKey, out TElement> : IEnumerable<TElement>

[DebuggerDisplay("Key = {Key}")]
[DebuggerTypeProxy(typeof(SystemLinq_GroupingDebugView<,>))]
internal sealed class Grouping<TKey, TElement> : IGrouping<TKey, TElement>, IList<TElement>
internal sealed class Grouping<TKey, TElement> : IGrouping<TKey, TElement>, IList<TElement>, IReadOnlyList<TElement>
{
internal readonly TKey _key;
internal readonly int _hashCode;
Expand Down Expand Up @@ -398,6 +398,8 @@ public IEnumerator<TElement> GetEnumerator()

int ICollection<TElement>.Count => _count;

int IReadOnlyCollection<TElement>.Count => _count;

bool ICollection<TElement>.IsReadOnly => true;

void ICollection<TElement>.Add(TElement item) => ThrowHelper.ThrowNotSupportedException();
Expand Down Expand Up @@ -431,5 +433,18 @@ TElement IList<TElement>.this[int index]

set => ThrowHelper.ThrowNotSupportedException();
}

TElement IReadOnlyList<TElement>.this[int index]
{
get
{
if ((uint)index >= (uint)_count)
{
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.index);
}

return _elements[index];
}
}
}
}
4 changes: 2 additions & 2 deletions src/libraries/System.Linq/src/System/Linq/Last.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public static TSource LastOrDefault<TSource>(this IEnumerable<TSource> source, F

private static TSource? TryGetLastNonIterator<TSource>(IEnumerable<TSource> source, out bool found)
{
if (source is IList<TSource> list)
if (source is IReadOnlyList<TSource> list)
{
int count = list.Count;
if (count > 0)
Expand Down Expand Up @@ -126,7 +126,7 @@ public static TSource LastOrDefault<TSource>(this IEnumerable<TSource> source, F
return ordered.TryGetLast(predicate, out found);
}

if (source is IList<TSource> list)
if (source is IReadOnlyList<TSource> list)
{
for (int i = list.Count - 1; i >= 0; --i)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public override int GetCount(bool onlyIfCheap)
return iterator.GetCount(onlyIfCheap);
}

return !onlyIfCheap || _source is ICollection<TElement> || _source is ICollection ? _source.Count() : -1;
return !onlyIfCheap || _source is IReadOnlyCollection<TElement> || _source is ICollection ? _source.Count() : -1;
}

internal TElement[] ToArray(int minIdx, int maxIdx)
Expand Down
6 changes: 3 additions & 3 deletions src/libraries/System.Linq/src/System/Linq/Reverse.SpeedOpt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public override int GetCount(bool onlyIfCheap) =>

public override TSource? TryGetElementAt(int index, out bool found)
{
if (_source is IList<TSource> list)
if (_source is IReadOnlyList<TSource> list)
{
int count = list.Count;
if ((uint)index < (uint)count)
Expand Down Expand Up @@ -59,7 +59,7 @@ public override int GetCount(bool onlyIfCheap) =>
{
return iterator.TryGetLast(out found);
}
else if (_source is IList<TSource> list)
else if (_source is IReadOnlyList<TSource> list)
{
int count = list.Count;
if (count > 0)
Expand Down Expand Up @@ -95,7 +95,7 @@ public override int GetCount(bool onlyIfCheap) =>
{
return iterator.TryGetFirst(out found);
}
else if (_source is IList<TSource> list)
else if (_source is IReadOnlyList<TSource> list)
{
if (list.Count > 0)
{
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Linq/src/System/Linq/SequenceEqual.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public static bool SequenceEqual<TSource>(this IEnumerable<TSource> first, IEnum
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.second);
}

if (first is ICollection<TSource> firstCol && second is ICollection<TSource> secondCol)
if (first is IReadOnlyCollection<TSource> firstCol && second is IReadOnlyCollection<TSource> secondCol)
{
if (first.TryGetSpan(out ReadOnlySpan<TSource> firstSpan) && second.TryGetSpan(out ReadOnlySpan<TSource> secondSpan))
{
Expand All @@ -34,7 +34,7 @@ public static bool SequenceEqual<TSource>(this IEnumerable<TSource> first, IEnum
return false;
}

if (firstCol is IList<TSource> firstList && secondCol is IList<TSource> secondList)
if (firstCol is IReadOnlyList<TSource> firstList && secondCol is IReadOnlyList<TSource> secondList)
{
comparer ??= EqualityComparer<TSource>.Default;

Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Linq/src/System/Linq/Single.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public static TSource SingleOrDefault<TSource>(this IEnumerable<TSource> source,
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source);
}

if (source is IList<TSource> list)
if (source is IReadOnlyList<TSource> list)
{
switch (list.Count)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ private void AddIfNew(string serviceName)
/// </summary>
private static int GetCountOrOne(IEnumerable collection)
{
ICollection<string>? c = collection as ICollection<string>;
IReadOnlyCollection<string>? c = collection as IReadOnlyCollection<string>;
return c != null ? c.Count : 1;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public ConcurrentQueue(IEnumerable<T> collection)
// case we round its length up to a power of 2, as all segments must
// be a power of 2 in length.
int length = InitialSegmentLength;
if (collection is ICollection<T> c)
if (collection is IReadOnlyCollection<T> c)
{
int count = c.Count;
if (count > length)
Expand Down
Loading

0 comments on commit ec48347

Please sign in to comment.