Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add if! (if-bang) keyword to computation expressions #863

Open
jwosty opened this issue Apr 26, 2020 · 20 comments
Open

Add if! (if-bang) keyword to computation expressions #863

jwosty opened this issue Apr 26, 2020 · 20 comments

Comments

@jwosty
Copy link
Contributor

jwosty commented Apr 26, 2020

Add if! (if-bang) keyword to computation expressions

I propose we extend computation expressions with a new keyword, if! (if-bang), that functions a let! (let-bang) combined with an if statement. For example, instead of writing:

let! shouldSolveWorldHunger= confirmAsync ("Solve world hunger?")
if shouldSolveWorldHunger then solveWorldHunger () else dont ()

or even:

match! confirmAsync("Solve world hunger?") with
| true -> solveWorldHunger ()
| false -> dont ()

We should be able to do:

if! confirmAsync("Solve world hunger?") then
    solveWorldHunger ()
else dont ()

Pros and Cons

Pros: Aids code readability in specialized cases.

Cons: It's work, and it increases complexity. (whether or not this is worth the effort for potentially such small return is up to debate / whoever would do it, but I figure it's worth filing [probably low priority :) ] since I am not aware of a prior suggestion -- would be brownie points for the F# language)

Extra information

S

Related suggestions: (put links to related suggestions here)

#572

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
@dsyme
Copy link
Collaborator

dsyme commented Apr 27, 2020

This seems reasonable

@nikonthethird
Copy link

Should that include elif! as well?

let x = async {
    if! asyncBool then
        do! task1
    elif! anotherAsyncBool then
        do! task2
    else
        do! task3
}

@yatli
Copy link

yatli commented Apr 27, 2020

@nikonthethird probably yes, and pretty effortless, since elif! = else if!

@realvictorprm
Copy link
Member

Good catch, that's really quite reasonable! :3

@mvkara
Copy link

mvkara commented Apr 28, 2020

It would be good to consider other typical branching constructs as well; in particular the "while" keyword. A good use case would be using ADO.NET; not sure if this is already supported. Normally you would need a recursive function to do this - would be much easier with .

asyncSeq {
    // https://docs.microsoft.com/en-us/dotnet/api/system.data.common.dbdatareader.readasync?view=netcore-3.1
    while! reader.MoveNextAsync() do
        yield {| Column1 = reader.GetInt32(0); Column2 = reader.GetInt32(1) |}
}

@Tarmil
Copy link

Tarmil commented Apr 28, 2020

@mvkara There's a major difference between while! (and for! I guess) on one hand, and match! and if! on the other.

  • match! and if! are just syntactic sugar: they're equivalent to let! followed by the corresponding keyword. So they don't require adding anything to the CE, as soon as they're added to the language they're usable with any CE.

  • while!'s condition is not evaluated just once, so it's not so simple. It would need to be implemented explicitly by a CE (presumably as a method WhileFrom).

So I think it's a good idea but probably belongs in a separate suggestion.

@yatli
Copy link

yatli commented Apr 28, 2020

I agree with @Tarmil -- plus that it's likely these constructs should be designed to incorporate with IAsyncEnumerable

@Lanayx
Copy link

Lanayx commented Apr 28, 2020

Wanted to add one more comment for discussion, not directly related to the topic, but not sure where to put it - it would be nice to support bang operations inside if without overhead of raising 0 to async:

async {
    let result = 
        if condition then
            let! x = doSmthAsyncReturningInt()
            x
        else
            0
    return result
}

this can be easily done in C#, but not F#:

var result = condition ? await doSmthAsyncReturningInt() : 0;

@nikonthethird
Copy link

It is actually possible to implement a while! today by adjusting the async expression builder. You just have to add another overload for while that accepts an asynchronous guard:

type AsyncBuilder with
    member this.While (guard, body) =
        let guard = guard ()
        let rec loop = function
        | false -> this.Zero ()
        | _ -> this.Combine (body, this.Bind (guard, loop))
        this.Bind (guard, loop)
    

Async.RunSynchronously (async {
    let mutable count = 5
    while async { return count > 0 } do
        do printfn "Counting down: %d" count
        do count <- count - 1
})

@Tarmil
Copy link

Tarmil commented Apr 28, 2020

@Lanayx The problem in your code isn't the if actually; it's that you're putting a let! inside the definition part of a let, which means it's not part of the CE anymore. See for example:

// This does NOT work, even without an if, because there's a let! inside a let:
async {
    let sum =
        let! x = someAsyncCode()
        x + y
    return sum * 2
}

// This DOES work, even with the if:
async {
    if someCondition() then
        let! x = someAsyncCode()
        return x + y
    else
        return y * 2
}

@Lanayx
Copy link

Lanayx commented Apr 28, 2020

@Tarmil Thanks for the explanation! However as you can see, the problem is not in my code, but in this nested limitation - C# doesn't have such limitation and it would be nice not to have it in F# either.

@yatli
Copy link

yatli commented Apr 28, 2020

@Lanayx I think I understand the problem. But having different types on if-else branches feel wrong to me.

@Lanayx
Copy link

Lanayx commented Apr 29, 2020

@yatli this is very helpful for caching scenarios - if local cache exists return cached value, otherwise make async call. To go over this limitation I had to use mutable variable

@yatli
Copy link

yatli commented Apr 29, 2020

Hmm. Something like a new function Async.FromCompleted<'T>?

But I still think it's better to use the return directive because of type consistency.

It's just like you cannot write if cond then 1 else 0.5 -- and mandatory explicit typecast is a good thing.

@Lanayx
Copy link

Lanayx commented Apr 29, 2020

@yatli yes, if this cast is free :) With return it is an additional allocation and method call, and when it's on the hot path (like in my case), this should be avoided.

@sideeffffect
Copy link

could this be done, in a short-circuiting way?

let x = async {
    if! asyncBool and asyncBool2 then
        do! task1
    elif! anotherAsyncBool or asyncBool3 then
        do! task2
    else
        do! task3
}

@yatli
Copy link

yatli commented Nov 29, 2020

@Lanayx I don't think it's optimized that way... More like a syntax sugar that wraps 0 into the completed task.

#LINQPad optimize+

async void Main()
{
	await compute(3);
}

async System.Threading.Tasks.Task<int> realAsync() {
	await System.Threading.Tasks.Task.Delay(100);
	return 0;
}

async System.Threading.Tasks.Task<int> compute(int x) => x > 2 ? await realAsync() : 0;

Turns to:

realAsync:
IL_0000:  ldloca.s    00 
IL_0002:  call        System.Runtime.CompilerServices.AsyncTaskMethodBuilder<System.Int32>.Create
IL_0007:  stfld       UserQuery+<realAsync>d__5.<>t__builder
IL_000C:  ldloca.s    00 
IL_000E:  ldc.i4.m1   
IL_000F:  stfld       UserQuery+<realAsync>d__5.<>1__state
IL_0014:  ldloca.s    00 
IL_0016:  ldflda      UserQuery+<realAsync>d__5.<>t__builder
IL_001B:  ldloca.s    00 
IL_001D:  call        System.Runtime.CompilerServices.AsyncTaskMethodBuilder<System.Int32>.Start<<realAsync>d__5>
IL_0022:  ldloca.s    00 
IL_0024:  ldflda      UserQuery+<realAsync>d__5.<>t__builder
IL_0029:  call        System.Runtime.CompilerServices.AsyncTaskMethodBuilder<System.Int32>.get_Task
IL_002E:  ret         

compute:
IL_0000:  ldloca.s    00 
IL_0002:  call        System.Runtime.CompilerServices.AsyncTaskMethodBuilder<System.Int32>.Create
IL_0007:  stfld       UserQuery+<compute>d__6.<>t__builder
IL_000C:  ldloca.s    00 
IL_000E:  ldarg.0     
IL_000F:  stfld       UserQuery+<compute>d__6.<>4__this
IL_0014:  ldloca.s    00 
IL_0016:  ldarg.1     
IL_0017:  stfld       UserQuery+<compute>d__6.x
IL_001C:  ldloca.s    00 
IL_001E:  ldc.i4.m1   
IL_001F:  stfld       UserQuery+<compute>d__6.<>1__state
IL_0024:  ldloca.s    00 
IL_0026:  ldflda      UserQuery+<compute>d__6.<>t__builder
IL_002B:  ldloca.s    00 
IL_002D:  call        System.Runtime.CompilerServices.AsyncTaskMethodBuilder<System.Int32>.Start<<compute>d__6>
IL_0032:  ldloca.s    00 
IL_0034:  ldflda      UserQuery+<compute>d__6.<>t__builder
IL_0039:  call        System.Runtime.CompilerServices.AsyncTaskMethodBuilder<System.Int32>.get_Task
IL_003E:  ret   

<compute>d__6.MoveNext:
IL_0000:  ldarg.0     
IL_0001:  ldfld       UserQuery+<compute>d__6.<>1__state
IL_0006:  stloc.0     
IL_0007:  ldarg.0     
IL_0008:  ldfld       UserQuery+<compute>d__6.<>4__this
IL_000D:  stloc.1     
IL_000E:  ldloc.0     
IL_000F:  brfalse.s   IL_0051
IL_0011:  ldarg.0     
IL_0012:  ldfld       UserQuery+<compute>d__6.x
IL_0017:  ldc.i4.2    
IL_0018:  ble.s       IL_0078
IL_001A:  ldloc.1     
IL_001B:  call        UserQuery.realAsync
IL_0020:  callvirt    System.Threading.Tasks.Task<System.Int32>.GetAwaiter
IL_0025:  stloc.s     04 
IL_0027:  ldloca.s    04 
IL_0029:  call        System.Runtime.CompilerServices.TaskAwaiter<System.Int32>.get_IsCompleted
IL_002E:  brtrue.s    IL_006E
IL_0030:  ldarg.0     
IL_0031:  ldc.i4.0    
IL_0032:  dup         
IL_0033:  stloc.0     
IL_0034:  stfld       UserQuery+<compute>d__6.<>1__state
IL_0039:  ldarg.0     
IL_003A:  ldloc.s     04 
IL_003C:  stfld       UserQuery+<compute>d__6.<>u__1
IL_0041:  ldarg.0     
IL_0042:  ldflda      UserQuery+<compute>d__6.<>t__builder
IL_0047:  ldloca.s    04 
IL_0049:  ldarg.0     
IL_004A:  call        System.Runtime.CompilerServices.AsyncTaskMethodBuilder<System.Int32>.AwaitUnsafeOnCompleted<TaskAwaiter`1,<compute>d__6>
IL_004F:  leave.s     IL_00AB
IL_0051:  ldarg.0     
IL_0052:  ldfld       UserQuery+<compute>d__6.<>u__1
IL_0057:  stloc.s     04 
IL_0059:  ldarg.0     
IL_005A:  ldflda      UserQuery+<compute>d__6.<>u__1
IL_005F:  initobj     System.Runtime.CompilerServices.TaskAwaiter<System.Int32>
IL_0065:  ldarg.0     
IL_0066:  ldc.i4.m1   
IL_0067:  dup         
IL_0068:  stloc.0     
IL_0069:  stfld       UserQuery+<compute>d__6.<>1__state
IL_006E:  ldloca.s    04 
IL_0070:  call        System.Runtime.CompilerServices.TaskAwaiter<System.Int32>.GetResult
IL_0075:  stloc.3     
IL_0076:  br.s        IL_007A
IL_0078:  ldc.i4.0    
IL_0079:  stloc.3     
IL_007A:  ldloc.3     
IL_007B:  stloc.2     
IL_007C:  leave.s     IL_0097
IL_007E:  stloc.s     05 
IL_0080:  ldarg.0     
IL_0081:  ldc.i4.s    FE 
IL_0083:  stfld       UserQuery+<compute>d__6.<>1__state
IL_0088:  ldarg.0     
IL_0089:  ldflda      UserQuery+<compute>d__6.<>t__builder
IL_008E:  ldloc.s     05 
IL_0090:  call        System.Runtime.CompilerServices.AsyncTaskMethodBuilder<System.Int32>.SetException
IL_0095:  leave.s     IL_00AB
IL_0097:  ldarg.0     
IL_0098:  ldc.i4.s    FE 
IL_009A:  stfld       UserQuery+<compute>d__6.<>1__state
IL_009F:  ldarg.0     
IL_00A0:  ldflda      UserQuery+<compute>d__6.<>t__builder
IL_00A5:  ldloc.2     
IL_00A6:  call        System.Runtime.CompilerServices.AsyncTaskMethodBuilder<System.Int32>.SetResult
IL_00AB:  ret         

<compute>d__6.SetStateMachine:
IL_0000:  ldarg.0     
IL_0001:  ldflda      UserQuery+<compute>d__6.<>t__builder
IL_0006:  ldarg.1     
IL_0007:  call        System.Runtime.CompilerServices.AsyncTaskMethodBuilder<System.Int32>.SetStateMachine
IL_000C:  ret         
      

You see, IL_0076 is the last one from the realAsync branch -- before that, the result is unwrapped from GetResult.
For the other branch, starting at IL_0078, it just loads 0. Nothing fancy.
But after that, the two branches reunion, and the result is pushed into the return task's SetResult.
So you get a task builder and a state machine, plus a task object for return value, no matter which branch you go -- kinda like the two return statements that raise the results from two branches from @Tarmil 's example.

@Lanayx
Copy link

Lanayx commented Nov 30, 2020

@yatli Right, for simple case it's obvious that result should be wrapped in completed task. However in case we have multiple async calls this can be avoided, here is a sharplab

@yatli
Copy link

yatli commented Dec 1, 2020

Thank you @Lanayx , I didn't know it could pull that off.
The C# version is obviously advantageous in your case, because the IL simply jumps from one unwrapped result to the next if the right conditions are met.

Would you mind opening another thread to continue the discussion? There are both syntax and compiler optimization issues, but out of scope of this suggestion.

@kurtschelfthout
Copy link
Member

Consider allowing an elaboration to a new If method builder. Such a method would be somewhere between Applicative and Monad in power, and so can allow e.g. more efficient implementation, or conservative static analysis. See https://www.staff.ncl.ac.uk/andrey.mokhov/selective-functors.pdf

If F# automatically desugars it to Bind - as I think this suggestion is suggesting - then the implementor of the computation expression can't take that opportunity.

Perhaps a related suggestion then is to automatically desugar "lower power" computation expression operations in terms of explicitly defined "higher power" operations if the lower power operations are not available. E.g if Bind is available then Map can be generated in terms of Bind, and so on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests