Synthesize type for collection expressions targeting IEnumerable<T>, IReadOnlyCollection<T>, or IReadOnlyList<T>

[CyrusNajmabadi]

For collection expressions of the target types IEnumerable<T>, IReadOnlyCollection<T>, or IReadOnlyList<T>, generate an instance of a single internal synthesized type.

The synthesized type:

• is used for collection expressions regardless of whether the expression has a known length
• implements IEnumerable<T>, IReadOnlyCollection<T>, IReadOnlyList<T>, and ICollection<T> and IList<T>
• returns true for ICollection<T>.IsReadOnly
• throws a System.NotSupportedException from mutating methods in ICollection<T> and IList<T>, including IList<T>.this[int] { set; }

For collection expressions of the target types ICollection<T> or IList<T>, the compiler will generate instances of List<T> (no changes).

Relates to test plan #66418

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[alrz]

Is it valid to use Array.AsReadOnly, List<T>.AsReadOnly instead of synthesizing the whole type?

(the former does a length check, perhaps it could be added to the latter too but it would be nice if the runtime could avoid the allocation altogether.)

[CyrusNajmabadi]

List.AsReadOnly would be 3 allocations. The array, the surrounding list, and the surrounding readonly list.

This would be valid, but wasteful.

[alrz]

Using that helper wouldn't change the number of allocations (a List, or array is already allocated), this would only save the type being synthesized. But I suppose using a helper opens up the possibility of optimizing things further in the runtime.

[CyrusNajmabadi]

an extra allocation on every instance seems a lot worse than just having the extra synthesized type :)

[alrz]

an extra allocation on every instance seems

That's already the case: SynthesizedListType->List->Array. To avoid allocating SynthesizedListType would it be overkill to generate (a subset of) List<T> instead? Because other than avoiding misusage, it seem to be completely unnecessary imo.

[CyrusNajmabadi]

That's already the case: SynthesizedListType->List->Array

It shouldn't be. And, if that's teh case, we should fix that. The synthesized list should just hold the backing data.

[alrz]

That should be easy to implement except that it would incur a copy for List<T>.ToArray() (collection marshal would only give you a span) maybe that's still better than holding onto the entire List<T> instance.

[alrz]

Note, that's only talking about unknown-length collection expressions; what I was getting at there was that before collection expressions, one would use the array instance directly,

IReadOnlyCollection<T> M() { return new[] { 1 , 2 , 3 }; }

Using collection expressions, you will need to pay extra for the wrapper type.

IReadOnlyCollection<T> M() { return [1 , 2 , 3]; }

The wrapper type for IEnumerable<T> could be useful (to inline the eumerator the same way yield works), but other than that (for IReadOnlyList and IReadOnlyCollection) collection expressions would be suboptimal.

[CyrusNajmabadi]

@alrz my overall point is this:

1. The language rules have specific restrictions that must be abided to by a compliant impl.
2. Within those bounds, the compiler is free to optimize.
3. We should optimize to lower allocations when easy to do so.

For the IReadOnlyXXX interfaces that means:

1. when it's a known length, allocate the array, and wrap that array with a trivial wrapper that ensures readonlyness.
2. when it's an unknown length, use a growable array, wrap that array and the count with a trivial wrapper that ensures readonlyness.