Use ICollection<T> check instead of IList<T> in TryCopyTo

[prozolic]

#88181

This PR change the type check from IList to ICollection. With this change, collections that implement ICollection will have ICollection.CopyTo invoked.

• Change the type check from IList<T> to ICollection<T>
• If the type does not match List<T>, T[], or ImmutableArray<T>, fall back to ICollection<T>.CopyTo
• Add conditional compilation( #if NET) to ensure the sequence is not an array before falling back to ICollection<T>.CopyTo to avoid issues with array covariance in .NET Framework

Benchmark

#88181 (comment)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR changes ImmutableExtensions.TryCopyTo to use ICollection<T> as the type gate instead of IList<T>. This allows collections that implement ICollection<T> but not IList<T> (such as Dictionary<K,V>.Keys and Dictionary<K,V>.Values) to benefit from the fast CopyTo path rather than falling back to per-element enumeration. The performance motivation is that LINQ's own ToArray already takes this ICollection<T> path, and the previous restriction was making immutable collection operations unnecessarily slower.

Changes:

• Changes the outer type check from IList<T> to ICollection<T>, broadening the fast-copy path to any ICollection<T> implementation.
• After the well-known-type fast paths (List<T>, exact T[], ImmutableArray<T>), falls through to a generic ICollection<T>.CopyTo call for all other implementors.
• Adds a #if !NET guard to skip arrays that are not exactly T[] (covariant arrays) when running on .NET Framework, where ICollection<T>.CopyTo on such arrays can throw ArrayTypeMismatchException.

---

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

---

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

The comment in the #if !NET guard states: "if 'sequence' is actually a covariant array (for example, string[] used as ICollection<object>)". This example is incorrect: generic interfaces like ICollection<T> are invariant, so string[] cannot satisfy ICollection<object>. Therefore, a string[] would not pass the sequence is ICollection<T> guard at all when T is object, making it impossible to reach this code with that example. A more accurate explanation might reference situations where an array subtype passes the ICollection<T> check on .NET Framework despite the sequence.GetType() == typeof(T[]) check not matching. If no such scenario exists, the guard and comment should explain the actual risk more precisely, or be documented as a conservative safety measure.

[stephentoub]

https://github.com/dotnet/runtime/pull/125304/changes#diff-79ebb1b84d82a6117697d669c596bdcb999803d9ecf5a5339d6431d1ac0c5f7eR93-R96 suggests the lack of this is on purpose

[prozolic]

Thanks for the pointer. You're right that the original IList restriction was intentional.
The implementation approach follows what was discussed and considered in #88181 (#88093 (comment)).
@adamsitnik also confirmed in #88181 that the benchmark results look good.