[RFCs FS-1051, FS-1052, FS-1053] support for span, readonly refs, byref-like structs

[dsyme]

Adding support for some C# 7.0-7.2 features:

RFCs

• https://github.com/fsharp/fslang-design/blob/master/RFCs/FS-1053-span.md

Simple samples compile, e.g. taken from here

open FSharp.NativeInterop
open System.Runtime.InteropServices

let SafeSum (bytes: Span<byte>) =
    let mutable sum = 0
    for i in 0 .. bytes.Length - 1 do 
        sum <- sum + int bytes.[i]
    sum

let test() = 
    // managed memory
    let arrayMemory = Array.zeroCreate<byte>(100)
    let arraySpan = new Span<byte>(arrayMemory);
    SafeSum arraySpan |> printfn "res = %d"

    // native memory
    let nativeMemory = Marshal.AllocHGlobal(100);
    let nativeSpan = new Span<byte>(nativeMemory.ToPointer(), 100);
    SafeSum nativeSpan |> printfn "res = %d"
    Marshal.FreeHGlobal (nativeMemory);

    // stack memory
    let mem = NativePtr.stackalloc<byte>(100)
    let mem2 = mem |> NativePtr.toVoidPtr
    let stackSpan = Span<byte>(mem2, 100)
    SafeSum stackSpan |> printfn "res = %d"

TODO

• factor out the cleanup to keep this minimal (and perhaps revert some of it)
• test ReadOnlyMemory and ReadOnlySpan
• design: byref returns should automatically dereference unless & is used. currently you must assign to a local and then use that local (where the use of the local does an automatic dereference)
• resolve all unresolved questions in RFC
• add positive tests
• add negative tests
• Fix this baseline https://github.com/Microsoft/visualfsharp/pull/4888/files#diff-24bfe33fd52123c01906c88689b77404R2
• Add positive tests for emit of modreq on inref parameters
• Consider design issue re stackalloc
• Fix bug in subsumption calls
• Improve an error message
• Fix assignment to return byref
• Fix assignment to byref using setter syntax
• Fix immediate return of span local
• Check for both IsByRefLike and Struct
• Review rules on Span safety and differentiate between byref and span safety rules, disallowing byref fields in IsByRefLike types.

[dsyme]

Right now proto is broken, am fixing that

Once this starts to work I'd greatly appreciate help in testing/proofing it. It's not a feature I currently need so I'm likely to miss things. We missed at least one thing with ref returns (mentioned above - we should just fix it since ref returns are currently very rare) and that was because it was not proofed enough

[cartermp]

cc @drvink and @zpodlovics who should have some use cases to throw at this

[dsyme]

@cartermp I think it's a matter of systematically translating C# tests and samples (as I did above).

[smoothdeveloper]

@dsyme @cartermp "Future of C#" video from Build 2018 has a section about C# refs:

https://youtu.be/QZ0rWLaMZeI?t=1214

using this ref as extension member

Consider making F# able to call C# "this ref extension methods", and possibly to define those as well.

ref return / ref locals

• A local variable can be a shared reference to another field / variable
• ref readonly return / ref readonly locals, not sure how the semantics will play out with our dealing with mutable/immutable bindings, I expect F# to at least be able to define ref readonly return methods
• escape analysis of taking a ref of something which goes out of scope

C# also added a new modifier beside out and ref: in, it acts the same as ref but ensure that the callee can't reassign the ref to a new value.

[dsyme]

@smoothdeveloper Thanks, yes, I've added those to the list of TODO items. Some of it is already done as part of F# 4.1 ref returns but there are more details to do. This series https://blogs.msdn.microsoft.com/mazhou/tag/c/ is a good guide

[zpodlovics]

Comment about local ref reassignment was moved to here:
fsharp/fslang-design#287

[TIHan]

Ok, so this should compile according to the C# examples:

type SpanLikeType = Span<int>

let m1 (x: byref<Span<int>>) (y: Span<byte>) =
    // this is all valid, unconcerned with stack-referring stuff
    let local = SpanLikeType()
    x <- local
    x

I can't do a type alias to Span, but I don't know if that makes sense to do.

[TIHan]

Ok, here are the samples from C#:

type SpanLikeType = Span<int>

let m1 (x: byref<Span<int>>) (y: Span<byte>) =
    // this is all valid, unconcerned with stack-referring stuff
    let local = SpanLikeType()
    x <- local
    x

let test1 (param1: byref<Span<int>>) (param2: Span<byte>) =
    let stackReferring1 = Span<byte>()
    let mutable stackReferring2 = Span<int>()

    // this is allowed
    stackReferring2 <- m1 &stackReferring2 stackReferring1

    // this is NOT allowed
    stackReferring2 <- m1 &param1 stackReferring1

    // this is NOT allowed
    param1 <- m1 &stackReferring2 stackReferring1

    // this is NOT allowed
    let param2 = stackReferring1.Slice(10)

    // this is allowed
    param1 <- Span<int>()

    // this is allowed
    stackReferring2 <- param1

let m2 (x: byref<Span<int>>) =
    // should compile
    &x

let test2 (param1: byref<Span<int>>) (param2: Span<byte>) =
    let stackReferring1 = Span<byte>()
    let mutable stackReferring2 = Span<int>()

    let stackReferring3 = &(m2 &stackReferring2)

    // this is allowed
    stackReferring3 <- m1 &stackReferring2 stackReferring1

    // this is allowed
    (m2 &stackReferring3) <- stackReferring2

    // this is NOT allowed
    (m1 &param1) <- stackReferring2

    // this is NOT allowed
    param1 <- stackReferring3

    // this is NOT allowed
    &stackReferring3

    // this is allowed
    //&param1 - uncomment to test return

Currently, we violate some of these. Hopefully I did the conversion correctly. Here are the C# examples https://github.com/dotnet/csharplang/blob/master/proposals/csharp-7.2/span-safety.md#examples

[TIHan]

I want to mention these rely on stack information, so this code:

    let stackReferring1 = Span<byte>()
    let mutable stackReferring2 = Span<int>()

Isn't accurate, we need to find something analogous to this:

   Span<byte> stackReferring1 = stackalloc byte[10];
   var stackReferring2 = new SpanLikeType(stackReferring1);

[dsyme]

OK, @TIHan and I have been over all remaining known cases and fixes and tests are now in for all of them.

[TIHan]

test Windows_NT Release_ci_part2 Build please

[TIHan]

More testing today, looking at inref<'T>. I found a few issues:

This code:

[<Struct>]
type TestMut =

    val mutable x : int

    member this.AnAction() =
        this.x <- 1

let testIn (m: inref<TestMut>) =
    m.x <- 1

let testAction (m: inref<TestMut>) =
    m.AnAction()
    printfn "%A" m.x

let test() =
    let x = TestMut()
    testIn (&x)
    testAction (&x)
    x

Should not compile due to m.x <- 1 because inref is readonly. I would also assume the AnAction call in this case would do a defensive copy and not change the value of ref, however, in the printfn call, it prints out 1 which it should be 0. Those are the bigger issues.

--

I also want to make note of some IL generated when calling a method that takes a ref, it's not super important or a big concern because the JIT optimizes this very well (identical if I were to do the C# equivelant). Here is the example:

[<Struct>]
type TestMut =

    val mutable x : int

    member this.AnAction() =
        this.x <- 1

[<MethodImpl(MethodImplOptions.NoInlining)>]
let testIn (m: inref<TestMut>) =
    Console.WriteLine(m.x)

[<MethodImpl(MethodImplOptions.NoInlining)>]
let testRef (m: byref<TestMut>) =
    Console.WriteLine(m.x)

let callTestIn() =
    let x = TestMut()
    testIn (&x)

let callTestInMutable() =
    let mutable x = TestMut()
    testIn (&x)

let callTestRefMutable() =
    let mutable x = TestMut()
    testRef (&x)

And here is the IL generated for callTestIn, callTestInMutable, and callTestRefMutable:

  .method public static void 
    callTestIn() cil managed 
  {
    .maxstack 3
    .locals init (
      [0] valuetype Program/TestMut V_0,
      [1] valuetype Program/TestMut V_1
    )

    // [32 5 - 32 47]
    IL_0000: ldloca.s     V_1
    IL_0002: initobj      Program/TestMut
    IL_0008: ldloc.1      // V_1
    IL_0009: stloc.0      // V_0
    IL_000a: ldloca.s     V_0
    IL_000c: call         void Program::testIn(valuetype Program/TestMut&)
    IL_0011: nop          
    IL_0012: ret          

  } // end of method Program::callTestIn

  .method public static void 
    callTestInMutable() cil managed 
  {
    .maxstack 3
    .locals init (
      [0] valuetype Program/TestMut V_0,
      [1] valuetype Program/TestMut V_1
    )

    // [37 5 - 37 47]
    IL_0000: ldloca.s     V_1
    IL_0002: initobj      Program/TestMut
    IL_0008: ldloc.1      // V_1
    IL_0009: stloc.0      // V_0
    IL_000a: ldloca.s     V_0
    IL_000c: call         void Program::testIn(valuetype Program/TestMut&)
    IL_0011: nop          
    IL_0012: ret          

  } // end of method Program::callTestInMutable

  .method public static void 
    callTestRefMutable() cil managed 
  {
    .maxstack 3
    .locals init (
      [0] valuetype Program/TestMut V_0,
      [1] valuetype Program/TestMut V_1
    )

    // [42 5 - 42 47]
    IL_0000: ldloca.s     V_1
    IL_0002: initobj      Program/TestMut
    IL_0008: ldloc.1      // V_1
    IL_0009: stloc.0      // V_0
    IL_000a: ldloca.s     V_0
    IL_000c: call         void Program::testRef(valuetype Program/TestMut&)
    IL_0011: nop          
    IL_0012: ret          

  } // end of method Program::callTestRefMutable

It looks like we have too many locals init here where as a C# equivalent code would look like this:

    .method public hidebysig static 
        void CallTestRef () cil managed 
    {
        // Method begins at RVA 0x209c
        // Code size 16 (0x10)
        .maxstack 1
        .locals init (
            [0] valuetype Beef/Hello
        )

        IL_0000: ldloca.s 0
        IL_0002: initobj Beef/Hello
        IL_0008: ldloca.s 0
        IL_000a: call void Beef::TestRef(valuetype Beef/Hello&)
        IL_000f: ret
    } // end of method Beef::CallTestRef

But, looking at the JIT ASM for either, they generate the same ASM code. So it probably does not matter, just wanted to make note of it.

[TIHan]

Currently, tests are failing on the core/byrefs/test.fsx, I believe it is due to this code:

        let test() = 
            let addr = &f (C()) 
            addr <- addr + 1
            let addr = &f (ObjExpr()) 
            addr <- addr + 1
            check2 "cepojcwem16" 3 x

It's failing on this line let addr = &f (C()) with:
*
Error FS0412 A type instantiation involves a byref type. This is not permitted by the rules of Common IL.
*
And there are two of them.

[TIHan]

Another minor issue:

[<Struct>]
type TestMut =

    val mutable x : int

    static member Test(x: inref<TestMut>) = ()

    static member CallTest() =
        TestMut.Test(TestMut())

let testIn (m: inref<TestMut>) =
    ()

let callTestIn() =
    testIn (TestMut())

This doesn't compile due to this line: testIn (TestMut()). It looks like let-bound functions are not using the same rules as member functions when instantiating something and not be explicit when getting the address when calling a function that takes an inref. This could be a design decision for let-bound functions, but I think they should be the same as member functions. We should have consistency for refs across let-bound and member functions, otherwise it would be confusing.

[TIHan]

This is another minor issue:

[<Struct>]
type TestS =

    static member Test(x: outref<TestS>) = ()

let testIn (m: outref<TestS>) =
    ()

This should not compile and give you an error saying that your outref variable has not been assigned. This means we should be checking to make sure the outref variable gets assigned in all branches of the body of the function. Technically, we were able to do [<Out>] on a parameter before, but it never did do any checks as well.

Since the checks haven't been there before anyway, I don't think we have to include them in 4.5 - we could do them later if it requires a lot more work.

[dsyme]

Thanks. All fixes are in and being validated in CI.

This could be a design decision for let-bound functions,

Yes, it is by design. I've noted it in the RFC. I understand why you would expect the same conversions to take place, but we currently don't do any such conversions (e.g. F# lambda --> System.Func) on calls to let-bound functions, and I'm loathe to start doing them here , it would be better to make a systematic change here that took into account all conversions.

...out writing... Since the checks haven't been there before anyway, I don't think we have to include them in 4.5 - we could do them later if it requires a lot more work.

Yes, we don't make this check today and it won't be part of F# 4.5. I've noted that in the RFC

But, looking at the JIT ASM for either, they generate the same ASM code. So it probably does not matter, just wanted to make note of it.

Yes, it's ok, we generate some more locals but the JIT does a good job of them

[TIHan]

test Ubuntu16.04 Release_default Build please

[dsyme]

@dotnet-bot test Ubuntu16.04 Release_default Build please

[TIHan]

And we got it! Any bugs we find, we will thoroughly review with the C# team; but so far, I feel this is ready considering the massive testing we have done, especially for a personal project that uses this stuff heavily.