RFCs FS-1050, FS-1051, FS-1052 discussion (Span, IsByRefLike, IsReadOnly) #287
Comments
|
This comment was moved from the pull request dotnet/fsharp#4888 Please note ref local reassignment is now allowed in C#: dotnet/csharplang#933 An another ref feature is conditional references, this seems possible to express already (at least in compiled code) unless it used with FSI: FSI session: |
|
Well, F# struct are assumed as readonly, but it's not really. At least the whole struct could be replaced from a method (I saw an evil example earlier in a C# as this was the reason why readonly structs are introduced). I guess similar replacement is possible with struct tuples struct records any other struct types too. Please note readonly and immutable are not the same, you can have a readonly reference to a mutable thing. open System
open System.Runtime.InteropServices
open System.Runtime.CompilerServices
[<Struct>]
type S =
val Value: int
new(v) = { Value=v }
member this.Replace(newValue: S) =
this <- newValue
[<IsReadOnly;Struct>]
type ROS =
val Value: int
new(v) = { Value=v }
member this.Replace(newValue: ROS) =
this <- newValue
let test() =
let s1 = S(1)
s1.Replace(S(2))
printfn "S1: %d" s1.Value
let ros1 = ROS(1)
ros1.Replace(ROS(2))
printfn "ROS1: %d" ros1.Value
let test2() =
let mutable ms1 = S(1)
ms1.Replace(S(2))
printfn "mutable S1: %d" ms1.Value
let mutable mros1 = ROS(1)
mros1.Replace(ROS(2))
printfn "mutable ROS1: %d" mros1.Value
test()
test2()
However introducing the readonly structs will fix this, the most important thing for this is to prevent struct copy for readonly/immutable values by allowing passing readonly references on readonly/immutable things (CanTakeAddressOfImmutableVal) Would it be possible to have different type (or type alias or constraint) for readonly references? How the function method signatures will (or should?) reflect these type restrictions? |
|
@zpodlovics Thanks for helping with this, it's really useful having someone providing evil examples: The main initial aim is to allow effective consumption of APIs using The main niggles left to sort out are indeed about There is also a set of questions about how many of the constraints/conditions we check for new declarations of ref structs and explicit uses of You're correct that F# structs are not always readonly/immutable.
For (2) I'm not sure if we will be able to make a change to make the Currently in the prototype, adding
Just to mention that that limitation is just with top-level bindings, you can do this in a function: let f(arr1: int[], arr2: int[]) =
let x = if true then &arr1.[0] else &arr2.[0]
x <- 1 |
|
@dsyme The byref capability design looks really good, especially with the inref outref aliases! Immediately reminded me to Pony capablity design [1] [2]. In fact it's soo good, that it may worth adopting some "ideas" from there. Usually the more restriction the type have the more optimization opportunity the compiler will have. It would be really good to have some kind of constraints to represent IsReadOnly and ByRefLike (and 'blittable' types - similar to 'unmanaged' constraints but it's more stricter than that [3] [4] as it does not allow reference fields), even if the compiler optimizations not fully there yet. However the user codes could immediately profit from these type restrictions. These constraint could allow the developers to write tight generic code without using reflection "magic" to checks whether if these constraints holds. Custom constraints would be also good but cannot exploited by the compiler to provide even more performant code. Constraints proposal related to Span<'T>: [<Struct>]
[<StructLayout(LayoutKind.Sequential, Pack=1, Size=128)
[<IsReadOnly>]
type PaddedHistogram4 =
val Count1: int64
val Count2: int64
val Count3: int64
val Count4: int64
let msg<'T where 'T: readonly and 'T: blittable and 'T: packed and 'T: byreflike > (v: inbyref<'T>, host: RDMACloudHost) =The readonly constraint required to make sure the value raw "internals" will change during the transport call, the blittable and packed constraint require to make sure it represented in the same was on all host (within the same endiannes, no bigendian/littleendian conversion) and the byref like attribute required to make sure there is no heap allocation will happen with the type.
I would be suprised if anybody use the this replacement I showed in the 'evil' example. Inferring readonly could introduce snowflake/wavefront like changes in the code. If the change is non-readonly -> readonly (promote to readonly), it's good. However when readonly demoted to non-readonly, would be a disaster. Small change in the code (assuming proper functional type composition or in an external library), and suddenly most of the generated code path runs on a less/non-optimized code path with lot's of defensive struct copy. This would be a nightmare for developing performance critical code. In performance critical code predictability is everything. Yes, I can and I will annotate my types with readonly attribute, but what about the F# library code and existing library code out there that I have no way to control? This is why the readonly, byreflike, blittable, packed constraints would be important - if something changes in other libraries at least I would be immediately notified on compilation if something will go wrong. If it possible/feasible to restrict - the assumed readonly/immutable F# types should really be readonly/immutable added explictly with these types. eg.: struct tupes, struct record types.
It is critical to have byref struct with the some control about it as a plain struct type. For example I can use sequential layout with Pack=1 to represent basic blittable types / packets / messages. I can use the Explicit layout with Pack=1 to represent the (overlapped) union of these types / packets / messages. In order to avoid cache false sharing I can use sequential/explicit fields combined with Size attribute. So if possible I would be really happy to use the byref struct layout control and implement everything in pure F# (instead of using C# code for some performance critical code that F# cannot express...).
I would be more happy to have span support and have this check in a later F# version, than delaying the span support for later version.
I would be more happy to have span support and have this check in a later F# version, than delaying the span support for later version. [1] https://blog.acolyer.org/2016/02/17/deny-capabilities/ |
|
+1 for allowing support for performant extension members in F#, because extension members already used in several places eg.: SRTP The second attribute syntax looks a bit more familiar [1] and I guess for C# compatible extension methods F# may (if it represented in the same way in C#, I did not checked this) already have a way to express the ref struct extension members: [<Extension>]
type ExtraCSharpStyleExtensionMethodsInFSharp() =
[<Extension>]
static member inline Sum(x: inref<MyStruct>) = x.Field1 + x.Field2But I do not know the real-world usability of these F# created C# compatible extension members in F# code and how well it works in type inference, SRTPs, etc. Therefore native F# extension methods may still preferred for this purpose. [1] https://docs.microsoft.com/en-us/dotnet/fsharp/language-reference/type-extensions |
Thanks, yes, I'm very happy with how this turned out. It's tempting to try to generalise the mechanism to sets of attributes on tags on types, (ala units of measure but with set semantics instead of kgm = mkg abelian group semantics).
Yes, adding capabilities like this is common in the ML/OCaml tradition. And I work with the wonderful @sylvanc, the Pony designer, who is at Microsoft Research Cambridge
The ideas in Pony dealing with concurrency and capabilities are great but would have to be slightly separate or additional to this. I'll talk to Sylvan about it to check we're not doing anything silly and we're not closing the door to adding further capabilities later.
ByRefLike types aren't usable as generic type parameters - not even for inline code. I'm not sure what to do about constraints for other things. We'll first have to consume the C# constraints being added recently.
The above thing about "Sequential" was taken from some C# design note I read. But we just have to do whatever C# does here.
This is taken from C# design note as well. We don't have any special compiler rule either way |
|
Update: moved from fsharp/fslang-suggestions#648 (comment) How the overload resolution interop supposed to work between F# -> C# and C# -> F#? Original Source: Example: using System;
namespace inattributeoverload
{
public class C
{
public void A(int a)
{
Console.WriteLine("int a");
}
public void A(in int a)
{
Console.WriteLine("in int a");
}
}
class Program
{
static void Main(string[] args)
{
Console.WriteLine("Hello World!");
C c = new C();
int x = 1;
c.A(in x); // A(in int)
//c.A(x); // A(int)
}
}
}Right now when the second And I have noticed that the generated IL callwith will pass the x argument with an |
|
Would it be possible to have a "non extractive/non-stack-allocating" (does not allocate the "extracted" struct on stack when accessing these readonly struct cases / patterns) way to do pattern matching in F#? Right now the "extraction" reference type patterns / small value types seems cheap, but larger value types could be costly to do pattern maching (even if the "extracted" cases are unused). What I mean by "non-extractive/non-stack-allocating" pattern maching on readonly types? The syntax is ad-hoc and fictional (probably better way to express it is exists). Consider the following example, when the match expression will not allocate the struct case on the stack as it match BigStructDU.Case1 because it used as an inref type. let matchBigStructDUByRef (x: BigStructDU inref) (v: int) =
match x with
| BigStructDU.Case1 inref _ -> Baseline.Invoke(v,1)
| BigStructDU.Case2 inref _ -> Baseline.Invoke(v,2)
| BigStructDU.Case3 inref _ -> Baseline.Invoke(v,3)
| BigStructDU.Case4 inref _ -> Baseline.Invoke(v,4)
| BigStructDU.Case5 inref _ -> Baseline.Invoke(v,5)
| BigStructDU.Case6 inref _ -> Baseline.Invoke(v,6)
| BigStructDU.Case7 inref _ -> Baseline.Invoke(v,7)
| BigStructDU.Case8 inref _ -> Baseline.Invoke(v,8)
| BigStructDU.Case9 inref _ -> Baseline.Invoke(v,9)
| BigStructDU.Case10 inref _ -> Baseline.Invoke(v,10)
From the RFC: As pattern matching is in the core of most F# code, this could be greatly improve the performance of most of the exsting F# codebase with a few changes or without a few changes (if the compier could infer it). To demonstrate the larger value types pattern matching existing overhead I have attached a benchmark project. Please feel free to use it for any purpose (eg.: benchmark, test,..). Pattern matching on larger value types could (and should) be as fast as reference or small value types. StructDU vs BigStructDU Benchmark and the result: .NET Core Versions: |
|
Design looks great, really impressed with the inoutref rules. Would a way to express |
Yes, it's possible. I'm not going to be able to do it in this PR however. Basically we need to rejig the pattern match compiler in F# so it can accept an LValue as its starting point rather than RValue. It's fairly conceptually straightforward but needs care.
There is already |
@zpodlovics Thank you for examining the generated code! I agree that this looks incorrect (though I think benign) and will double check the corresponding generated C# |
I don't get an error when your second line is enabled: Which exact version of the C# compiler are you using? |
|
Oddly I also don't get the |
|
@zpodlovics I am updating to Visual Studio 15.7.1 to check if behaviour changed (the above was with 15.6.7) |
|
Removed the 2.1.4 sdk as an experiment and reinstalled the 2.0.3, however the C# compiler will not recognize the in construct: Earlier sdk (2.0.3) will not recognize the in keyword: WIth 2.1.4 dotnet sdk installed (I used this at the time of the report). Build log: After removing the offending lines, the output dll IL extracted with monodis: Full project with compiled binaries in case monodis was buggy: |
|
@dsyme I have managed to get ildasm working on linux (with some symlink hacks to enable coreclr initialization), here is the ildasm output: |
|
@zpodlovics I'm not getting any warning or error when compiling this using C# in Visual Studio 2017 15.7.1 |
|
I suspect the presence of |
|
Anyway, if I understand correctly, to follow the C# behaviour then the following should call the first overload (without complaint), and not the second. |
|
@zpodlovics @dsyme As far as I can tell, the ambiguity error you're talking about was resolved in Roslyn 2.7 (VS 15.6, .Net Core SDK 2.1.100). See dotnet/csharplang#945. So it's expected that with .Net Core SDK 2.1.4, you will get the error (2.1.4 < 2.1.100). |
|
Well, I would rather vote for explicit type signatures without auto conversion + explicit conversion functionality in case it needed. It may worth adopting their ref readonly testsuite cases and port it to F# plus testing the C# -> F# C# -> F# interop too. It's still a hot topic for csharplang and roslyn: dotnet/csharplang#945 dotnet/roslyn#19216 Example use case: assuming I have a large struct something like I posted earlier, and I would like to have functionality that could consume this large struct by value (eg.: to process on an another thread running on different physical cpu - do not share data between physical cpus to avoid cpu-cpu interconnect traffic) and by readonly reference (eg.: to process on thread using the same physical cpu). Would it be possible to have also an explicit 'T -> type C() =
static member M(x: System.DateTime) = x.AddDays(1.0)
static member M(x: inref<System.DateTime>) = x.AddDays(2.0)
let t = System.DateTime.Now
let tAsInRref = &t
let v1 = C.M(t)
let v2 = C.M((inref<System.DateTime>) t)
let v3 = C.M(&t) |
module NativePtr =
val toVoidPtr : address:nativeptr<'T> -> voidptr
val ofVoidPtr : voidptr -> nativeptr<'T>More curiosity than anything here. Why were these described as Feedback around
Will such structs also be implicitly marked as |
|
@jaredpar That's the module signature, take a look to these changelogs: [1] https://github.com/Microsoft/visualfsharp/pull/4888/files#diff-085a94dd8e50b6ab66ae9a3263edcdabR70 |
|
A couple of questions from #306:
|
|
@jaredpar @OmarTawfik Please keep reviewing and asking questions, thanks! Each question is making me go and check both the implementation and, critically, the test suite in the PR.
@jaredpar Both need to be specified. I'll add an unresolved question about what happens if one or both is absent
@jaredpar yes. That made me go and review the negative tests in the PR and there are some problems with them, so I've added a TODO about that, thanks!
@jaredpar Yes, the
@OmarTawfik I don't think so. Authoring these types in F# for consumption by down-level C# consumers will be extremely rare (if it ever happens at all) in F#. Down-level consumption by F# consumers will also never happen. I'll add that to the RFC.
@OmarTawfik Yes, the latest version of the implementation does this, see here and its callsites |
|
@dsyme since you're adding tests, here are some C# tests that might give you ideas on corner cases/bugs we saw around these features: Semantics:
CodeGen:
|
The proposed design is that this doesn't happen, partly because it depends on whether the attribute is actually available to emit at all.
@NinoFloris The attribute is defined in .NET libraries. |
|
How are ref returns handled for assignment? ex: let write8 (data: Span<byte>) offset value =
data.[offset] <- byte valueor something.GetByRef() <- valueToday you can't do this, so I'm wondering what the rules/intent are here. |
|
@TIHan had a similar question around implicit dereference. Is the intent of F# to always implicitly deref here? In general C# implicitly derefs but there are a few cases in which the value is not implicitly derefed:
For the above assume we have the following C# code Assume for a second we have a C# method with the following signature: struct Widget {
internal void Move() { ... }
}
ref Widget M() => ...Not trying to imply that F# should necessarily behavior like C# here. But wanted to understand the intent and where the languages may differ. Also how does type inference work here with implicit deref? Consider this: let array = [| 0 |]
let main() =
let f (): byref<int> = &array.[0]
let g () = f ()Is the type of g here meant to be |
|
@jaredpar asks:
Thanks for highlighting these cases. Assignment to ref-returns is not currently supported, so type C() =
static let mutable v = System.DateTime.Now
static member M() = &v
let F1() =
C.M() <- System.DateTime.Nowgives an error. I will add that as an unresolved issue. You are right it is reasonable to support this. Member invocation compiles correctly, e.g. gives IL code:
let array = [| 0 |]
let main() =
let f (): byref<int> = &array.[0]
let g () = f ()Currently in the RFC let-bound functions do not have implicit dereference, this is noted in the RFC. I've spoken with Will Smith and he said that during your trialling of the feature you noticed this as an oddity. I think I made the wrong call here and implicit dereference of byref returns should happen for let-bound functions too. I will add it as an unresolved issue. But with regard to type inference, this code: type C() =
static let mutable v = System.DateTime.Now
static member M() = &v
static member G() = C.M()gives: whereas this code: type C() =
static let mutable v = System.DateTime.Now
static member M() = &v
static member G() = &C.M()gives: |
|
@dsyme and I had a discussion and we believe we should have implicit dereferencing on ref returns when calling let-bound functions, just like member functions. That would change the RFC here. |
|
From: dotnet/fsharp#4888 On parity with C# support also means producting not only consuming and using Span! Please do not limit the usefullness of Span support by limiting IsByRefLike struct creations, at least allow it with an experimental attribute, compiler warning (this feature may change in the future or whatever) or something like that. Some of the developers may only interested to consume and use Span and byreflike from C# but I would like to also able to produce it. It would be really hard to sell F# when the core libraries will still have to write in C#... |
|
@dsyme reading through the spec again this morning noticed this line I'd missed in the first reading:
This isn't allow in C# because it ends up complicating the safety rules considerably for consumers of [<IsByRefLike; Struct>]
type S(x: byref<int>) =
member this.X = x
member this.Evil (y: byref<int>) = this <- S(y) In order for the let f (s: S) =
let y = 42
s.Evil(&y) // Error!!!This pattern actually shows up a lot in CoreFx and it was really hurting the ability to write usable APIs. Eventually we concluded the best way to move forward was to eliminate the possibility of a I realized yesterday in talking with @TIHan that we'd forgotten to write this down in our span safety rules. I sent a PR out last night to correct this. There is an addendum at the end on how we thought we could support this in the future in a more sensible way. |
|
@dsyme quick spec question.
Should this be:
? |
|
Closing as the feature (set) is implemented and shipping as-is with the latest documented scoping rules. Further changes will likely just be bug fixes. Bigger changes in behavior will necessitate a new RFC, and thus, a new discussion isssue. |
|
@dsyme I have a question about this: https://github.com/fsharp/fslang-design/blob/master/FSharp-4.5/FS-1053-span.md#ignoring-obsolete-attribute-on-existing-byreflike-definitions
What is the reason for not doing this for F#? It's kind a gross solution, but it does make it unambiguous why an issue is here because the message says outright that your compiler does not understand this type. Otherwise, users with downlevel compilers (e.g., F# 4.1 targeting .NET Core 2.1) could be very, very confused about consuming an F# component that emits a ByRefLike type. |
|
Apologies for the late reply
I think it is just a feature limitation due to time constraints and the much lower probability of this scenario happening in significant ways (it is just rare to consume F# components using very latest features in down-level compilers) |
Discussion thread for (RFC FS-1053](https://github.com/fsharp/fslang-design/blob/master/FSharp-4.5/FS-1053-span.md) covering all of
The text was updated successfully, but these errors were encountered: