Add labels and gotos. (Fixes #101)

[dcjones]

I've taken a crack at implementing gotos.

What I've done is add new ast nodes SymbolicLabelNode and SymbolicGotoNode mirroring LabelNode and GotoNode but with symbol labels instead of integers. In goto-form, integer labels are assigned along with everything else, so this plays nicely with other control flow.

Rather than introducing new syntax or keywords I just made macros @label and @goto. I figured goto is (should be) used rarely enough that new syntax might not be worth it.

Now we can make local jumps to our hearts' content:

function f()
    @goto a
    @label c
    println("they seem")
    return
    @label b
    println("not what")
    @goto c
    @label a
    println("things are")
    @goto b
    return
end

This is my first foray into the src directory, so there could be something I've totally overlooked.

[timholy]

Impressive, @dcjones!

[JeffBezanson]

This does pretty much work; no real src-related problems.

I suspect the macroexpander will have to handle renaming labels to make them locally unique.

The good news is the new types of AST nodes are not really necessary for this approach. Macros only need to emit Expr objects, and since the front-end (in julia-syntax.scm) lowers them to normal goto and label nodes, the lowered form remains the same as before. The @label macro could just return Expr(:symboliclabel, name).

[dcjones]

Ok, so the AST nodes are only actually needed if I were to introduce new syntax. If no one feels strongly about that, I'll strip those out and update this PR.

[JeffBezanson]

No, new AST nodes are only needed in the internal lowered form, if you need to convey something fundamentally different to the code generator. New surface syntax can be added just in the parser, without touching any other layers.

[dcjones]

I've finished my parser generator backend using this PR. The results are both encouraging and disappointing. Running the generated parser function on my benchmark the first time take ~30 seconds, and the second time ~0.5 seconds. The 0.5 number is a terrific start; it's only about 4-5x slower than the fastest C parser, but the function takes a seemingly excessive time to compile.

Granted, the resulting code is super gnarly, with ~1700 labels and ~3000 gotos. Maybe there's no easy remedy, but is there at least a good way to go about tracking down the bottleneck? The profiler doesn't tell me much here. Is valgrind the next step?

[JeffBezanson]

Probably certain LLVM optimization passes are very complex in the number of basic blocks. You can try commenting out FPM->run(*f) in codegen.cpp.

[dcjones]

That drops it by a few seconds, but it's still ~25 seconds on the first run.

[StefanKarpinski]

Hmm. Seems like a good use case for pre-compilation.

[JeffBezanson]

Aha, I think there are some cases (at least in inference.jl) where we do linear lookup per-label. That will add an n^2 term. We should use a hash table for long functions.

[dcjones]

Updated with goto without the unnecessary AST nodes, plus some simple optimizations. My test case takes 10 seconds now instead of 25. As far as I can tell, most of the time is still spent bouncing around in typeinf, but I don't understand that code well enough to try to optimize it.

[dcjones]

Can we have gotos in 0.3, pretty please?

There are now proper error messages when the same label is defined twice. And labels that are both defined and referenced in the same macro are renamed by macroexpander, which seemed to me the right way to deal with that.

[JeffBezanson]

Looking pretty solid; great job with this. I'll read it in detail in a bit but this seems possible to merge for 0.3.

[dcjones]

Tagging as 0.3 just to pester you. 😉

It's ok if this doesn't make 0.3, but please consider before tagging.

[JeffBezanson]

Is this necessary? It looks like the front end converts all symbolicgotos to plain gotos.

[dcjones]

The interpret.c changes were unnecessary, just leftovers from figuring the right way to do this. I removed them and rebased.

[JeffBezanson]

Ok, I think my only remaining concern here is exception handlers. Looking at the case for break in goto-form shows what's needed. Every statement has a handler-level, and if you branch from a higher handler level to a lower you need to do a (leave ,(- handler-level dest-handler-level)). If the destination handler level is greater, the goto should probably be disallowed.

The really hard problem is finally. For now, we might just have to live with finally blocks not running when you exit the try block with goto.

[vtjnash]

this appears to be missing the code for inlining these correctly

[JeffBezanson]

I don't think it matters, since these are lowered to normal gotos.
On Jun 7, 2014 4:34 PM, "Jameson Nash" notifications@github.com wrote:

this appears to be missing the code for inlining these correctly

—
Reply to this email directly or view it on GitHub
#5699 (comment).

[dcjones]

How's that?

It will throw an error if jumping into a try/catch block, and generate the correct leave expression when jumping out now.

[Keno]

What about finally blocks? I'm in favor of throwing an error for now when jumping out of finally blocks.

[dcjones]

Oh, right. Goto out of a finally throws an error now.

[Keno]

Not exactly what I meant:

try 
@goto a 
finally
println("I won't be executed")
end
@label a

[dcjones]

Ah, that makes more sense.

[jakebolewski]

Does this work with nested try/catch/finally blocks?

[dcjones]

You mean detecting gotos that skip finally blocks? It should. All these examples correctly throw errors.

try
    try
        @goto a
    catch
    finally
    end
    @label a
catch
finally
end

try
    try
        @goto a
    catch
    finally
    end
catch
finally
end
@label a

try
    @goto a
    try
    catch
    finally
    end
catch
finally
end
@label a

[quinnj]

Bump. One of the remaining v0.3 issues.

[vtjnash]

I'm still not convinced that this feature needs to be in 0.3

[StefanKarpinski]

I'm not either, but it seems so very close. The only issue is jumping out of a try/finally, which I don't think we need to support immediately.

[dcjones]

Obviously goto statements aren't the mission-critical feature that everyone's been pining for. But I think this PR is pretty solid now, it should not effect existing behavior, and the addition of goto statements will enable some pretty nice things: notably, non-standard control-flow constructs implemented as macros (e.g. switch statements, multilevel breaks), and very fast pure julia parsers (as mentioned, I've already written a julia ragel backend but need this PR to actually use it).

[JeffBezanson]

Looks like these are now unused?

[dcjones]

Right. I've removed them now.

[JeffBezanson]

While it does seem strange to add a new control flow construct at this point, the implementation is now pretty much impeccable. After all of @dcjones ' work on this, I'm inclined to merge it.

[StefanKarpinski]

+1

[Keno]

Yes, I think we can merge this as well.

[timholy]

Nice! Looking forward to trying it.

[quinnj]

Hey me too! Time to convert all my for and while loops to gotos! :P

[ivarne]

@quinnj Don't forget the else blocks.

[StefanKarpinski]

Am I doing something really obvious wrong?

julia> function foo()
           @label top
           x = 0
           println(x)
           x += 1
           if x < 10
               @goto top
           end
       end
foo (generic function with 1 method)

julia> foo()
ERROR: error compiling foo: unsupported or misplaced expression symbolicgoto in function foo

[dcjones]

It looks like there is an issue with @goto being the last expression in a function. This works:

function foo()
    @label top
    x = 0
    println(x)
    x += 1
    if x < 10
        @goto top
    end
    return
end

I'll see if I can figure out the correct behavior. I guess a special case is needed for dealing with implicit returns.

[StefanKarpinski]

Funny how different people hit entirely different use cases naturally. The first several things I tried all resulted in this problem – clearly that didn't ever happen while you were testing.