Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Span<'t> pinning #761

Open
jwosty opened this issue Jun 25, 2019 · 11 comments

Comments

Projects
None yet
6 participants
@jwosty
Copy link
Contributor

commented Jun 25, 2019

Allow Span pinning

There should be some way to pin Spans and take their native addresses for interop purposes. Currently you just can't do this in F# as far as I know. In C# you can just use the fixed statement, but F# only supports a limited set of constructs, none of which you can get to work with Spans.

I propose that at the very least, the fixed keyword should be extended to support Span<'t> directly. Ideally it would be extended to support anything that supports GetPinnableReference, like C#'s fixed statement does.

Pros and Cons

Advantages:

  • You can use spans in native interop situations without being forced to copy it to an array first
  • Feature parity with C#

Disadvantages:

  • It's work

Extra information

Estimated cost (XS, S, M, L, XL, XXL): M? Just a guess.

Related suggestions: (put links to related suggestions here)

Affidavit (please submit!)

Please tick this by placing a cross in the box:

  • This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it.

Please tick all that apply:

  • This is not a breaking change to the F# language design
  • I or my company would be willing to help implement and/or test this
@cartermp

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

I think this should definitely be allowed.

@jwosty

This comment has been minimized.

Copy link
Contributor Author

commented Jun 25, 2019

Come to think of it, fixed should probably also support Memory<'t> directly too.

@cartermp do you think this addition should only concern itself with Span/Memory, or should it be a more general expansion to things that have GetPinnableReference?

@Happypig375

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2019

FSharp.Core currently doesn't depend on System.Memory. Using statically resolved type parameters with member constraints would be the way to go. Therefore, this should be a more general expansion to things that have GetPinnableReference.

@cartermp

This comment has been minimized.

Copy link
Member

commented Jun 26, 2019

@jwosty We should match what C# does here and lookfor the GetPinnableReference member. @TIHan can be of help in figuring out how to determine that.

@TIHan

This comment has been minimized.

Copy link

commented Jun 26, 2019

I support this. This is a very reasonable request.

@jwosty

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2019

I say we should also support arbitrary pinning of refs. For example, C# lets you do the following (shamelessly copied from the C# docs):

class Point 
{ 
    public int x;
    public int y; 
}

unsafe private static void ModifyFixedStorage()
{
    // Variable pt is a managed variable, subject to garbage collection.
    Point pt = new Point();

    // Using fixed allows the address of pt members to be taken,
    // and "pins" pt so that it is not relocated.

    fixed (int* p = &pt.x)
    {
        *p = 1;
    }
}

That way, it's a more general construct that you can use to workaround anything that's not supported directly by fixed, like in that stackoverflow answser I linked to in the OP.

@dsyme

This comment has been minimized.

Copy link
Collaborator

commented Jul 1, 2019

Yup, approved as far as I'm concerned.

@jwosty

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2019

@dsyme what do you think about supporting GetPinnableReference vs supporting any ref? Would an RFC be approved if it contained both?

@dsyme

This comment has been minimized.

Copy link
Collaborator

commented Jul 2, 2019

@dsyme what do you think about supporting GetPinnableReference vs supporting any ref? Would an RFC be approved if it contained both?

I don't see why not - @TIHan do you agree?

@davidglassborow

This comment has been minimized.

Copy link

commented Jul 9, 2019

Here is the csharp pattern based fixed proposal that discusses GetPinnableReference

@TIHan

This comment has been minimized.

Copy link

commented Jul 9, 2019

Based on what I know right now, supporting both sounds very reasonable to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.