Skip to content

Commit ec48347

Browse files
stephentoubmichaelgsharp
stephentoub
authored and
michaelgsharp
committed
Update ICollection<T> usage to IReadOnlyCollection<T> where applicable (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.
1 parent 717e940 commit ec48347

File tree

25 files changed

+121
-36
lines changed

25 files changed

+121
-36
lines changed

‎src/libraries/Common/src/System/Diagnostics/DiagnosticsHelper.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,22 @@ internal static bool CompareTags(List<KeyValuePair<string, object?>>? sortedTags
3535
int size = count / (sizeof(ulong) * 8) + 1;
3636
BitMapper bitMapper = new BitMapper(size <= 100 ? stackalloc ulong[size] : new ulong[size]);
3737

38+
#if NET9_0_OR_GREATER // ICollection<T> : IReadOnlyCollection<T> on .NET 9+
39+
if (tags2 is IReadOnlyCollection<KeyValuePair<string, object?>> tagsCol)
40+
#else
3841
if (tags2 is ICollection<KeyValuePair<string, object?>> tagsCol)
42+
#endif
3943
{
4044
if (tagsCol.Count != count)
4145
{
4246
return false;
4347
}
4448

49+
#if NET9_0_OR_GREATER // IList<T> : IReadOnlyList<T> on .NET 9+
50+
if (tagsCol is IReadOnlyList<KeyValuePair<string, object?>> secondList)
51+
#else
4552
if (tagsCol is IList<KeyValuePair<string, object?>> secondList)
53+
#endif
4654
{
4755
for (int i = 0; i < count; i++)
4856
{

‎src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSetInternalBase.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,11 @@ private protected override bool IsProperSubsetOfCore(IEnumerable<T> other)
3232
{
3333
Debug.Assert(_thisSet.Count != 0, "EmptyFrozenSet should have been used.");
3434

35+
#if NET9_0_OR_GREATER // ICollection<T> : IReadOnlyCollection<T> on .NET 9+
36+
if (other is IReadOnlyCollection<T> otherAsCollection)
37+
#else
3538
if (other is ICollection<T> otherAsCollection)
39+
#endif
3640
{
3741
int otherCount = otherAsCollection.Count;
3842

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

66+
#if NET9_0_OR_GREATER // ICollection<T> : IReadOnlyCollection<T> on .NET 9+
67+
if (other is IReadOnlyCollection<T> otherAsCollection)
68+
#else
6269
if (other is ICollection<T> otherAsCollection)
70+
#endif
6371
{
6472
int otherCount = otherAsCollection.Count;
6573

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

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

‎src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableExtensions.Minimal.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,13 @@ internal static bool TryGetCount<T>(this IEnumerable sequence, out int count)
3838
return true;
3939
}
4040

41+
#if !NET9_0_OR_GREATER // ICollection<T> : IReadOnlyCollection<T> on .NET 9+
4142
if (sequence is ICollection<T> collectionOfT)
4243
{
4344
count = collectionOfT.Count;
4445
return true;
4546
}
47+
#endif
4648

4749
if (sequence is IReadOnlyCollection<T> readOnlyCollection)
4850
{

‎src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ public void EnqueueRange(IEnumerable<TElement> elements, TPriority priority)
462462
ArgumentNullException.ThrowIfNull(elements);
463463

464464
int count;
465-
if (elements is ICollection<TElement> collection &&
465+
if (elements is IReadOnlyCollection<TElement> collection &&
466466
(count = collection.Count) > _nodes.Length - _size)
467467
{
468468
Grow(checked(_size + count));

‎src/libraries/System.Collections/src/System/Collections/Generic/SortedSet.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1323,7 +1323,7 @@ public bool Overlaps(IEnumerable<T> other)
13231323
if (Count == 0)
13241324
return false;
13251325

1326-
if (other is ICollection<T> c && c.Count == 0)
1326+
if (other is IReadOnlyCollection<T> c && c.Count == 0)
13271327
return false;
13281328

13291329
SortedSet<T>? asSorted = other as SortedSet<T>;

‎src/libraries/System.Linq.Expressions/src/System/Dynamic/ExpandoObject.cs

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ namespace System.Dynamic
1717
/// <summary>
1818
/// Represents an object with members that can be dynamically added and removed at runtime.
1919
/// </summary>
20-
public sealed class ExpandoObject : IDynamicMetaObjectProvider, IDictionary<string, object?>, INotifyPropertyChanged
20+
public sealed class ExpandoObject : IDynamicMetaObjectProvider, IDictionary<string, object?>, IReadOnlyDictionary<string, object?>, INotifyPropertyChanged
2121
{
2222
private static readonly MethodInfo s_expandoTryGetValue =
2323
typeof(RuntimeOps).GetMethod(nameof(RuntimeOps.ExpandoTryGetValue))!;
@@ -618,6 +618,10 @@ System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
618618

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

621+
IEnumerable<string> IReadOnlyDictionary<string, object?>.Keys => new KeyCollection(this);
622+
623+
IEnumerable<object?> IReadOnlyDictionary<string, object?>.Values => new ValueCollection(this);
624+
621625
object? IDictionary<string, object?>.this[string key]
622626
{
623627
get
@@ -636,6 +640,18 @@ System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
636640
}
637641
}
638642

643+
object? IReadOnlyDictionary<string, object?>.this[string key]
644+
{
645+
get
646+
{
647+
if (!TryGetValueForKey(key, out object? value))
648+
{
649+
throw System.Linq.Expressions.Error.KeyDoesNotExistInExpando(key);
650+
}
651+
return value;
652+
}
653+
}
654+
639655
void IDictionary<string, object?>.Add(string key, object? value)
640656
{
641657
this.TryAddMember(key, value);
@@ -650,6 +666,15 @@ System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
650666
return index >= 0 && data[index] != Uninitialized;
651667
}
652668

669+
bool IReadOnlyDictionary<string, object?>.ContainsKey(string key)
670+
{
671+
ArgumentNullException.ThrowIfNull(key);
672+
673+
ExpandoData data = _data;
674+
int index = data.Class.GetValueIndexCaseSensitive(key);
675+
return index >= 0 && data[index] != Uninitialized;
676+
}
677+
653678
bool IDictionary<string, object?>.Remove(string key)
654679
{
655680
ArgumentNullException.ThrowIfNull(key);
@@ -662,6 +687,11 @@ System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
662687
return TryGetValueForKey(key, out value);
663688
}
664689

690+
bool IReadOnlyDictionary<string, object?>.TryGetValue(string key, out object? value)
691+
{
692+
return TryGetValueForKey(key, out value);
693+
}
694+
665695
#endregion
666696

667697
#region ICollection<KeyValuePair<string, object>> Members

‎src/libraries/System.Linq.Parallel/src/System/Linq/ParallelEnumerable.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1852,7 +1852,7 @@ public static int Count<TSource>(this ParallelQuery<TSource> source)
18521852
// If the data source is a collection, we can just return the count right away.
18531853
if (source is ParallelEnumerableWrapper<TSource> sourceAsWrapper)
18541854
{
1855-
if (sourceAsWrapper.WrappedEnumerable is ICollection<TSource> sourceAsCollection)
1855+
if (sourceAsWrapper.WrappedEnumerable is IReadOnlyCollection<TSource> sourceAsCollection)
18561856
{
18571857
return sourceAsCollection.Count;
18581858
}
@@ -1923,7 +1923,7 @@ public static long LongCount<TSource>(this ParallelQuery<TSource> source)
19231923
// If the data source is a collection, we can just return the count right away.
19241924
if (source is ParallelEnumerableWrapper<TSource> sourceAsWrapper)
19251925
{
1926-
if (sourceAsWrapper.WrappedEnumerable is ICollection<TSource> sourceAsCollection)
1926+
if (sourceAsWrapper.WrappedEnumerable is IReadOnlyCollection<TSource> sourceAsCollection)
19271927
{
19281928
return sourceAsCollection.Count;
19291929
}

‎src/libraries/System.Linq/src/System/Linq/AnyAll.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public static bool Any<TSource>(this IEnumerable<TSource> source)
1515
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source);
1616
}
1717

18-
if (source is ICollection<TSource> gc)
18+
if (source is IReadOnlyCollection<TSource> gc)
1919
{
2020
return gc.Count != 0;
2121
}

‎src/libraries/System.Linq/src/System/Linq/AppendPrepend.SpeedOpt.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ public override int GetCount(bool onlyIfCheap)
127127
return count == -1 ? -1 : count + 1;
128128
}
129129

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

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

279-
return !onlyIfCheap || _source is ICollection<TSource> ? _source.Count() + _appendCount + _prependCount : -1;
279+
return !onlyIfCheap || _source is IReadOnlyCollection<TSource> ? _source.Count() + _appendCount + _prependCount : -1;
280280
}
281281
}
282282
}

‎src/libraries/System.Linq/src/System/Linq/Count.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public static int Count<TSource>(this IEnumerable<TSource> source)
1515
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source);
1616
}
1717

18-
if (source is ICollection<TSource> collectionoft)
18+
if (source is IReadOnlyCollection<TSource> collectionoft)
1919
{
2020
return collectionoft.Count;
2121
}
@@ -101,7 +101,7 @@ public static bool TryGetNonEnumeratedCount<TSource>(this IEnumerable<TSource> s
101101
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source);
102102
}
103103

104-
if (source is ICollection<TSource> collectionoft)
104+
if (source is IReadOnlyCollection<TSource> collectionoft)
105105
{
106106
count = collectionoft.Count;
107107
return true;

‎src/libraries/System.Linq/src/System/Linq/DefaultIfEmpty.SpeedOpt.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public override List<TSource> ToList()
3030
public override int GetCount(bool onlyIfCheap)
3131
{
3232
int count;
33-
if (!onlyIfCheap || _source is ICollection<TSource> || _source is ICollection)
33+
if (!onlyIfCheap || _source is IReadOnlyCollection<TSource> || _source is ICollection)
3434
{
3535
count = _source.Count();
3636
}

‎src/libraries/System.Linq/src/System/Linq/ElementAt.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public static TSource ElementAt<TSource>(this IEnumerable<TSource> source, int i
1616
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source);
1717
}
1818

19-
if (source is IList<TSource> list)
19+
if (source is IReadOnlyList<TSource> list)
2020
{
2121
return list[index];
2222
}
@@ -115,7 +115,7 @@ public static TSource ElementAt<TSource>(this IEnumerable<TSource> source, Index
115115

116116
private static TSource? TryGetElementAt<TSource>(this IEnumerable<TSource> source, int index, out bool found)
117117
{
118-
if (source is IList<TSource> list)
118+
if (source is IReadOnlyList<TSource> list)
119119
{
120120
return (found = (uint)index < (uint)list.Count) ?
121121
list[index] :

‎src/libraries/System.Linq/src/System/Linq/First.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public static TSource FirstOrDefault<TSource>(this IEnumerable<TSource> source,
7878

7979
private static TSource? TryGetFirstNonIterator<TSource>(IEnumerable<TSource> source, out bool found)
8080
{
81-
if (source is IList<TSource> list)
81+
if (source is IReadOnlyList<TSource> list)
8282
{
8383
if (list.Count > 0)
8484
{

‎src/libraries/System.Linq/src/System/Linq/Grouping.cs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ public interface IGrouping<out TKey, out TElement> : IEnumerable<TElement>
351351

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

399399
int ICollection<TElement>.Count => _count;
400400

401+
int IReadOnlyCollection<TElement>.Count => _count;
402+
401403
bool ICollection<TElement>.IsReadOnly => true;
402404

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

432434
set => ThrowHelper.ThrowNotSupportedException();
433435
}
436+
437+
TElement IReadOnlyList<TElement>.this[int index]
438+
{
439+
get
440+
{
441+
if ((uint)index >= (uint)_count)
442+
{
443+
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.index);
444+
}
445+
446+
return _elements[index];
447+
}
448+
}
434449
}
435450
}

‎src/libraries/System.Linq/src/System/Linq/Last.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public static TSource LastOrDefault<TSource>(this IEnumerable<TSource> source, F
7777

7878
private static TSource? TryGetLastNonIterator<TSource>(IEnumerable<TSource> source, out bool found)
7979
{
80-
if (source is IList<TSource> list)
80+
if (source is IReadOnlyList<TSource> list)
8181
{
8282
int count = list.Count;
8383
if (count > 0)
@@ -126,7 +126,7 @@ public static TSource LastOrDefault<TSource>(this IEnumerable<TSource> source, F
126126
return ordered.TryGetLast(predicate, out found);
127127
}
128128

129-
if (source is IList<TSource> list)
129+
if (source is IReadOnlyList<TSource> list)
130130
{
131131
for (int i = list.Count - 1; i >= 0; --i)
132132
{

‎src/libraries/System.Linq/src/System/Linq/OrderedEnumerable.SpeedOpt.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public override int GetCount(bool onlyIfCheap)
5858
return iterator.GetCount(onlyIfCheap);
5959
}
6060

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

6464
internal TElement[] ToArray(int minIdx, int maxIdx)

‎src/libraries/System.Linq/src/System/Linq/Reverse.SpeedOpt.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public override int GetCount(bool onlyIfCheap) =>
3030

3131
public override TSource? TryGetElementAt(int index, out bool found)
3232
{
33-
if (_source is IList<TSource> list)
33+
if (_source is IReadOnlyList<TSource> list)
3434
{
3535
int count = list.Count;
3636
if ((uint)index < (uint)count)
@@ -59,7 +59,7 @@ public override int GetCount(bool onlyIfCheap) =>
5959
{
6060
return iterator.TryGetLast(out found);
6161
}
62-
else if (_source is IList<TSource> list)
62+
else if (_source is IReadOnlyList<TSource> list)
6363
{
6464
int count = list.Count;
6565
if (count > 0)
@@ -95,7 +95,7 @@ public override int GetCount(bool onlyIfCheap) =>
9595
{
9696
return iterator.TryGetFirst(out found);
9797
}
98-
else if (_source is IList<TSource> list)
98+
else if (_source is IReadOnlyList<TSource> list)
9999
{
100100
if (list.Count > 0)
101101
{

‎src/libraries/System.Linq/src/System/Linq/SequenceEqual.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public static bool SequenceEqual<TSource>(this IEnumerable<TSource> first, IEnum
2222
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.second);
2323
}
2424

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

37-
if (firstCol is IList<TSource> firstList && secondCol is IList<TSource> secondList)
37+
if (firstCol is IReadOnlyList<TSource> firstList && secondCol is IReadOnlyList<TSource> secondList)
3838
{
3939
comparer ??= EqualityComparer<TSource>.Default;
4040

‎src/libraries/System.Linq/src/System/Linq/Single.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public static TSource SingleOrDefault<TSource>(this IEnumerable<TSource> source,
6969
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source);
7070
}
7171

72-
if (source is IList<TSource> list)
72+
if (source is IReadOnlyList<TSource> list)
7373
{
7474
switch (list.Count)
7575
{

‎src/libraries/System.Net.Security/src/System/Security/Authentication/ExtendedProtection/ServiceNameCollection.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ private void AddIfNew(string serviceName)
147147
/// </summary>
148148
private static int GetCountOrOne(IEnumerable collection)
149149
{
150-
ICollection<string>? c = collection as ICollection<string>;
150+
IReadOnlyCollection<string>? c = collection as IReadOnlyCollection<string>;
151151
return c != null ? c.Count : 1;
152152
}
153153

‎src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentQueue.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public ConcurrentQueue(IEnumerable<T> collection)
8989
// case we round its length up to a power of 2, as all segments must
9090
// be a power of 2 in length.
9191
int length = InitialSegmentLength;
92-
if (collection is ICollection<T> c)
92+
if (collection is IReadOnlyCollection<T> c)
9393
{
9494
int count = c.Count;
9595
if (count > length)

0 commit comments

Comments
 (0)