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

Forbid CE syntax which misleadingly looks like an early return #15759

Open
Smaug123 opened this issue Aug 7, 2023 · 4 comments
Open

Forbid CE syntax which misleadingly looks like an early return #15759

Smaug123 opened this issue Aug 7, 2023 · 4 comments

Comments

@Smaug123
Copy link
Contributor

Smaug123 commented Aug 7, 2023

Is your feature request related to a problem? Please describe.

The following code compiles, but its semantics are not at all obvious: the return keyword is not in fact returning anything, and it prints "Here".

async {
    if true then
        return ()
    printfn "Here"
    return ()
}
|> Async.RunSynchronously

I claim this behaviour is very counterintuitive, because return is explicitly a word from the imperative paradigm, but it does not behave like the corresponding imperative construct.

(Note that if you try and early-return anything other than unit, you are correctly told that the thing you're trying to return needs to be of unit type to satisfy the constraint that it's the body of an if block.)

Describe the solution you'd like

A warning indicating that this code likely does not do what I expect, suggesting that I remove the return keyword.

I don't know how wide-ranging this problem is, because I haven't thought hard about it. Does the warning make sense only for the async computation expression, or will it have similarly unexpected semantics in every computation expression that implements Return?

@vzarytovskii
Copy link
Member

A use case for analyzers, for sure.

@edgarfgp
Copy link
Contributor

edgarfgp commented Aug 8, 2023

A use case for analyzers, for sure.

I was wondering if #11057 can have some attention in the near future. ?

@vzarytovskii
Copy link
Member

vzarytovskii commented Aug 8, 2023

A use case for analyzers, for sure.

I was wondering if #11057 can have some attention in the near future. ?

Probably not near, but yes, at some point. It requires a lot of designing, and probably pausing on some feaures.

@baronfel
Copy link
Member

baronfel commented Aug 8, 2023

Idea: can we make a label like analyzer-candidate and apply it to issues like these to get an easy way to track analyzer requests? That will help motivate working on analyzers support all-up.

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

5 participants