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 support for mixed async/nonasync branches in thernary if clause (corresponding to C#) #937

Closed
4 of 5 tasks
Lanayx opened this issue Dec 1, 2020 · 5 comments
Closed
4 of 5 tasks

Comments

@Lanayx
Copy link

Lanayx commented Dec 1, 2020

Title of Suggestion

I propose we add a possibility of adding synchronous branch to the if statement inside async/task CE. This is extremely helpful in caching cases when cache can be got synchronously otherwise asynchronous call is made. This can already be achieved in C#

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

There are two way of approaching this problem in F#:

  1. Return completed task or async.Return. This leads to additional allocations and method calls
async {
    let! result = 
        if condition then
            doSmthAsyncReturningInt()
        else
            async.Return 0
}
  1. Use mutable variable
async {
    let mutable result = -1
    if condition then
        let! newResult = doSmthAsyncReturningInt()
        result <- newResult
    else
        result <- 0
}

(sharplab)

Pros and Cons

The advantages of making this adjustment to F# are - to conform convenient C# way of dealing with cache and async

The disadvantages of making this adjustment to F# are - it's unclear which syntax should be used for that, with C# it's very straitforward since await is standing near the called function, with F# it on the other side of the equal sign, so it might look like this

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

Extra information

Estimated cost (M):

Related suggestions: (#863 as some initial conversation happened there, #581)

Affidavit (please submit!)

Please tick this by placing a cross in the box:

  • This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • 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:

  • This is not a breaking change to the F# language design
  • 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 👍 emoji on this issue. These counts are used to generally order the suggestions by engagement.

@Lanayx Lanayx changed the title Add support for mixed async/nonasync branches in if clause (corresponding to C#) Add support for mixed async/nonasync branches in thernary if clause (corresponding to C#) Dec 1, 2020
@yatli
Copy link

yatli commented Dec 1, 2020

Basically, the current way of approaching this is to introduce nested CEs:

async {
    let! v1 = async { if cond1 then return! doSmthAsyncReturningInt() else return 0 }
    let! v2 = async { if v1 > 77 then return! doSmthAsyncReturningInt() else return 1 }
    let! v3 = async { if v2 > 88 then return! doSmthAsyncReturningInt() else return 2 }
    return v3
}

... while in C#, there's no such thing as nested async states within one method.
This then leads to the difference that the F# nested async contexts have explicit boundaries, and to weave the dependencies of these contexts one has to repeatedly wrap/unwrap the values.

I'm not sure about the syntax that allow different types (wrapped vs. unwrapped) as proposed by @Lanayx (e.g. why only async but not other CEs? Is there a better syntactic proposal to make the typing consistent?), but at least, can we do something in the compiler so that nested CEs can be integrated with the outer one?

@Lanayx
Copy link
Author

Lanayx commented Jun 3, 2021

Just an update - it seems we can close the issue once dotnet/fsharp#6811 is merged, since nested CE won't create an overhead anymore (for tasks). we'll just need to change async to task to get rid of the overhead.

@dsyme
Copy link
Collaborator

dsyme commented Jun 3, 2021

Just an update - it seems we can close the issue once dotnet/fsharp#6811 is merged, since nested CE won't create an overhead anymore (for tasks). we'll just need to change async to task to get rid of the overhead.

Just to mention this is not completely the case - nested CEs will still create a sort of overhead even for tasks (another state machine + method, though stack allocated initially). For CEs like Async where there is allocation or boxing this will minimally involve those allocations.

@NinoFloris
Copy link

Ideally you would have binds under let bindings bubble up to the nearest CE, how feasible that is I don't know.

@dsyme
Copy link
Collaborator

dsyme commented Jun 14, 2022

Will close in favour of #1070 and #1000

@dsyme dsyme closed this as completed Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants