Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Make mutable generic collection interfaces implement read-only collection interfaces (#95830)" #101644

Merged
merged 3 commits into from
Apr 27, 2024
Merged

Revert "Make mutable generic collection interfaces implement read-only collection interfaces (#95830)" #101644

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
75ca2b3
Revert "Update ICollection<T> usage to IReadOnlyCollection<T> where a…
jkotas Apr 27, 2024
096d59d
Revert "Make mutable generic collection interfaces implement read-onl…
jkotas Apr 27, 2024
055eb09
Update src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs
jkotas Apr 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions src/coreclr/vm/array.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Loading
Loading