Move (ReadOnly)Span.DangerousGetPinnableReference to MemoryMarshal

[karelz]

Separated from https://github.com/dotnet/corefx/issues/23879#issuecomment-341482899

We should also rename it as part of the move. Pinnable does not make sense if it is not going to be used for pinning. And Dangerous can be omitted too because of methods under System.Runtime.InteropServices are dangerous by convention.

So what about:

namespace System.Runtime.InteropServices
{
    public static class MemoryMarshal
    {
        public static readonly ref T GetReference<T>(in ReadOnlySpan<T> span);
        public static ref T GetReference<T>(in Span<T> span);
    }
}

[terrajobst]

Video

Looks good as proposed. We should also remove the corresponding GetDangerousXxxx() instance methods.

@jkotas why are they marked as in? Does it make a difference?

[jkotas]

Not sure. We should measure.

in mimics what happens to the instance method method today - the struct instance methods take reference to the structs, not copies.

[karelz]

FYI: The API review discussion was recorded - see https://youtu.be/o5JFZtgaJLs?t=76 (17 min duration)

[davidfowl]

Can we do this only after we get pinning support in the compiler?

[jkotas]

Can we do this only after we get pinning support in the compiler?

This issue is primarily about adding the new non-Dangerous APIs. The new APIs are orthogonal to the compiler pinning support.

We should also remove the corresponding GetDangerousXxxx() instance methods.

This should be attacked separately, once the compiler pinning support is figured out. The current proposal is that these methods will stay and they will be used by the compiler pinning support: dotnet/csharplang#1100

[ahsonkhan]

Changing the API to the following based on feedback from @VSadov, @jkotas:
dotnet/coreclr#15532 (comment)

public static class MemoryMarshal
{
        public static ref T GetReference<T>(in ReadOnlySpan<T> span);
        public static ref T GetReference<T>(in Span<T> span);
}

[davidfowl]

Can you not remove the instance methods until ASP.NET is updated as well? It would be nice to avoid breaking everything all at once.

/cc @BrennanConroy @pakrym

[ahsonkhan]

I have updated corefx/corert/coreclr. What is left is aspnet and corefxlab. On the aspnet side, which repos would require updates? SignalR and Kestrel or are there others?

[ahsonkhan]

See aspnet/KestrelHttpServer#2222

Depends on a package update to aspnet/Universe to go through - aspnet/Universe#717

• SignalR doesn't use DangerousGetPinnableReference or DangerousTryGetArray (these APIs will be removed/changed).
• Kestrel doesn't use DangerousTryGetArray
• I couldn't find any other aspnet repo that relied on these two APIs.