Cleanup after removal of MemoryExtensions As* api.

[KrzysztofCwalina]

In the Framework, there are many types that can be converted into Span<T>, ReadOnlySpan, Memory<T>, and ReadOnlyMemory<T> (a.k.a. slice types).

The conversions are implemented as:

• constructors of slice types, e.g. Span<T> ctor taking T[] parameter
• cast operators from source type to slice type, e.g. implicit operator Memory<T> (T[] array)
• As<slice_type> extensions methods, e.g. static ReadOnlySpan<char> AsReadOnlySpan(this string text)

Unfortunately, the set of these conversions is not very consistent. For example, AsReadOnlySpan method converting strings to ReadOnlySpan<char> has three overloads:

public static System.ReadOnlySpan<char> AsReadOnlySpan(this string text);
public static System.ReadOnlySpan<char> AsReadOnlySpan(this string text, int start);
public static System.ReadOnlySpan<char> AsReadOnlySpan(this string text, int start, int length);

But logically equivalent T[] to Span<T> conversion method has only one overload:

public static System.Span<T> AsSpan<T>(this T[] array) { throw null; }

This means string can be sliced using the following code:

var slice = str.AsReadOnlySpan(5, 10);

... but to slice an array, developers have to write:

var slice = arr.AsSpan().Slice(5, 10);

This proposal outlines guidelines we could adopt to make the APIs more consistent and easier to use:

1. Always provide three overloads of As<slice_type> extension or instance methods. Such extension method will be the main conversion API (i.e. other conversions like ctors and casts are just icing on the cake).

public static <slice_type> As<slice_type>(this <source_type> value) ;
public static <slice_type> As<slice_type>(this <source_type> value, int start);
public static <slice_type> As<slice_type>(this <source_type> value, int start, int length);

2. If a type can be converted to both heapable slices and by-ref slices, provide extension methods for both.

public static Span<T> AsSpan(this T[] value) ;
public static Span<T> AsSpan(this T[] value, int start);
public static Span<T> AsSpan(this T[] value, int start, int length);
public static Memory<T> AsMemory(this T[] value) ;
public static Memory<T> AsMemory(this T[] value, int start);
public static Memory<T> AsMemory(this T[] value, int start, int length);

3. Do not provide conversions to both read-only and read/write slice types. e.g T[] will have conversions to only ``SpanandMemory```, not to ```ReadOnlySpan``` or ```ReadOnlyMemory```. Slice types provide cast from r/w versions to read-only versions.

4. Do provide implicit casts between types if appropriate.

Not sure if we want, but we should discuss:

5. Limit constructors to the longest (most flexible) overload.

6. Provide ```AsReadOnly()`` methods on r/w/ slice types.

7. Skip ReadOnly from conversion method names if the source type is already read-only. For example, string can be converted only to ReadOnlySpan<char> and so the As<slice_type> method should be called AsSpan, not AsReadOnlySpan.

The resulting changes are:

public static partial class MemoryExtensions
{
	// * ADD *

        // String
	public static System.ReadOnlyMemory<char> AsMemory(this string text) { throw null; }
        public static System.ReadOnlyMemory<char> AsMemory(this string text, int start) { throw null; }
        public static System.ReadOnlyMemory<char> AsMemory(this string text, int start, int length) { throw null; }

        public static System.ReadOnlySpan<char> AsSpan(this string text) { throw null; }
        public static System.ReadOnlySpan<char> AsSpan(this string text, int start) { throw null; }
        public static System.ReadOnlySpan<char> AsSpan(this string text, int start, int length) { throw null; }

        // T[]
	public static System.Memory<T> AsMemory(this T[] array) { throw null; }
        public static System.Memory<T> AsMemory(this T[] array, int start) { throw null; }
        public static System.Memory<T> AsMemory(tthis T[] array, int start, int length) { throw null; }

        public static System.Span<T> AsSpan(this T[] array) { throw null; } // this already exist
        public static System.Span<T> AsSpan(this T[] array, int start) { throw null; }
        public static System.Span<T> AsSpan(this T[] array, int start, int length) { throw null; }

	// ArraySegment
	public static System.Memory<T> AsMemory(this ArraySegment<T> segment) { throw null; }
        public static System.Memory<T> AsMemory(this ArraySegment<T> segment, int start) { throw null; }
        public static System.Memory<T> AsMemory(this ArraySegment<T> segment, int start, int length) { throw null; }

        public static System.Span<T> AsSpan(this ArraySegment<T> segment) { throw null; } // this already exist, just rename the parameter
        public static System.Span<T> AsSpan(this ArraySegment<T> segment, int start) { throw null; }
        public static System.Span<T> AsSpan(this ArraySegment<T> segment, int start, int length) { throw null; }

        // * REMOVE *

	// these actually just get renamed to AsMemory/Span (see above)
        public static System.ReadOnlyMemory<char> AsReadOnlyMemory(this string text) { throw null; }
        public static System.ReadOnlyMemory<char> AsReadOnlyMemory(this string text, int start) { throw null; }
        public static System.ReadOnlyMemory<char> AsReadOnlyMemory(this string text, int start, int length) { throw null; }
        public static System.ReadOnlySpan<char> AsReadOnlySpan(this string text) { throw null; }
        public static System.ReadOnlySpan<char> AsReadOnlySpan(this string text, int start) { throw null; }
        public static System.ReadOnlySpan<char> AsReadOnlySpan(this string text, int start, int length) { throw null; }

        // remove and simply use the casts, e.g. from Memory<T> to ReadOnlyMemory<T> instead
	public static System.ReadOnlyMemory<T> AsReadOnlyMemory<T>(this System.Memory<T> memory) { throw null; }
        public static System.ReadOnlySpan<T> AsReadOnlySpan<T>(this System.Span<T> span) { throw null; }	
        public static System.ReadOnlySpan<T> AsReadOnlySpan<T>(this System.ArraySegment<T> arraySegment) { throw null; }
        public static System.ReadOnlySpan<T> AsReadOnlySpan<T>(this T[] array) { throw null; }       
}

[KrzysztofCwalina]

cc: @ahsonkhan, @joshfree, @stephentoub

[stephentoub]

We've got the Span ctor, a cast to Span, and an AsSpan method:

T[] array = ...;
Span<T> s1 = new Span<T>(array);
Span<T> s2 = array;
Span<T> s3 = array.AsSpan();

And with this, we'll have the Memory ctor, a cast to Memory, and an AsMemory method:

T[] array = ...;
Memory<T> m1 = new Memory<T>(array);
Memory<T> m2 = array;
Memory<T> m3 = array.AsMemory();

For both Span and Memory, do we need all three? I don't have a strong opinion, I just want to make sure we're doing this thoughtfully. Do we need these AsXx methods?

[jkotas]

• Similar type of inconsistency between string and everything else: We have 3 variants of AsReadOnlySpan for strings, but not for anything else. Should we have just the first one?

ReadOnlySpan<char> AsReadOnlySpan(this string text)
ReadOnlySpan<char> AsReadOnlySpan(this string text, int start)
ReadOnlySpan<char> AsReadOnlySpan(this string text, int start, int length)

[ahsonkhan]

We should add AsMemory method to MemoryExtensions class.

This API was rejected in a previous API review: https://github.com/dotnet/corefx/issues/23862

FYI: The API review discussion was recorded - see https://youtu.be/bHwCPVNQLwo?t=333 (37 min duration)

If we are going to add AsMemory, we probably want to add AsReadOnlyMemory as well.

The reason we added AsSpan initially was to enable the builder pattern style of coding to easily and cleanly chain methods together. Otherwise, we would have code like this, which is awkward to read (and write):
https://github.com/dotnet/corefxlab/blob/010238eb489ccfb04a47774cc606653afb82464a/src/System.Azure.Experimental/System/Azure/Sha256.Unix.cs#L33

new Span<byte>(hash).CopyTo(buffer); // hash.AsSpan().CopyTo(buffer);

https://github.com/dotnet/corefxlab/blob/4bf5f6a527c109c888a78ff8b2593ee631f33126/samples/LowAllocationWebServer/LowAllocationWebServerLibrary/TcpConnectionFormatter.cs#L58

toSend = _buffer.AsSpan().Slice(ChunkPrefixSize, _written);

https://github.com/dotnet/corefxlab/blob/d197075cf865def8da82eabecab1c91f595c587f/src/System.Buffers.Experimental/System/Buffers/Sequences/ReadWriteBytes.cs#L248

array.AsSpan().Slice(_startIndex, length).CopyTo(buffer);

https://github.com/dotnet/corefxlab/blob/d197075cf865def8da82eabecab1c91f595c587f/tests/System.Text.Primitives.Tests/Encoding/Utf8Utf16ConversionTests.cs#L234

return codePoints.AsSpan().AsBytes().ToArray();

https://github.com/aspnet/SignalR/blob/a0c47c0c664a1e6c5b1bc669592d25bb4e68d186/test/Microsoft.AspNetCore.SignalR.Tests/EndToEndTests.cs#L93

 Assert.Equal(bytes, buffer.Array.AsSpan().Slice(0, result.Count).ToArray());

It also helps in ASP.NET (for type inference) where var is used in a lot of places:
https://github.com/aspnet/SignalR/blob/a9d643a93e5b388984e208c3664366895baa4449/src/Microsoft.AspNetCore.SignalR.Common/Internal/Formatters/BinaryMessageFormatter.cs#L33-L34

var buffer = ArrayPool<byte>.Shared.Rent(lenNumBytes + payload.Length);
var bufferSpan = buffer.AsSpan(); // otherwise, figure out type T of buffer and then call new Span<T>(buffer);

cc @davidfowl, @pakrym

[karelz]

API review: We would like to see a complete proposal.

[KrzysztofCwalina]

I updated the first comment in this issue with more complete proposal. We should discuss today.

[terrajobst]

Video

Looks good as proposed. Regarding the open issues at the end:

5. Limit constructors to the longest (most flexible) overload.. Makes sense, we want the .ctors to be largely plumbing. However, we don't want to lose perf due to excessive argument validation. We'll add overloads as needed for perf, but not usability.

6. Provide AsReadOnly() methods on r/w/ slice types.. We agreed to not adding these methods.

7. Skip ReadOnly from conversion method names if the source type is already read-only. For example, string can be converted only to ReadOnlySpan<char> and so the As<slice_type> method should be called AsSpan, not AsReadOnlySpan. Makes sense.

[benaadams]

7. Skip ReadOnly from conversion method names if the source type is already read-only. For example, string can be converted only to ReadOnlySpan<char> and so the As<slice_type> method should be called AsSpan, not AsReadOnlySpan.

I like this. If you are fussy about the return type you won't be using var anyway so will be writing; so its clear anyway

ReadOnlySpan<char> sp = "hello".AsSpan()

What do you gain with

ReadOnlySpan<char> sp = "hello".AsReadOnlySpan()

Not much really; and the terse crowd will be unhappy they can't do

var sp  = "hello".AsSpan(); 

but have to do

var sp  = "hello".AsReadOnlySpan(); 

Also it means all the .As... types will be consistent and in the same place; rather than being .AsR.. or .AsS depending on type.

[KrzysFR]

Regarding the As<slice_type>(this <source_type> value, int start) overload (with a single int): when reading code like byte_array.AsSpan(5), my first instinct would be to think that it will create a span with the first 5 bytes of the buffer, while here it will skip the first 5 bytes.

A typical scenario would be when slicing a pooled buffer that has been filled with data, and creating a span from it and passing it to some parsing method (in this case, the data typically starts at offset 0):

byte[] buffer = /* allocate from some pool */;
int read = ReadOrFillSomeBytes(buffer, ...); //stream, serializer, ...
Span<byte> span = buffer.AsSpan(read); //**BUG**: would actually _skip_ the section with the data!

I could see myself making this mistake a lot, and always needing to change it to buffer.AsSpan(0, read) (which is a different overload, and would waste some CPU cycles checking that offset 0 is non-negative and inside the array).

So what would be the scenario where you would call .AsSpan<byte>(this byte[], int start)?

I think that when people are starting from a byte[], they are probably slicing a pooled buffer, that has been filled with data starting at offset 0. They would probably look for a helper method that only take the size of the span.

I also see that people already working on a Span<byte> are probably parsing or consuming it, and are looking for a way to drop the first N bytes. But then they would call Slice(N), not (not_a_span).AsSpan(N)?

As I understand it, these overloads are intended to be shortcuts for As<slice_type>(this <source_type>).Slice(...), which makes at lot of sense. But since the As... prefix is less explicit than Substring() or Slice(), it may not be immediately evident to new comers?

[KrzysztofCwalina]

@KrzysFR, it's possible that the single-int AsSpan overload will be used less often than others (or very rarely) , but I think there is value in consistency here. We are not going to remove the single int overloads in the other places (e.g. Slice methods), and so I think people will just have to learn what the single int overloads mean.

[jkotas]

The C# is about to add first class slicing syntax: dotnet/csharplang#185. Once that happens, I expect that there will be a strong preference to write either array.AsSpan()[..5] or array.AsSpan()[5..] instead of the multi-argument AsSpan APIs.

[KrzysztofCwalina]

We could also add AsSpan(Range) overload and so people would write array.AsSpan(..5) and array.AsSpan(5..)

@terrajobst