Skip to content

Commit

Permalink
Revert "Make mutable generic collection interfaces implement read-onl…
Browse files Browse the repository at this point in the history 
…y 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
  • Loading branch information
@jkotas
jkotas committed Apr 27, 2024
1 parent 55d2ada commit b8fe1d0
Show file tree
Hide file tree
Showing 37 changed files with 199 additions and 610 deletions.
19 changes: 17 additions & 2 deletions src/coreclr/vm/array.cpp
View file
Expand Up @@ -1210,14 +1210,29 @@ MethodDesc* GetActualImplementationForArrayGenericIListOrIReadOnlyListMethod(Met
}
CONTRACTL_END

int slot = pItfcMeth->GetSlot();

// We need to pick the right starting method depending on the depth of the inheritance chain
static const BinderMethodID startingMethod[] = {
METHOD__SZARRAYHELPER__GETENUMERATOR, // First method of IEnumerable`1
METHOD__SZARRAYHELPER__GET_COUNT, // First method of ICollection`1/IReadOnlyCollection`1
METHOD__SZARRAYHELPER__GET_ITEM // First method of IList`1/IReadOnlyList`1
};

// Subtract one for the non-generic IEnumerable that the generic enumerable inherits from
unsigned int inheritanceDepth = pItfcMeth->GetMethodTable()->GetNumInterfaces() - 1;
PREFIX_ASSUME(0 <= inheritanceDepth && inheritanceDepth < ARRAY_SIZE(startingMethod));

MethodDesc *pGenericImplementor = CoreLibBinder::GetMethod((BinderMethodID)(startingMethod[inheritanceDepth] + slot));

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

// OPTIMIZATION: For any method other than GetEnumerator(), we can safely substitute
// "Object" for reference-type theT's. This causes fewer methods to be instantiated.
if (inheritanceDepth != 0 && !theT.IsValueType())
if (startingMethod[inheritanceDepth] != METHOD__SZARRAYHELPER__GETENUMERATOR &&
!theT.IsValueType())
{
theT = TypeHandle(g_pObjectClass);
}
Expand Down
8 changes: 0 additions & 8 deletions src/libraries/Common/src/System/Diagnostics/DiagnosticsHelper.cs
View file
Expand Up @@ -35,22 +35,14 @@ 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
152 changes: 0 additions & 152 deletions src/libraries/Common/tests/System/Collections/CollectionAsserts.cs
View file
@@ -1,7 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
using System.Linq;
using Xunit;
Expand All @@ -10,151 +9,6 @@ namespace System.Collections.Tests
{
internal static class CollectionAsserts
{
public static void HasCount<T>(ICollection<T> collection, int count)
{
Assert.Equal(count, collection.Count);
#if !NETFRAMEWORK
IReadOnlyCollection<T> readOnlyCollection = collection;
Assert.Equal(count, readOnlyCollection.Count);
#endif
}

public static void EqualAt<T>(IList<T> list, int index, T expected)
{
Assert.Equal(expected, list[index]);
#if !NETFRAMEWORK
IReadOnlyList<T> readOnlyList = list;
Assert.Equal(expected, readOnlyList[index]);
#endif
}

public static void NotEqualAt<T>(IList<T> list, int index, T expected)
{
Assert.NotEqual(expected, list[index]);
#if !NETFRAMEWORK
IReadOnlyList<T> readOnlyList = list;
Assert.NotEqual(expected, readOnlyList[index]);
#endif
}

public static void ThrowsElementAt<T>(IList<T> list, int index, Type exceptionType)
{
Assert.Throws(exceptionType, () => list[index]);
#if !NETFRAMEWORK
IReadOnlyList<T> readOnlyList = list;
Assert.Throws(exceptionType, () => readOnlyList[index]);
#endif
}

public static void ElementAtSucceeds<T>(IList<T> list, int index)
{
T result = list[index];
#if !NETFRAMEWORK
IReadOnlyList<T> readOnlyList = list;
Assert.Equal(result, readOnlyList[index]);
#endif
}

public static void EqualAt<TKey, TValue>(IDictionary<TKey, TValue> dictionary, TKey key, TValue expected)
{
Assert.Equal(expected, dictionary[key]);
#if !NETFRAMEWORK
IReadOnlyDictionary<TKey, TValue> readOnlyDictionary = dictionary;
Assert.Equal(expected, readOnlyDictionary[key]);
#endif
}

public static void ContainsKey<TKey, TValue>(IDictionary<TKey, TValue> dictionary, TKey key, bool expected)
{
Assert.Equal(expected, dictionary.ContainsKey(key));
#if !NETFRAMEWORK
IReadOnlyDictionary<TKey, TValue> readOnlyDictionary = dictionary;
Assert.Equal(expected, readOnlyDictionary.ContainsKey(key));
#endif
}

public static void TryGetValue<TKey, TValue>(IDictionary<TKey, TValue> dictionary, TKey key, bool expected, TValue expectedValue = default)
{
Assert.Equal(expected, dictionary.TryGetValue(key, out TValue value));
if (expected)
{
Assert.Equal(expectedValue, value);
}
#if !NETFRAMEWORK
IReadOnlyDictionary<TKey, TValue> readOnlyDictionary = dictionary;
Assert.Equal(expected, readOnlyDictionary.TryGetValue(key, out value));
if (expected)
{
Assert.Equal(expectedValue, value);
}
#endif
}

public static void Contains<T>(ISet<T> set, T expected)
{
Assert.True(set.Contains(expected));
#if !NETFRAMEWORK
ICollection<T> collection = set;
Assert.True(collection.Contains(expected));
IReadOnlySet<T> readOnlySet = set;
Assert.True(readOnlySet.Contains(expected));
#endif
}

public static void IsProperSubsetOf<T>(ISet<T> set, IEnumerable<T> enumerable, bool expected)
{
Assert.Equal(expected, set.IsProperSubsetOf(enumerable));
#if !NETFRAMEWORK
IReadOnlySet<T> readOnlySet = set;
Assert.Equal(expected, readOnlySet.IsProperSubsetOf(enumerable));
#endif
}

public static void IsProperSupersetOf<T>(ISet<T> set, IEnumerable<T> enumerable, bool expected)
{
Assert.Equal(expected, set.IsProperSupersetOf(enumerable));
#if !NETFRAMEWORK
IReadOnlySet<T> readOnlySet = set;
Assert.Equal(expected, readOnlySet.IsProperSupersetOf(enumerable));
#endif
}

public static void IsSubsetOf<T>(ISet<T> set, IEnumerable<T> enumerable, bool expected)
{
Assert.Equal(expected, set.IsSubsetOf(enumerable));
#if !NETFRAMEWORK
IReadOnlySet<T> readOnlySet = set;
Assert.Equal(expected, readOnlySet.IsSubsetOf(enumerable));
#endif
}

public static void IsSupersetOf<T>(ISet<T> set, IEnumerable<T> enumerable, bool expected)
{
Assert.Equal(expected, set.IsSupersetOf(enumerable));
#if !NETFRAMEWORK
IReadOnlySet<T> readOnlySet = set;
Assert.Equal(expected, readOnlySet.IsSupersetOf(enumerable));
#endif
}

public static void Overlaps<T>(ISet<T> set, IEnumerable<T> enumerable, bool expected)
{
Assert.Equal(expected, set.Overlaps(enumerable));
#if !NETFRAMEWORK
IReadOnlySet<T> readOnlySet = set;
Assert.Equal(expected, readOnlySet.Overlaps(enumerable));
#endif
}

public static void SetEquals<T>(ISet<T> set, IEnumerable<T> enumerable, bool expected)
{
Assert.Equal(expected, set.SetEquals(enumerable));
#if !NETFRAMEWORK
IReadOnlySet<T> readOnlySet = set;
Assert.Equal(expected, readOnlySet.SetEquals(enumerable));
#endif
}

public static void Equal(ICollection expected, ICollection actual)
{
Assert.Equal(expected == null, actual == null);
Expand Down Expand Up @@ -189,12 +43,6 @@ public static void Equal<T>(ICollection<T> expected, ICollection<T> actual)
return;
}
Assert.Equal(expected.Count, actual.Count);
#if !NETFRAMEWORK
IReadOnlyCollection<T> readOnlyExpected = expected;
Assert.Equal(expected.Count, readOnlyExpected.Count);
IReadOnlyCollection<T> readOnlyActual = actual;
Assert.Equal(actual.Count, readOnlyActual.Count);
#endif
IEnumerator<T> e = expected.GetEnumerator();
IEnumerator<T> a = actual.GetEnumerator();
while (e.MoveNext())
Expand Down

0 comments on commit b8fe1d0

Please sign in to comment.