Implement try/finally by inlining the finally code.

Implement try/finally by inlining the finally code.

Try/finally is implemented by inlining.  There is a try/catch to catch
exceptions in the try block.  The catch body contains the finally code
followed by a rethrow.  The code for finally is translated again after the
normal exit of the try block.  Break, continue, and return exits in the try
block have the finally code inlined just before the exit is taken.

Try/catch/finally is not yet supported, it requires some changes to the
assigned variables analysis.

R=asgerf@google.com

Committed: https://github.com/dart-lang/sdk/commit/089ed2965bb7d6ae6a5e8134a67f8a3889b0c6e4

[Kevin Millikin (Google), 2015-06-22 15:36:40]

kmillikin@google.com changed reviewers:
+ asgerf@google.com

[Kevin Millikin (Google), 2015-06-22 15:36:42]

https://codereview.chromium.org/1201983002/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart (right):

https://codereview.chromium.org/1201983002/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart:1782: JumpCollector join = new
ForwardJumpCollector(environment);
This is the code for try/catch.  It is unchanged except for indentation and word
wrapping.

https://codereview.chromium.org/1201983002/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart:1904: // Try/finally.
This is the code for try/finally.  There is a lot of duplication with the code
above.  The duplication will be eliminated when we implement try/catch/finally
next.

https://codereview.chromium.org/1201983002/diff/40001/pkg/pkg.status
File pkg/pkg.status (right):

https://codereview.chromium.org/1201983002/diff/40001/pkg/pkg.status#newcode154
pkg/pkg.status:154: analyzer/test/cancelable_future_test: Crash # Invalid
argument(s)
All these "Invalid argument(s)" failures have something to do with closure
variables but I didn't look more closely.

https://codereview.chromium.org/1201983002/diff/40001/tests/compiler/dart2js_...
File tests/compiler/dart2js_extra/dart2js_extra.status (right):

https://codereview.chromium.org/1201983002/diff/40001/tests/compiler/dart2js_...
tests/compiler/dart2js_extra/dart2js_extra.status:93:
deferred/deferred_class_test: RuntimeError # receiver.get$_collection$_nums is
not a function
I haven't dug deeply into this, but it seems to be a bug found by increasing
test coverage, not a bug in try/finally itself.

[asgerf, 2015-06-23 08:43:40]

l-g-t-m if the bug is fixed.

https://codereview.chromium.org/1201983002/diff/40001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart (right):

https://codereview.chromium.org/1201983002/diff/40001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart:187: if (!builder.isOpen)
return;
The inlined finally block seems to be evaluated in scope of its own catch
handler.

Here is a test:
https://gist.github.com/asgerf/0c79a1d8de3e4819864f

Like the fallthrough exit, it must happen at the join point.

[Kevin Millikin (Google), 2015-06-23 09:26:59]

https://codereview.chromium.org/1201983002/diff/40001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart (right):

https://codereview.chromium.org/1201983002/diff/40001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart:187: if (!builder.isOpen)
return;
On 2015/06/23 08:43:40, asgerf wrote:
> The inlined finally block seems to be evaluated in scope of its own catch
> handler.
> 
> Here is a test:
> https://gist.github.com/asgerf/0c79a1d8de3e4819864f
> 
> Like the fallthrough exit, it must happen at the join point.

That's pretty serious, thanks for spotting it.

[Kevin Millikin (Google), 2015-06-23 14:13:04]

Fixed.  Please take another look.  I've moved the code for finally outside the
scope of the try handler.  This entails binding a continuation outside the
handler that contains the finally code.

The JumpCollector class is modified to support passing an argument in a nicer
way.

This fixes a related stack overflow caused by return in finally: the finally
block was translated in the scope of itself, which caused translation of the
finally block....

[asgerf, 2015-06-23 14:23:45]

LGTM

[Kevin Millikin (Google), 2015-06-24 08:12:57]

Committed patchset #5 (id:80001) manually as
089ed2965bb7d6ae6a5e8134a67f8a3889b0c6e4 (presubmit successful).