[FS-1044] Add NativePtr.toByRef

[mjmckp]

The toByref method fills a gap in the NativePtr module - converting typed native pointers to byref pointers. This is necessary, e.g., to call methods such as Thread.VolatileRead or Thread.VolatileWrite. This method was originally proposed here #409 but wasn't incorporated into the NativePtr module.

RFC https://github.com/fsharp/fslang-design/blob/master/RFCs/FS-1044-add-nativeptr-tobyref.md

[msftclas]

All CLA requirements met.

[abelbraaksma]

I think it is a good addition 👍, but I'd suggest to name it toByRef in stead. If I look at the code base, that seems to be the common nomenclature (i.e., compare System.MarshalByRefObject or member __.IsByRef in Symbols.fs).

I don't know whether the compiled name should include the suffix Inlined?

[mjmckp]

@abelbraaksma I have changed the name as suggested. I included the suffix Inlined for the compiled name to be consistent with the compiled name for all the other inline methods in the module.

[abelbraaksma]

Before @dsyme or @KevinRansom can merge this, you should probably fix the tests. I didn't check, but perhaps the surface area changed (there's a test that deliberately fails each time a public method is added, changed, removed)? I am not 100% sure what is required to add public methods to the core in terms of procedure.

[mjmckp]

Thanks @abelbraaksma, I had a look through the tests and there doesn't appear to be a single place in which the NativePtr module is tested, would you mind suggesting a location? I was thinking of just a basic read/write test on the resulting byref for a few core types, would that be sufficient?

[dsyme]

Some tests come up in this search: https://github.com/Microsoft/visualfsharp/search?utf8=%E2%9C%93&q=NativePtr&type=

[mjmckp]

I've updated the tests, thanks. No idea why the CI builds are failing, tho.

[mjmckp]

There is a similar method in System.Runtime.CompilerServices.Unsafe here, which has a nontrivial implementation for non .NET core (does a round-trip via a local). Is there any chance the same thing applies here?

[realvictorprm]

@dotnet-bot test this please

[realvictorprm]

Triggering test because details ain't available.

[realvictorprm]

@mjmckp please rename the PR name to respect the changed naming.

[realvictorprm]

@dsyme all Checks have passed. Is this ready to merge now?

@mjmckp thanks!

[realvictorprm]

@KevinRansom F# 4.3 ?

[dsyme]

Approved. But this should have a tracking RFC too

@KevinRansom F# SDK 4.3 ?

@realvictorprm "F# SDK 4.3" means F# 4.1 with FSharp.Core 4.4.3.0 and updated compiler

[dsyme]

RFC is here https://github.com/fsharp/fslang-design/blob/master/RFCs/FS-1044-add-nativeptr-tobyref.md

[KevinRansom]

Thanks for this contribution