Skip to content

Commit b8fe1d0

Browse files
jkotasjkotas
authored
Revert "Make mutable generic collection interfaces implement read-only collection interfaces (#95830)" (#101644)
* Revert "Update ICollection<T> usage to IReadOnlyCollection<T> where applicable (#101469)" This reverts commit e92b7d0. * Revert "Make mutable generic collection interfaces implement read-only collection interfaces (#95830)" This reverts commit a2bd583. * Update src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs
1 parent 55d2ada commit b8fe1d0

File tree

37 files changed

+199
-610
lines changed

37 files changed

+199
-610
lines changed

‎src/coreclr/vm/array.cpp

+17-2
Original file line numberDiff line numberDiff line change
@@ -1210,14 +1210,29 @@ MethodDesc* GetActualImplementationForArrayGenericIListOrIReadOnlyListMethod(Met
12101210
}
12111211
CONTRACTL_END
12121212

1213+
int slot = pItfcMeth->GetSlot();
1214+
1215+
// We need to pick the right starting method depending on the depth of the inheritance chain
1216+
static const BinderMethodID startingMethod[] = {
1217+
METHOD__SZARRAYHELPER__GETENUMERATOR, // First method of IEnumerable`1
1218+
METHOD__SZARRAYHELPER__GET_COUNT, // First method of ICollection`1/IReadOnlyCollection`1
1219+
METHOD__SZARRAYHELPER__GET_ITEM // First method of IList`1/IReadOnlyList`1
1220+
};
1221+
12131222
// Subtract one for the non-generic IEnumerable that the generic enumerable inherits from
12141223
unsigned int inheritanceDepth = pItfcMeth->GetMethodTable()->GetNumInterfaces() - 1;
1224+
PREFIX_ASSUME(0 <= inheritanceDepth && inheritanceDepth < ARRAY_SIZE(startingMethod));
1225+
1226+
MethodDesc *pGenericImplementor = CoreLibBinder::GetMethod((BinderMethodID)(startingMethod[inheritanceDepth] + slot));
12151227

1216-
MethodDesc *pGenericImplementor = MemberLoader::FindMethodByName(g_pSZArrayHelperClass, pItfcMeth->GetName());
1228+
// The most common reason for this assert is that the order of the SZArrayHelper methods in
1229+
// corelib.h does not match the order they are implemented on the generic interfaces.
1230+
_ASSERTE(pGenericImplementor == MemberLoader::FindMethodByName(g_pSZArrayHelperClass, pItfcMeth->GetName()));
12171231

12181232
// OPTIMIZATION: For any method other than GetEnumerator(), we can safely substitute
12191233
// "Object" for reference-type theT's. This causes fewer methods to be instantiated.
1220-
if (inheritanceDepth != 0 && !theT.IsValueType())
1234+
if (startingMethod[inheritanceDepth] != METHOD__SZARRAYHELPER__GETENUMERATOR &&
1235+
!theT.IsValueType())
12211236
{
12221237
theT = TypeHandle(g_pObjectClass);
12231238
}

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

-8
Original file line numberDiff line numberDiff line change
@@ -35,22 +35,14 @@ 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
4138
if (tags2 is ICollection<KeyValuePair<string, object?>> tagsCol)
42-
#endif
4339
{
4440
if (tagsCol.Count != count)
4541
{
4642
return false;
4743
}
4844

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

‎src/libraries/Common/tests/System/Collections/CollectionAsserts.cs

-152
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System;
54
using System.Collections.Generic;
65
using System.Linq;
76
using Xunit;
@@ -10,151 +9,6 @@ namespace System.Collections.Tests
109
{
1110
internal static class CollectionAsserts
1211
{
13-
public static void HasCount<T>(ICollection<T> collection, int count)
14-
{
15-
Assert.Equal(count, collection.Count);
16-
#if !NETFRAMEWORK
17-
IReadOnlyCollection<T> readOnlyCollection = collection;
18-
Assert.Equal(count, readOnlyCollection.Count);
19-
#endif
20-
}
21-
22-
public static void EqualAt<T>(IList<T> list, int index, T expected)
23-
{
24-
Assert.Equal(expected, list[index]);
25-
#if !NETFRAMEWORK
26-
IReadOnlyList<T> readOnlyList = list;
27-
Assert.Equal(expected, readOnlyList[index]);
28-
#endif
29-
}
30-
31-
public static void NotEqualAt<T>(IList<T> list, int index, T expected)
32-
{
33-
Assert.NotEqual(expected, list[index]);
34-
#if !NETFRAMEWORK
35-
IReadOnlyList<T> readOnlyList = list;
36-
Assert.NotEqual(expected, readOnlyList[index]);
37-
#endif
38-
}
39-
40-
public static void ThrowsElementAt<T>(IList<T> list, int index, Type exceptionType)
41-
{
42-
Assert.Throws(exceptionType, () => list[index]);
43-
#if !NETFRAMEWORK
44-
IReadOnlyList<T> readOnlyList = list;
45-
Assert.Throws(exceptionType, () => readOnlyList[index]);
46-
#endif
47-
}
48-
49-
public static void ElementAtSucceeds<T>(IList<T> list, int index)
50-
{
51-
T result = list[index];
52-
#if !NETFRAMEWORK
53-
IReadOnlyList<T> readOnlyList = list;
54-
Assert.Equal(result, readOnlyList[index]);
55-
#endif
56-
}
57-
58-
public static void EqualAt<TKey, TValue>(IDictionary<TKey, TValue> dictionary, TKey key, TValue expected)
59-
{
60-
Assert.Equal(expected, dictionary[key]);
61-
#if !NETFRAMEWORK
62-
IReadOnlyDictionary<TKey, TValue> readOnlyDictionary = dictionary;
63-
Assert.Equal(expected, readOnlyDictionary[key]);
64-
#endif
65-
}
66-
67-
public static void ContainsKey<TKey, TValue>(IDictionary<TKey, TValue> dictionary, TKey key, bool expected)
68-
{
69-
Assert.Equal(expected, dictionary.ContainsKey(key));
70-
#if !NETFRAMEWORK
71-
IReadOnlyDictionary<TKey, TValue> readOnlyDictionary = dictionary;
72-
Assert.Equal(expected, readOnlyDictionary.ContainsKey(key));
73-
#endif
74-
}
75-
76-
public static void TryGetValue<TKey, TValue>(IDictionary<TKey, TValue> dictionary, TKey key, bool expected, TValue expectedValue = default)
77-
{
78-
Assert.Equal(expected, dictionary.TryGetValue(key, out TValue value));
79-
if (expected)
80-
{
81-
Assert.Equal(expectedValue, value);
82-
}
83-
#if !NETFRAMEWORK
84-
IReadOnlyDictionary<TKey, TValue> readOnlyDictionary = dictionary;
85-
Assert.Equal(expected, readOnlyDictionary.TryGetValue(key, out value));
86-
if (expected)
87-
{
88-
Assert.Equal(expectedValue, value);
89-
}
90-
#endif
91-
}
92-
93-
public static void Contains<T>(ISet<T> set, T expected)
94-
{
95-
Assert.True(set.Contains(expected));
96-
#if !NETFRAMEWORK
97-
ICollection<T> collection = set;
98-
Assert.True(collection.Contains(expected));
99-
IReadOnlySet<T> readOnlySet = set;
100-
Assert.True(readOnlySet.Contains(expected));
101-
#endif
102-
}
103-
104-
public static void IsProperSubsetOf<T>(ISet<T> set, IEnumerable<T> enumerable, bool expected)
105-
{
106-
Assert.Equal(expected, set.IsProperSubsetOf(enumerable));
107-
#if !NETFRAMEWORK
108-
IReadOnlySet<T> readOnlySet = set;
109-
Assert.Equal(expected, readOnlySet.IsProperSubsetOf(enumerable));
110-
#endif
111-
}
112-
113-
public static void IsProperSupersetOf<T>(ISet<T> set, IEnumerable<T> enumerable, bool expected)
114-
{
115-
Assert.Equal(expected, set.IsProperSupersetOf(enumerable));
116-
#if !NETFRAMEWORK
117-
IReadOnlySet<T> readOnlySet = set;
118-
Assert.Equal(expected, readOnlySet.IsProperSupersetOf(enumerable));
119-
#endif
120-
}
121-
122-
public static void IsSubsetOf<T>(ISet<T> set, IEnumerable<T> enumerable, bool expected)
123-
{
124-
Assert.Equal(expected, set.IsSubsetOf(enumerable));
125-
#if !NETFRAMEWORK
126-
IReadOnlySet<T> readOnlySet = set;
127-
Assert.Equal(expected, readOnlySet.IsSubsetOf(enumerable));
128-
#endif
129-
}
130-
131-
public static void IsSupersetOf<T>(ISet<T> set, IEnumerable<T> enumerable, bool expected)
132-
{
133-
Assert.Equal(expected, set.IsSupersetOf(enumerable));
134-
#if !NETFRAMEWORK
135-
IReadOnlySet<T> readOnlySet = set;
136-
Assert.Equal(expected, readOnlySet.IsSupersetOf(enumerable));
137-
#endif
138-
}
139-
140-
public static void Overlaps<T>(ISet<T> set, IEnumerable<T> enumerable, bool expected)
141-
{
142-
Assert.Equal(expected, set.Overlaps(enumerable));
143-
#if !NETFRAMEWORK
144-
IReadOnlySet<T> readOnlySet = set;
145-
Assert.Equal(expected, readOnlySet.Overlaps(enumerable));
146-
#endif
147-
}
148-
149-
public static void SetEquals<T>(ISet<T> set, IEnumerable<T> enumerable, bool expected)
150-
{
151-
Assert.Equal(expected, set.SetEquals(enumerable));
152-
#if !NETFRAMEWORK
153-
IReadOnlySet<T> readOnlySet = set;
154-
Assert.Equal(expected, readOnlySet.SetEquals(enumerable));
155-
#endif
156-
}
157-
15812
public static void Equal(ICollection expected, ICollection actual)
15913
{
16014
Assert.Equal(expected == null, actual == null);
@@ -189,12 +43,6 @@ public static void Equal<T>(ICollection<T> expected, ICollection<T> actual)
18943
return;
19044
}
19145
Assert.Equal(expected.Count, actual.Count);
192-
#if !NETFRAMEWORK
193-
IReadOnlyCollection<T> readOnlyExpected = expected;
194-
Assert.Equal(expected.Count, readOnlyExpected.Count);
195-
IReadOnlyCollection<T> readOnlyActual = actual;
196-
Assert.Equal(actual.Count, readOnlyActual.Count);
197-
#endif
19846
IEnumerator<T> e = expected.GetEnumerator();
19947
IEnumerator<T> a = actual.GetEnumerator();
20048
while (e.MoveNext())

0 commit comments

Comments
 (0)