Allow byrefs in inlined lambdas

pirrmann commented 2 years ago

I propose we allow using byrefs in inlined lambdas, which we known won't create any closure, and don't generate an FS0406 error.

For instance, when using a lock pattern (which now uses InlineIfLambda), we currently can't write:

member this.TryGet(x : outref<int>) : bool =
    lock this <| fun () ->
        x <- 1
        true

Using this code generates an FS0406 error: "Byrefs cannot be captured by closures or passed to inner functions".

The existing way of approaching this problem in F# is to not use a lambda, and "manually" inline the code to

member this.TryGet(x : outref<int>) : bool =
    let mutable lockTaken = false
    try
        Monitor.Enter(this, &lockTaken);
        x <- 1
        true
    finally
        if lockTaken then
            Monitor.Exit(this)

This relates to #688, but I believe that this is a different issue and not a duplicate, as this only relates to inlined lambdas, not regular inlined functions (even though I don't know how complicated or different that is during the compile process).

Pros and Cons

The advantages of making this adjustment to F# are that this allows writing code that is more concise and less error-prone

I currently don't see any disadvantages of making this adjustment to F#.

Extra information

Estimated cost (XS, S, M, L, XL, XXL): S

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.

kerams commented 2 years ago

I think the problem is this

https://github.com/fsharp/fslang-design/blob/main/FSharp-6.0/FS-1098-inline-if-lambda.md#detailed-design

We add InlineIfLambdaAtttribute to use on parameters of inlined functions and methods, indicating to the compiler that, if a lambda is supplied as a parameter, it should be inlined. The attribute is indicative only and may be ignored by the F# compiler.

pirrmann commented 2 years ago

I had missed that "indicative only" part. In practice in my code I haven't noticed any use case where this isn't inlined, but I understand why considering this as indicative only makes it less of a constraint on the compiler. I guess that unless that design decision is revisited, we can close this suggestion.

NinoFloris commented 2 years ago

Duplicate of https://github.com/fsharp/fslang-suggestions/issues/688

pirrmann commented 2 years ago

As I indicated in the description:

This relates to https://github.com/fsharp/fslang-suggestions/issues/688, but I believe that this is a different issue and not a duplicate, as this only relates to inlined lambdas, not regular inlined functions (even though I don't know how complicated or different that is during the compile process).

@NinoFloris are you certain that this is a duplicate?

NinoFloris commented 2 years ago

Sorry I missed that link!

The issue is that inlining happens after type checking, irrespective of a function being let bound or a lambda.

Effectively meaning: to support byref in any inlined functions the compiler would have to blend the phases to inline and type check. It needs to type check, run into byref issues, then inline that to see if the end result now type checks or needs to be inlined further

Additionally - just to give an example of some of the complexities involved - just as there could be another call to an inline function in the body of the initial inline fuction. There could also be a mix of calls, some inline while others aren't. What's the error for that? "This inline function does not support byref inlining due to generic call sites in its body not supporting inlining"? Should we require an attribute on functions to signify byref support?

Doing something here would either be a very small hack to allow it for, say, pipelining operators or full language support like SRTP. Anything in between would lead to a language where it would be very hard to understand when you can and cannot use byref generically.

matthewcrews commented 2 years ago

This would be amazing for the work that I am doing. This limitation keeps me from using Array.iter and Array.map in many of my use cases and leads to messier code.

roboz0r commented 1 year ago

Another use case for this is I have come across is the lock function. Using byrefs is unavoidable in my case as the value originates from an external interface.

fsharp / fslang-suggestions