Add structural equality and order comparison for common collections #80025
Conversation
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
ghost
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/area-system-collections Issue DetailsCloses #77209
|
|
|
||
| [Serializable] | ||
| [TypeForwardedFrom("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")] | ||
| // Needs to be public to support binary serialization compatibility |
There was a problem hiding this comment.
This is a new type. There are no binary compatibility concerns with new types. We explicitly do not want to make any new types compatible with binary serialization.
The type should be internal and all TypeForwardedFrom and Serializable attributes should be deleted from this change.
There was a problem hiding this comment.
Ok, I was not sure.
Removed!
| // if (x is Dictionary<TKey, TValue> dictx && dictx.Comparer.Equals(_keyComparer) && | ||
| // y is Dictionary<TKey, TValue> dicty && dicty.Comparer.Equals(_keyComparer)) | ||
| // { | ||
| // foreach ((TKey yKey, TValue yValue) in y) | ||
| // { | ||
| // if (!x.TryGetValue(yKey, out TValue xValue) || !_valueComparer.Equals(yValue, xValue)) | ||
| // return false; | ||
| // } | ||
| // | ||
| // return true; | ||
| // } |
There was a problem hiding this comment.
This optimization cannot be activated here, as the Dictionary<TKey, TValue> class has the constraint where TKey : notnull which cannot be enforced here.
Is there a way to solve this?
| where TDictionary : System.Collections.Generic.IReadOnlyDictionary<TKey, TValue> { throw null; } | ||
| } | ||
|
|
||
| public static class ComparerFactory |
There was a problem hiding this comment.
The approved API uses the Comparer class in the System.Collections.Generic namespace, but it is already present in the System.Collections namespace
https://github.com/dotnet/runtime/blob/5b8ebeabb32f7f4118d0cc8b8db28705b62469ee/src/libraries/System.Runtime/ref/System.Runtime.cs#L7251-L7258
Issue reported in #77209 (comment)
|
PR converted to draft as the approved API needs rework. |
ghost
closed this
|
Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it. |
ghost
locked as resolved and limited conversation to collaborators
Closes #77209