Add ImmutableArray<T>.Builder.AsSpan

[alrz]

Background and Motivation

Before calling ToImmutable and making a copy, you may want to access the inner array as a Span. Unlike MoveToImmutable which also doesn't incur a copy, this does not require Count == Capacity as AsSpan can only expose elements up to Count.

Proposed API

namespace System.Collections.Immutable
{
    public partial struct ImmutableArray<T>
    {
        public sealed class Builder
        {
+            public Span<T> AsSpan();
        }
     }
}

The implementation is straightforward:

public Span<T> AsSpan() => new Span<T>(_elements, 0, this.Count);

We may want to add a similar member to other builders as well.

[ghost]

Tagging subscribers to this area: @eiriktsarpalis, @jeffhandley
See info in area-owners.md if you want to be subscribed.

[alrz]

There is a similar (internal) API System.Collections.Generic.ValueListBuilder<T>.AsSpan() which returns a ReadOnlySpan<T>. I don't think that's helpful because we're still in the builder, so modifications should be allowed.

[eiriktsarpalis]

Related to #21727. I feel the concerns raised there also apply here. And given that immutable builders are niche compared to List<T> I would expect that such an addition would not be accepted. cc @stephentoub

[alrz]

given that immutable builders are niche compared to List I would expect that such an addition would not be accepted

To give some context here, I wanted to use this in Roslyn's ArrayBuilder type (a pooled wrapper around IA<T>.Builder) for an efficient post-processing before I get to call ToImmutable/Free.

[alrz]

I think the main concern is that any modification to the builder after AsSpan call would be unsafe.

Is there any "unsafe" way to get the span then? so that if someone is aware of the consequence at least they could save a copy

[Joe4evr]

I recall there was some discussion to have methods like this declared in CollectionMarshal, so that it's something a dev has to go out of their way for to look and (in theory) be aware that they are doing somewhat unsafe things.

It could be considered further if the Caller-Must-Be-Unsafe analyzer (#31354) gets done and shipped, so that anyone who wants to call this method knows that the rules of an unsafe context apply.

[mayorovp]

This api will be unsound:

ImmutableArray<int>.Builder builder = ...;
builder[0] = 5;

var span = builder.AsSpan();
ImmutableArray<int> array = builder.MoveToImmutable();

Console.WriteLine(array[0]); // 5
span[0] = 6;
Console.WriteLine(array[0]); // 6

[alrz]

I think if we empty the original array just like MoveToImmutable all concerns are addressed. It is a MoveToImmutable but without Count == Capacity requirement.

Might as well rename to MoveToSpan?

public ReadOnlySpan<T> MoveToSpan()
{
    T[] temp = _elements;
    _elements = ImmutableArray<T>.Empty.array!;
    _count = 0;
    return new ReadOnlySpan<T>(temp, 0, Count);
}

[mayorovp]

But ImmutableArray<T>.Builder is a builder for ImmutableArray<T>, not for Span<T>...

[alrz]

It seems wasteful to make a builder just for span. They use the same underlying type: array. So to me it makes sense to have a helper like that.

This is more important now that MoveToImmutable requires the same Capacity as Count. That's a huge limitation when you want to avoid the copy.

[mayorovp]

Yes, it is wasteful to make a ImmutableArray<T>.Builder just for span.

[SingleAccretion]

Related: #37852 and #27061.

[alrz]

I think it would make more sense for a CollectionsMarshal overload as @Joe4evr suggested:

namespace System.Runtime.InteropServices
{
    public static class CollectionsMarshal
    {
        public static Span<T> AsSpan<T>(ImmutableArray<T>.Builder builder);
    }
}

I withdraw this proposal.