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

Early return could be doable with if-else? #8

Open
Thorium opened this issue Mar 28, 2019 · 6 comments
Open

Early return could be doable with if-else? #8

Thorium opened this issue Mar 28, 2019 · 6 comments

Comments

@Thorium
Copy link

Thorium commented Mar 28, 2019

Documentation says early returns are not supported.
What this means?

I think early return could be doable:

if(x){
    /* some return */
    return f;
}
var c = 3;
// ...

with:

if x then
    (* some return *)
    f
else
let c = 3
// ...
@jindraivanek
Copy link
Owner

Yes, its doable in simple situations like this, but there is problematic cases. For example consider this:

        if(x){
            SomeAction();
        } else {
            return f;
        }

        return 0;

This cannot be directly rewritten into F#, because if-else branch must have same type.

To more complicate things, return can be in any combination of if, while, for and switchs.

@willsam100
Copy link

This is possible as I have done it in my own solution.
I solved this by having an intermediate tree that is similar to the F# AST, but captures the notion of return with an expression. A function can then be written to walk (map) the tree, rewriting the Sequential and IfThenElse statements to meet the F# syntax.

Here are the tests that I have over my solution (to sovle you pain of writing C#), which I believe covers most cases.

    [<Test>]
    member this.``convert if else statement with an early return in else`` () = 
        let csharp = 
             """public int Foo()
                {
                    if (x)
                    {
                        SomeAction();
                    } 
                    else {
                        return f;
                    }
    
                    return 0;
                }"""

        let fsharp = 
             """member this.Foo() =
                if x then
                    SomeAction()
                    0
                else f"""
                   
        csharp |> Converter.run |> should equal fsharp

    [<Test>]
    member this.``convert if else statement with early return if first if`` () = 
        let csharp = 
             """public int Foo()
                {
                    if (x)
                        return 0;

                    Bar();
                    return 1;
                }"""

        let fsharp = 
             """member this.Foo() =
                    if x then 0
                    else
                        Bar()
                        1"""
                       
        csharp |> Converter.run |> should equal fsharp

    [<Test>]
    member this.``convert multiple if statements with early returns`` () = 
        let csharp = 
             """public int Foo()
                {
                    if (x)
                        return 0;

                    Baz();
                    if (y)
                    {
                        return 42;
                    }
                    else 
                    {
                        Bar();
                    }
                                        
                    return 1;
                }"""

        let fsharp = 
             """member this.Foo() =
                    if x then 0
                    else
                        Baz()
                        if y then 42
                        else Bar()
                        1"""
                       
        csharp |> Converter.run |> should equal fsharp

NB: I believe the same approach can be done for continue

@knocte
Copy link

knocte commented Apr 2, 2019

Wanted to reply to last comment from @jindraivanek:

For example consider this:

That should be converted to this:

if x then
    SomeAction()
    0
else
    f

@willsam100 does your converter cover this corner case?

@knocte
Copy link

knocte commented Apr 2, 2019

@willsam100 Oops, nevermind, I just read the code of your last comment :)

@jindraivanek
Copy link
Owner

  • I agree it is doable, but there is tricky cases.
  • I now see it is doable for ifs with some reordering of blocks.
  • For for, while it probably can be rewritten as recursive function.
  • I see it as nice to have, it can be useful even if it will be used only in subset of cases (like only ifs, or even just not nested ifs)
  • It should be opt-in, as there can be quite big reordering of code.
  • I will happily accept PR implementing this.

@willsam100
Copy link

If it is helpful, my implementation of this is open-sourced here: https://github.com/willsam100/FShaper

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