Add collection literal support for Memory<T> and ReadOnlyMemory<T>.

[eiriktsarpalis]

Would be nice if we could add collection literal support for Memory types:

CollectionBuilderAttribute-based proposal

API Proposal

namespace System;

+[CollectionBuilder(typeof(MemoryExtensions), nameof(MemoryExtensions.CreateMemoryFromSpan)]
public partial readonly struct Memory<T>;

+[CollectionBuilder(typeof(MemoryExtensions), nameof(MemoryExtensions.CreateReadOnlyMemoryFromSpan))]
public partial readonly struct ReadOnlyMemory<T>;

public partial class MemoryExtensions
{
+    public static Memory<T> CreateMemoryFromSpan(ReadOnlySpan<T> span);
+    public static ReadOnlyMemory<T> CreateReadOnlyMemoryFromSpan(ReadOnlySpan<T> span);
}

Alternative Designs

Memory literal support could be implemented by the compiler directly, avoiding the need to copy elements from an intermediate span.

cc @stephentoub @CyrusNajmabadi

[dotnet-policy-service[bot]]

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

[stephentoub]

If we want to enable this, it'd be better if either a) it was implemented in the compiler or b) a new builder pattern was supported by the compiler to enable it to create an array and do ownership transfer. Otherwise, the compiler is going to need to build up a span and then pass that off to these methods which will be forced to allocate a copy. For small spans of known size that can be done by the compiler on the stack, that's probably not a big deal. For larger spans, including those of unknown size due to the use of spreads, it's a bigger deal.

[CyrusNajmabadi]

I agree with Stephen. Best would be to open a proposal directly at csharplang. It's possible this might be small enough to squeeze in. But that would be an @jaredpar question.

[eiriktsarpalis]

Thanks. Given there's consensus I'll transfer the issue to csharplang.

[CyrusNajmabadi]

@cston @RikkiGibson if we wanted to support arrays for the 'collection builder pattern' would this work be on the order of size as a bug fix? This is work I'd be willing to do.

[CyrusNajmabadi]

@eiriktsarpalis can you update the proposal to take arrays not spans.

[jnm2]

This is something I've been interested in, too. I've run into awkward situations where I wanted to spread into a ReadOnlyMemory method parameter and had to add an array cast.

[eiriktsarpalis]

can you update the proposal to take arrays not spans.

If I'm understanding this correctly there will be a future version of the compiler recognizing CollectionBuilderAttribute methods accepting arrays instead of span?

[stephentoub]

@eiriktsarpalis can you update the proposal to take arrays not spans.

It shouldn't need new methods: {ReadOnly}Memory already have constructors accepting T[].

[cston]

@CyrusNajmabadi,

if we wanted to support arrays for the 'collection builder pattern' would this work be on the order of size as a bug fix?

I think we'd need to review a proposal with language design to determine the scope of the change to collection builder support.

[jaredpar]

This idea was considered by C# LDM at least once. My memory was that we decided to not go forward with this because Memory isn't a traditional collection. For example it doesn't implement IEnumerable<T>. I thought we basically rejected that but I'm having trouble finding the supporting notes. The best I could find is that we sent it back to working group for further thoughts.

It's possible this might be small enough to squeeze in. But that would be an @jaredpar question.

We're over booked for C# 13 at this point. Earliest would be C# 14.

[jnm2]

For example it doesn't implement IEnumerable<T>.

Since then, we've dropped that requirement:
#7744
https://github.com/dotnet/csharplang/blob/main/meetings/2024/LDM-2024-01-31.md#relax-enumerable-requirement-for-collection-expressions

[CyrusNajmabadi]

If I'm understanding this correctly there will be a future version of the compiler recognizing CollectionBuilderAttribute methods accepting arrays instead of span?

That's what i want this proposal to be :)

[jnm2]

While considering supporting T[], should we look ahead to IEnumerable<T> as well? That will enable us to use existing dictionary CreateRange methods.

[RikkiGibson]

@cston @RikkiGibson if we wanted to support arrays for the 'collection builder pattern' would this work be on the order of size as a bug fix? This is work I'd be willing to do.

Whether it is "bug fix level", I am not sure. It needs spec changes, LDM approval, and tests. I guess I would expect the PR for it to be a few thousand lines. It could probably be done right in main.

It seems reasonable to add support for MyCollection<T> Create(T[] array) as well as for "array interfaces" like IEnumerable<T>, which we will just fulfill by making an array anyways. The language should assume that ownership of the array is transferring to MyCollection. In other words, the language shouldn't attempt to use the array being passed to Create for any other purpose afterwards.

The only thing I wouldn't want to add is MyCollectionBuilderType Create(MyOtherCollectionBuilderType). Just seems like undesirable complexity.

[CyrusNajmabadi]

Note: teh original spec allowed for this with builders. Our LDM decisions where that we didn't like how broad this was getting in scope, and we wanted to keep the allowed patterns small here unless necessary. I like the idea of keeping the set of supported cases minimal. Specifically, keeping it just with arrays/spans seems ideal to me as they're the lowest level primitives for creating contiguous data (either on the heap or stack) and allowing the builder to take over from there.

Also, it seems like it would keep costs much lower if we limit it just to arrays.

[RikkiGibson]

Also, it seems like it would keep costs much lower if we limit it just to arrays.

works for me!

[jaredpar]

Whether it is "bug fix level", I am not sure. It needs spec changes, LDM approval, and tests. I guess I would expect the PR for it to be a few thousand lines. It could probably be done right in main.

We are over booked for C# 13 right now. I've already had to cut several items. Think this should be a candidate for 14 but do not want to add additional stress by putting this into 13.

[alrz]

The workaround seems to be new([]) which is not too bad because you need a target type anyways.