cs2fs icon indicating copy to clipboard operation
cs2fs copied to clipboard

Early return could be doable with if-else?

Open Thorium opened this issue 6 years ago • 6 comments
trafficstars

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
// ...

Thorium avatar Mar 28 '19 16:03 Thorium

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.

jindraivanek avatar Mar 28 '19 18:03 jindraivanek

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

willsam100 avatar Apr 01 '19 07:04 willsam100

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 avatar Apr 02 '19 07:04 knocte

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

knocte avatar Apr 02 '19 07:04 knocte

  • 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.

jindraivanek avatar Apr 02 '19 15:04 jindraivanek

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

willsam100 avatar Apr 18 '19 19:04 willsam100