fslang-suggestions
fslang-suggestions copied to clipboard
Emit direct delegates when possible
F# currently never emits "direct" delegates - it only ever emits delegates that go via a closure object.
As background, .NET IL permits the emit of direct delegates, e.g. for the code:
delegate D of someArg: int -> int
type C() =
static method M(arg:int) = 1
System.Func<int,int>(C.M)
F# currently always emits a closure. Instead it is possible to emit
ldnull // load null because method is static
ldftn void C::M(int)
newobj instance void class System.Func`2<int, int>::.ctor(object, native int)
That is, a direct delegate is not a closure but rather a delegate with delegate.TargetMethod known to be the exact MethodInfo for the target.
This needs careful specification and is best done as an RFC as the differences are, in some sense, observable to reflection.
The basic situation is
SomeDelegateType<...>(expr)
The cases we care about are where the target is a "known" function or method, for example:
System.Action<_,_>(handler) // non-eta-expanded known target
System.Action<_,_>(fun a b -> handler a b) // eta-expanded known target
System.Action<_,_>(fun a b -> handler (a,b)) // eta-expanded known target with different currying/tupling but same compiled representation
System.Action<_,_>(SomeType.StaticMethod) // same with a static method
System.Action<_,_>(fun a b -> SomeType.StaticMethod a b) // same with a static method
System.Action<_,_>(fun a b -> SomeType.StaticMethod(a,b)) // same with a static method
System.Action<_,_>(SomeType<...>.StaticMethod<...>) // same with a generic static method
System.Action<_,_>(fun a b -> SomeType<...>.StaticMethod<...> a b) // same with a generic static method
System.Action<_,_>(fun a b -> SomeType<...>.StaticMethod<...>(a,b)) // same with a generic static method
System.Action<_,_>(someObject.InstanceMethod) // same with an instance method
System.Action<_,_>(fun a b -> someObject.InstanceMethod a b) // same with an instance method
System.Action<_,_>(fun a b -> someObject.InstanceMethod(a, b)) // same with an instance method
System.Action<_,_>(fun a b -> someObject.InstanceMethod<...> a b) // same with a generic instance method
System.Action<_,_>(fun a b -> someObject.InstanceMethod<...>(a, b)) // same with a generic instance method
Arguably we may also care about partial applications - here a closure is needed but the argument names generated for the closure could be those drawn from the elided arguments of the obvious target:
System.Action<_,_>(handler arg1 arg2) // non-eta-expanded known target via partial application
System.Action<_,_>(SomeType.StaticMethod arg1) // non-eta-expanded known target via partial application
The questions are
- When do we guarantee to create a direct delegate?
- If we don't, do we guarantee to at least create a closure with specific argument names
We have to consider these questions for both Debug and Release code.
The proposal for the cases above is:
-
Direct delegate?
- For non-eta-expanded - currently no - proposal is to change this to "yes" for both debug and release code
- For eta-expanded - currently no - proposal is to change this to "yes" for release code. The user has made the lambdas explicit, however release code can be more efficient eliminating intermediate closures. For debug code we would still generate a closure with the user-given argument names.
-
If no direct delegate, then what are the closure argument names?
- For non-eta-expanded - currently you get a closure with funny argument names "delegateArg0" etc. The proposal is to always generate a direct delegate, so there is nothing more to do beyond that
- For non-eta-expanded partial applications - currently you get a closure with funny argument names "delegateArg0" etc. The proposal is to use the argument names drawn from the obvious target
- For eta-expanded - currently we use the argument names derived from the patterns in the source code. The proposal is to leave this unchanged for debug code. In release code we will generate a direct delegate.
-
Are these changes visible in quotations?
- The proposal is that quotations wouldn't change.
-
Are these changes visible to FCS?
- The proposal is that FCS reported expressions won't change.
This is in theory a potential breaking change because user code doing inspection on delegate value .TargetMethod may detect the difference. However within the context of the language spec it's a reasonable change as the TargetMethod is currently a closure and so inspection is not particularly useful, and the language spec gives no guarantees about this situation. For this reason it is reasonable to make this change via an RFC and /langversion
The transformation would be implemented in the F# optimizer and Ilxgen.
Pros and Cons
The advantages of making this adjustment to F# are efficiency and more direct translation of F# code
The disadvantages of making this adjustment to F# are it's work.
Extra information
Estimated cost (XS, S, M, L, XL, XXL): S
Related suggestions: (put links to related suggestions here)
Affidavit (please submit!)
Please tick this by placing a cross in the box:
- [x] This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
- [x] I have searched both open and closed suggestions on this site and believe this is not a duplicate
- [x] 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:
- [x] This is not a breaking change to the F# language design
- [x] I or my company would be willing to help implement and/or test this
For Readers
If you would like to see this issue implemented, please click the :+1: emoji on this issue. These counts are used to generally order the suggestions by engagement.
One problem with the above is that in the case of a multi-argument delegate there's no simple way to always get a direct delegate
Consider this code:
type C() =
static method M(arg1: int, arg2: int) = 1
System.Func<int,int,int>(C.M)
This doesn't compile - the problem is that F# asks delegates to be implemented using curried functions, so an explicit lambda is needed:
type C() =
static method M(arg1: int, arg2: int) = 1
System.Func<int,int,int>(fun a b -> C.M(a,b))
Per the above spec, a direct delegate would only be generated for release code, but not debug code. We could in theory always generate a direct delegate here even for debug code though it would be a little unusual because you would then no longer be able to breakpoint inside the lambda on the call to C.M(a,b)
. There's no obvious solution to this dilemma - we have problems if we do, and problems if we don't.
Note there are situations where C# APIs actually do care about the names of arguments and characteristics of target methods - they use inspection to do various things and it can even effect data decoding and deserialization. As far as I understand things this includes the ASP.NET Core Minimal WebAPI implementation.
I'll mark this as approved in principle, though there is one technical question remaining as mentioned above.
As an aside, it should be noted that delegate construction always delays (and reevaluates) the relevant function on each call, so, for example
new System.Action<int>(failwith "")
is equivalent to
new System.Action<int>(fun arg -> (failwith "") arg)
and doesn't fail on delegate construction, but rather delegate invocation.
Notes on the testing matrix for this
-
"unit returning" cases should be considered. This is the matrix of cases where the compiled representation of the target method returns
void
orunit
or a generic type variable instantiated tounit
, and the delegate likewise has signature with returnunit
or a generic type variable instantiated tounit
. -
targets which are structs v. ref types should be considered.
-
targets which are generic v. non-generic should be considered.
-
targets which are F# methods v. IL methods should be considered.
-
targets which are generic with witnesses v. not
-
targets which are extension methods or not
-
targets which are static or instace
-
targets which are virtual and non-virtual
Note to self: when creating first-class lambdas for .NET methods we aren't giving good names to implied arguments for the lambda. This can be improved:
> System.IO.File.OpenText;;
val it : arg00:string -> System.IO.StreamReader
Emit direct delegates might be useful with mono p/invoke callbacks: https://stackoverflow.com/a/28570186
We are interested in this code:
CaptureDelegate d = MyCapture;
block_value.SetupBlock(d, null);
...
[MonoPInvokeCallback(typeof(CaptureDelegate))]
static void MyCapture(IntPtr block, IntPtr self, IntPtr uiView)
{
...
This code can be written in F# as follows:
let d = CaptureDelegate(MyClass.MyCapture)
block_value.SetupBlock(d, null)
but this throws an exception at runtime:
The method <StartupCode$MyApp>[email protected] does not have a [MonoPInvokeCallback] attribute.
Perhaps this is incorrect behavior of the compiler and compiler should add attributes to the method of closure object. @dsyme what do you think?
I found a way to create "direct" delegate via reflection:
let d = Delegate.CreateDelegate(typeof<CaptureDelegate>, typeof<MyClass>.GetMethod("MyCapture"))
block_value.SetupBlock(d, null)
and this code works well. But this looks like a dirty hack.
I will try to fix it one day; don't know when I will have time.
Considering the parameter name issue reported in #1145, does that not indicate that debug and release code should stay aligned?
The OP says:
This is in theory a potential breaking change because user code doing inspection on delegate value .TargetMethod may detect the difference. However within the context of the language spec it's a reasonable change as the TargetMethod is currently a closure and so inspection is not particularly useful, and the language spec gives no guarantees about this situation. For this reason it is reasonable to make this change via an RFC and /langversion
but if the user code relying on this inspection for API contracts is the ASP.NET Core framework, that's a whole other order of magnitude of breaking change.
Relevant minimal api issue https://github.com/dotnet/aspnetcore/issues/38906
Important to note, the one way to add attributes to parameters (for instance for FromHeader
and friends) would be to pull out the lambda to a named function. This then breaks the parameter name reflection. So realistically you're stuck if you need to do this.