Compile collection expressions to using ImmutableArray when targeting read-only interfaces

[IS4Code]

Summary

I propose that, in situations when a collection expression is currently compiled to using a compiler-generated collection type to wrap an array in a read-only fashion, that code should construct a System.Collections.Immutable.ImmutableArray<T> instead and not rely on a compiler-generated type.

Background and Motivation

At the moment, collection expressions that target read-only interfaces like IEnumerable<T> or IReadOnlyCollection<T> use a compiler-generated type that implements the desired interface, and a few other likely useful ones:

[CompilerGenerated]
internal sealed class <>z__ReadOnlyArray<T> : IEnumerable, ICollection, IList, IEnumerable<T>, IReadOnlyCollection<T>, IReadOnlyList<T>, ICollection<T>, IList<T>
{
    [CompilerGenerated]
    private readonly T[] _items;
    
    // implementation
}

However, this implementation prevents certain optimizations which test the resulting instance for a more concrete type, such as what TryGetSpan in LINQ does (testing for T[] and List<T>). Because there is no interface that the generated class could use to implement an AsSpan method, there is no way consumers of such instances could get a contiguous view over the items, and use that for efficient enumerating.

To improve this situation, a standard type like ImmutableArray<T> could be used in this case instead. This type implements all of the interfaces currently implemented by the synthesized ReadOnlyArray (and some more) and is structurally equivalent too ‒ all it does is wrapping an array. Using this as the concrete implementation for such collection expressions allows consumers to test for ImmutableArray<T> and call AsSpan easily:

void ProcessCollection(IEnumerable<T> collection)
{
    if(collection is ImmutableArray<T> array)
    {
        // fast processing using array.AsSpan()
    }
    else
    {
        // slow general path using GetEnumerator
    }
}

ProcessCollection([1, 2, 3]); // should go through the fast path

Using ImmutableArray<T> here as an implementation detail is allowed by the language, which permits free choice of either a pre-existing type or a compiler-generated one, as long it implements the interfaces correctly, and does not allow mutation.

Proposed Feature

The compiler should construct an ImmutableArray<T> from a collection expression if all of these conditions are true:

• A read-only interface is requested (IEnumerable<T>, IReadOnlyCollection<T>, IReadOnlyList<T>).
• An array is deemed an acceptable storage for the elements (i.e. something like ReadOnlySingleElementList is not picked instead).
• System.Collections.Immutable.ImmutableArray<T> is available and usable as a collection expression target. Ideally, this should lead to ImmutableCollectionsMarshal.AsImmutableArray.

This should also apply to params (and potentially other future collection expressions-like contexts).

Alternative Designs

• The same could be achieved by adding a new BCL interface that would allow implementing AsSpan in the generated class. While such an interface could be useful on its own, it would not be available for older .NET versions, whereas ImmutableArray<T> is already available and readily usable.
• An array could be used directly, through the requested interface as implemented by the array. This however breaks the requirement that the type must not be mutable. Nevertheless, this is the only option that gets rid of the double indirection when accessing the elements (unless string is considered for IEnumerable<char>).

[jnm2]

ImmutableArray<T> would work great in terms of the existing spec. It meets all the requirements in https://github.com/dotnet/csharplang/blob/main/proposals/csharp-12.0/collection-expressions.md#non-mutable-interface-translation. The alternative design suggestion of using an array directly would fall foul of the spec.

We could also consider expanding this proposal to replace the synthesized types that wrap a List<T>. The downside would be that there would be a final resizing copy when the final list's count is less than its capacity. If that downside is too great, folks will continue to see synthesized types when using spreads of dynamic lengths, and spans will be unavailable.

An additional benefit of using ImmutableArray<T> is that the debugger view is vastly superior compared to the debugger view with the synthesized collection types. This could be remedied, but ImmutableArray<T> is right there.

Current view of synthesized types:

[RikkiGibson]

Could you show the debugger view of ImmutableArray for comparison? What is it about ImmutableArray that causes a nicer display than for a type we synthesize?

[huoyaoyuan]

ImmutableArray is expanded like a List or array.

[CyrusNajmabadi]

I think the following emit strategy would be feasible:

1. For empty collections, emit Array.Empty<T>()
2. For singleton collections, emit the specialized single-allow wrapper around the value itself.
3. For fixed length collections, create the storage array, wrap as an ImmutableArray<T> (with ImmutableCollectionsMarshal).
4. For unknown-length collections, create a List<T> add the values, do a ToArray() and wrap with an ImmutableArray<T>.

[RikkiGibson]

What's nice is that strategies (1), (3), and (4) are already implemented. We just have to tell the lowering to use those strategies in the interface case. And for (1) we are already telling lowering to do that.

[CyrusNajmabadi]

@RikkiGibson Awesome!

[jnm2]

It would be a new thing to do the final resizing copy for the unknown length case (strategy 4). Are we sure we want to do that? Right now we keep the original list around inside the synthesized wrapper so as to avoid that.