Runtime: Add explicit conversion APIs for Span to ROSpan and Memory to ROMemory

Created on 11 Oct 2017  Â·  6Comments  Â·  Source: dotnet/runtime

Rationale

From https://github.com/dotnet/corefx/issues/23862#issuecomment-328097809

```C#
public static implicit operator ReadOnlyMemory(Memory memory);
public static implicit operator ReadOnlySpan (Span span);

This is the only way to convert from `Span<T>` to `ReadOnlySpan<T> and `Memory<T>` to `ReadOnlyMemory<T>`. [Framework Design Guildelines say](https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/operator-overloads):

> ✓ CONSIDER providing methods with friendly names that correspond to each overloaded operator.

Specifically, F# does not support implicit operators, but allows calling them as `op_Implicit`. The only way to convert from `Memory<T>` to `ReadOnlyMemory<T>` in F# would be the following?

```F#
let memoryToReadOnlyMemory (memory : Memory<'a>) : ReadOnlyMemory<'a> =
    Memory<'a>.op_Implicit memory

Proposed API

Therefore, add these explicit conversion APIs to Span<T> and Memory<T>:
C# public static ReadOnlyMemory<T> AsReadOnlyMemory(this Memory<T> memory); public static ReadOnlySpan<T> AsReadOnlySpan(this Span<T> span);

cc @svick, @KrzysztofCwalina, @jkotas, @terrajobst

api-approved area-System.Memory easy up-for-grabs

All 6 comments

@terrajobst, marking this one as approved as per the API review today (that got cut off near the end).

First time contributor, I can take this.

I think a conversion from Span to ROSpan is reasonable.

I don't think we should add a conversion from Memory to Span. It's relatively expensive and we want people to store the span into a local when they get it from Memory. It's especially expensive when Memory points to OwnedMemory.

BTW, this is the main reason I am not super thrilled about all these requests to add APIs to Memory. People will just call these APIs without realizing that it would be faster to get span out once and then use span APIs directly.

I don't think we should add a conversion from Memory to Span.

Can you please clarify which API proposals is suggesting we do this?

@zaytsev-victor thanks for your contribution. I added you as Collaborator. Please ping me when you accept, I will be able to assign you the issue when you do.
Pro-tip: Collaborators are auto-subscribed to all repo notifications. There is plenty of them. We recommend everyone to disable them and receive just 'mention' notifications.

@karelz I've accept invitation, thanks

Was this page helpful?
0 / 5 - 0 ratings

Related issues

GitAntoinee picture GitAntoinee  Â·  3Comments

matty-hall picture matty-hall  Â·  3Comments

jzabroski picture jzabroski  Â·  3Comments

omariom picture omariom  Â·  3Comments

chunseoklee picture chunseoklee  Â·  3Comments