Implement try/finally by inlining the finally code.

pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart

Index: pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart
diff --git a/pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart b/pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart
index d411ee447d6c30eb897f48d287c059f2754ca35b..1c4967ac26a95cc03073bb70f57e52698667b6be 100644
--- a/pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart
+++ b/pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart
@@ -131,6 +131,11 @@ abstract class JumpCollector {
   final List<Iterable<LocalVariableElement>> _boxedTryVariables =
       <Iterable<LocalVariableElement>>[];
 
+  /// A stack of all the enclosing finally blocks up to the target of the jump.
+  ///
+  /// There are null entries which correspond to try/catch (no finally).
+  final List<SubbuildFunction> _finallyBlocks = <SubbuildFunction>[];
+
   JumpCollector(this._continuationEnvironment, this.target);
 
   /// True if the collector has not recorded any jumps to its continuation.
@@ -152,9 +157,11 @@ abstract class JumpCollector {
   /// variables that were boxed on entry before invoking the target
   /// continuation.  Call this function before translating a try block and
   /// call [leaveTry] after translating it.
-  void enterTry(Iterable<LocalVariableElement> boxedOnEntry) {
+  void enterTry(Iterable<LocalVariableElement> boxedOnEntry,
+                [SubbuildFunction buildFinallyBlock]) {
     // The boxed variables are maintained as a stack to make leaving easy.
     _boxedTryVariables.add(boxedOnEntry);
+    _finallyBlocks.add(buildFinallyBlock);
   }
 
   /// Remove the most recently added set of variables boxed on entry to a try
@@ -164,6 +171,7 @@ abstract class JumpCollector {
   /// after translating it.
   void leaveTry() {
     _boxedTryVariables.removeLast();
+    _finallyBlocks.removeLast();
   }
 
   void _buildTryExit(IrBuilder builder) {
@@ -174,6 +182,10 @@ abstract class JumpCollector {
         builder.environment.update(variable, value);
       }
     }
+    for (SubbuildFunction buildFinallyBlock in _finallyBlocks.reversed) {
+      if (buildFinallyBlock != null) buildFinallyBlock(builder);
+      if (!builder.isOpen) return;

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

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]

That's pretty serious, thanks for spotting it.

+    }
   }
 }
 
@@ -224,6 +236,8 @@ class ForwardJumpCollector extends JumpCollector {
   void addJump(IrBuilder builder) {
     assert(_continuation == null);
     _buildTryExit(builder);
+    if (!builder.isOpen) return;
+
     ir.InvokeContinuation invoke = new ir.InvokeContinuation.uninitialized();
     builder.add(invoke);
     _invocations.add(invoke);
@@ -322,8 +336,10 @@ class BackwardJumpCollector extends JumpCollector {
 
   void addJump(IrBuilder builder) {
     assert(_continuation.parameters.length <= builder.environment.length);
-    isEmpty = false;
     _buildTryExit(builder);
+    if (!builder.isOpen) return;
+
+    isEmpty = false;
     builder.add(new ir.InvokeContinuation(_continuation,
         builder.environment.index2value.take(_continuation.parameters.length)
             .toList(),
@@ -399,6 +415,9 @@ class IrBuilderSharedState {
   /// A stack of collectors for continues.
   final List<JumpCollector> continueCollectors = <JumpCollector>[];
 
+  /// A stack of enclosing finally blocks, used when translating return.
+  final List<SubbuildFunction> finallyBlocks = <SubbuildFunction>[];
+
   final List<ConstDeclaration> localConstants = <ConstDeclaration>[];
 
   final ExecutableElement currentElement;
@@ -1731,6 +1750,7 @@ abstract class IrBuilder {
       {TryStatementInfo tryStatementInfo,
        SubbuildFunction buildTryBlock,
        List<CatchClauseInfo> catchClauseInfos: const <CatchClauseInfo>[],
+       SubbuildFunction buildFinallyBlock,
        ClosureClassMap closureClassMap}) {
     assert(isOpen);
 
@@ -1763,125 +1783,189 @@ abstract class IrBuilder {
     // scope of the handler.  The mutable bindings are dereferenced at the end
     // of the try block and at the beginning of the catch block, so the
     // variables are unboxed in the catch block and at the join point.
-    JumpCollector join = new ForwardJumpCollector(environment);
-    IrBuilder tryCatchBuilder = makeDelimitedBuilder();
-
-    // Variables treated as mutable in a try are not mutable outside of it.
-    // Work with a copy of the outer builder's mutable variables.
-    tryCatchBuilder.mutableVariables =
-        new Map<Local, ir.MutableVariable>.from(mutableVariables);
-    for (LocalVariableElement variable in tryStatementInfo.boxedOnEntry) {
-      assert(!tryCatchBuilder.isInMutableVariable(variable));
-      ir.Primitive value = tryCatchBuilder.buildLocalVariableGet(variable);
-      tryCatchBuilder.makeMutableVariable(variable);
-      tryCatchBuilder.declareLocalVariable(variable, initialValue: value);
-    }
+    if (catchClauseInfos.isNotEmpty) {
+      JumpCollector join = new ForwardJumpCollector(environment);
+      IrBuilder tryCatchBuilder = makeDelimitedBuilder();
+
+      // Variables treated as mutable in a try are not mutable outside of it.
+      // Work with a copy of the outer builder's mutable variables.
+      tryCatchBuilder.mutableVariables =
+          new Map<Local, ir.MutableVariable>.from(mutableVariables);
+      for (LocalVariableElement variable in tryStatementInfo.boxedOnEntry) {
+        assert(!tryCatchBuilder.isInMutableVariable(variable));
+        ir.Primitive value = tryCatchBuilder.buildLocalVariableGet(variable);
+        tryCatchBuilder.makeMutableVariable(variable);
+        tryCatchBuilder.declareLocalVariable(variable, initialValue: value);
+      }
 
-    IrBuilder tryBuilder = tryCatchBuilder.makeDelimitedBuilder();
+      IrBuilder tryBuilder = tryCatchBuilder.makeDelimitedBuilder();
 
-    void interceptJumps(JumpCollector collector) {
-      collector.enterTry(tryStatementInfo.boxedOnEntry);
-    }
-    void restoreJumps(JumpCollector collector) {
-      collector.leaveTry();
-    }
-    tryBuilder.state.breakCollectors.forEach(interceptJumps);
-    tryBuilder.state.continueCollectors.forEach(interceptJumps);
-    buildTryBlock(tryBuilder);
-    if (tryBuilder.isOpen) {
-      interceptJumps(join);
-      tryBuilder.jumpTo(join);
-      restoreJumps(join);
-    }
-    tryBuilder.state.breakCollectors.forEach(restoreJumps);
-    tryBuilder.state.continueCollectors.forEach(restoreJumps);
-
-    IrBuilder catchBuilder = tryCatchBuilder.makeDelimitedBuilder();
-    for (LocalVariableElement variable in tryStatementInfo.boxedOnEntry) {
-      assert(catchBuilder.isInMutableVariable(variable));
-      ir.Primitive value = catchBuilder.buildLocalVariableGet(variable);
-      // After this point, the variables that were boxed on entry to the try
-      // are no longer treated as mutable.
-      catchBuilder.removeMutableVariable(variable);
-      catchBuilder.environment.update(variable, value);
-    }
+      void interceptJump(JumpCollector collector) {
+        collector.enterTry(tryStatementInfo.boxedOnEntry);
+      }
+      void restoreJump(JumpCollector collector) {
+        collector.leaveTry();
+      }
+      tryBuilder.state.breakCollectors.forEach(interceptJump);
+      tryBuilder.state.continueCollectors.forEach(interceptJump);
+      buildTryBlock(tryBuilder);
+      if (tryBuilder.isOpen) {
+        interceptJump(join);
+        tryBuilder.jumpTo(join);
+        restoreJump(join);
+      }
+      tryBuilder.state.breakCollectors.forEach(restoreJump);
+      tryBuilder.state.continueCollectors.forEach(restoreJump);
+
+      IrBuilder catchBuilder = tryCatchBuilder.makeDelimitedBuilder();
+      for (LocalVariableElement variable in tryStatementInfo.boxedOnEntry) {
+        assert(catchBuilder.isInMutableVariable(variable));
+        ir.Primitive value = catchBuilder.buildLocalVariableGet(variable);
+        // After this point, the variables that were boxed on entry to the try
+        // are no longer treated as mutable.
+        catchBuilder.removeMutableVariable(variable);
+        catchBuilder.environment.update(variable, value);
+      }
 
-    // Handlers are always translated as having both exception and stack trace
-    // parameters.  Multiple clauses do not have to use the same names for
-    // them.  Choose the first of each as the name hint for the respective
-    // handler parameter.
-    ir.Parameter exceptionParameter =
-        new ir.Parameter(catchClauseInfos.first.exceptionVariable);
-    LocalVariableElement traceVariable;
-    CatchClauseInfo catchAll;
-    for (int i = 0; i < catchClauseInfos.length; ++i) {
-      CatchClauseInfo info = catchClauseInfos[i];
-      if (info.type == null) {
-        catchAll = info;
-        catchClauseInfos.length = i;
-        break;
+      // Handlers are always translated as having both exception and stack trace
+      // parameters.  Multiple clauses do not have to use the same names for
+      // them.  Choose the first of each as the name hint for the respective
+      // handler parameter.
+      ir.Parameter exceptionParameter =
+          new ir.Parameter(catchClauseInfos.first.exceptionVariable);
+      LocalVariableElement traceVariable;
+      CatchClauseInfo catchAll;
+      for (int i = 0; i < catchClauseInfos.length; ++i) {
+        CatchClauseInfo info = catchClauseInfos[i];
+        if (info.type == null) {
+          catchAll = info;
+          catchClauseInfos.length = i;
+          break;
+        }
+        if (traceVariable == null) {
+          traceVariable = info.stackTraceVariable;
+        }
       }
-      if (traceVariable == null) {
-        traceVariable = info.stackTraceVariable;
+      ir.Parameter traceParameter = new ir.Parameter(traceVariable);
+      // Expand multiple catch clauses into an explicit if/then/else.  Iterate
+      // them in reverse so the current block becomes the next else block.
+      ir.Expression catchBody;
+      if (catchAll == null) {
+        catchBody = new ir.Rethrow();
+      } else {
+        IrBuilder clauseBuilder = catchBuilder.makeDelimitedBuilder();
+        clauseBuilder.declareLocalVariable(catchAll.exceptionVariable,
+                                           initialValue: exceptionParameter);
+        if (catchAll.stackTraceVariable != null) {
+          clauseBuilder.declareLocalVariable(catchAll.stackTraceVariable,
+                                             initialValue: traceParameter);
+        }
+        catchAll.buildCatchBlock(clauseBuilder);
+        if (clauseBuilder.isOpen) clauseBuilder.jumpTo(join);
+        catchBody = clauseBuilder._root;
       }
-    }
-    ir.Parameter traceParameter = new ir.Parameter(traceVariable);
-    // Expand multiple catch clauses into an explicit if/then/else.  Iterate
-    // them in reverse so the current block becomes the next else block.
-    ir.Expression catchBody;
-    if (catchAll == null) {
-      catchBody = new ir.Rethrow();
-    } else {
-      IrBuilder clauseBuilder = catchBuilder.makeDelimitedBuilder();
-      clauseBuilder.declareLocalVariable(catchAll.exceptionVariable,
-                                         initialValue: exceptionParameter);
-      if (catchAll.stackTraceVariable != null) {
-        clauseBuilder.declareLocalVariable(catchAll.stackTraceVariable,
-                                           initialValue: traceParameter);
+      for (CatchClauseInfo clause in catchClauseInfos.reversed) {
+        IrBuilder clauseBuilder = catchBuilder.makeDelimitedBuilder();
+        clauseBuilder.declareLocalVariable(clause.exceptionVariable,
+                                           initialValue: exceptionParameter);
+        if (clause.stackTraceVariable != null) {
+          clauseBuilder.declareLocalVariable(clause.stackTraceVariable,
+                                             initialValue: traceParameter);
+        }
+        clause.buildCatchBlock(clauseBuilder);
+        if (clauseBuilder.isOpen) clauseBuilder.jumpTo(join);
+        ir.Continuation thenContinuation = new ir.Continuation([]);
+        thenContinuation.body = clauseBuilder._root;
+        ir.Continuation elseContinuation = new ir.Continuation([]);
+        elseContinuation.body = catchBody;
+
+        // Build the type test guarding this clause. We can share the
+        // environment with the nested builder because this part cannot mutate
+        // it.
+        IrBuilder checkBuilder = catchBuilder.makeDelimitedBuilder(environment);
+        ir.Primitive typeMatches =
+            checkBuilder.buildTypeOperator(exceptionParameter,
+                                           clause.type,
+                                           isTypeTest: true);
+        checkBuilder.add(new ir.LetCont.many([thenContinuation,
+                                              elseContinuation],
+                             new ir.Branch(new ir.IsTrue(typeMatches),
+                                           thenContinuation,
+                                           elseContinuation)));
+        catchBody = checkBuilder._root;
       }
-      catchAll.buildCatchBlock(clauseBuilder);
-      if (clauseBuilder.isOpen) clauseBuilder.jumpTo(join);
-      catchBody = clauseBuilder._root;
-    }
-    for (CatchClauseInfo clause in catchClauseInfos.reversed) {
-      IrBuilder clauseBuilder = catchBuilder.makeDelimitedBuilder();
-      clauseBuilder.declareLocalVariable(clause.exceptionVariable,
-                                         initialValue: exceptionParameter);
-      if (clause.stackTraceVariable != null) {
-        clauseBuilder.declareLocalVariable(clause.stackTraceVariable,
-                                           initialValue: traceParameter);
+
+      List<ir.Parameter> catchParameters =
+          <ir.Parameter>[exceptionParameter, traceParameter];
+      ir.Continuation catchContinuation = new ir.Continuation(catchParameters);
+      catchBuilder.add(catchBody);
+      catchContinuation.body = catchBuilder._root;
+
+      tryCatchBuilder.add(
+          new ir.LetHandler(catchContinuation, tryBuilder._root));
+      add(new ir.LetCont(join.continuation, tryCatchBuilder._root));
+      environment = join.environment;
+    } else {
+      // Try/finally.
+      JumpCollector join = new ForwardJumpCollector(environment);
+      IrBuilder tryFinallyBuilder = makeDelimitedBuilder();
+
+      tryFinallyBuilder.mutableVariables =
+          new Map<Local, ir.MutableVariable>.from(mutableVariables);
+      for (LocalVariableElement variable in tryStatementInfo.boxedOnEntry) {
+        assert(!tryFinallyBuilder.isInMutableVariable(variable));
+        ir.Primitive value = tryFinallyBuilder.buildLocalVariableGet(variable);
+        tryFinallyBuilder.makeMutableVariable(variable);
+        tryFinallyBuilder.declareLocalVariable(variable, initialValue: value);
       }
-      clause.buildCatchBlock(clauseBuilder);
-      if (clauseBuilder.isOpen) clauseBuilder.jumpTo(join);
-      ir.Continuation thenContinuation = new ir.Continuation([]);
-      thenContinuation.body = clauseBuilder._root;
-      ir.Continuation elseContinuation = new ir.Continuation([]);
-      elseContinuation.body = catchBody;
-
-      // Build the type test guarding this clause. We can share the environment
-      // with the nested builder because this part cannot mutate it.
-      IrBuilder checkBuilder = catchBuilder.makeDelimitedBuilder(environment);
-      ir.Primitive typeMatches =
-          checkBuilder.buildTypeOperator(exceptionParameter,
-                                         clause.type,
-                                         isTypeTest: true);
-      checkBuilder.add(new ir.LetCont.many([thenContinuation, elseContinuation],
-                           new ir.Branch(new ir.IsTrue(typeMatches),
-                                         thenContinuation,
-                                         elseContinuation)));
-      catchBody = checkBuilder._root;
-    }
 
-    List<ir.Parameter> catchParameters =
-        <ir.Parameter>[exceptionParameter, traceParameter];
-    ir.Continuation catchContinuation = new ir.Continuation(catchParameters);
-    catchBuilder.add(catchBody);
-    catchContinuation.body = catchBuilder._root;
+      IrBuilder tryBuilder = tryFinallyBuilder.makeDelimitedBuilder();
 
-    tryCatchBuilder.add(
-        new ir.LetHandler(catchContinuation, tryBuilder._root));
-    add(new ir.LetCont(join.continuation, tryCatchBuilder._root));
-    environment = join.environment;
+      void interceptJump(JumpCollector collector) {
+        collector.enterTry(tryStatementInfo.boxedOnEntry, buildFinallyBlock);
+      }
+      void restoreJump(JumpCollector collector) {
+        collector.leaveTry();
+      }
+      tryBuilder.state.breakCollectors.forEach(interceptJump);
+      tryBuilder.state.continueCollectors.forEach(interceptJump);
+      tryBuilder.state.finallyBlocks.add(buildFinallyBlock);
+      buildTryBlock(tryBuilder);
+      if (tryBuilder.isOpen) {
+        // To cover control falling off the end of the try block, the finally
+        // code is translated at the join point.  This ensures that it is
+        // correctly outside the scope of the catch handler.
+        join.enterTry(tryStatementInfo.boxedOnEntry);
+        tryBuilder.jumpTo(join);
+        join.leaveTry();
+      }
+      tryBuilder.state.breakCollectors.forEach(restoreJump);
+      tryBuilder.state.continueCollectors.forEach(restoreJump);
+      tryBuilder.state.finallyBlocks.removeLast();
+
+      IrBuilder catchBuilder = tryFinallyBuilder.makeDelimitedBuilder();
+      for (LocalVariableElement variable in tryStatementInfo.boxedOnEntry) {
+        assert(catchBuilder.isInMutableVariable(variable));
+        ir.Primitive value = catchBuilder.buildLocalVariableGet(variable);
+        catchBuilder.removeMutableVariable(variable);
+        catchBuilder.environment.update(variable, value);
+      }
+
+      buildFinallyBlock(catchBuilder);
+      if (catchBuilder.isOpen) {
+        catchBuilder.add(new ir.Rethrow());
+        catchBuilder._current = null;
+      }
+      List<ir.Parameter> catchParameters =
+          <ir.Parameter>[new ir.Parameter(null), new ir.Parameter(null)];
+      ir.Continuation catchContinuation = new ir.Continuation(catchParameters);
+      catchContinuation.body = catchBuilder._root;
+      tryFinallyBuilder.add(
+          new ir.LetHandler(catchContinuation, tryBuilder._root));
+      add(new ir.LetCont(join.continuation, tryFinallyBuilder._root));
+      environment = join.environment;
+      buildFinallyBlock(this);
+    }
   }
 
   /// Create a return statement `return value;` or `return;` if [value] is
@@ -1895,8 +1979,14 @@ abstract class IrBuilder {
     if (value == null) {
       value = buildNullConstant();
     }
-    add(new ir.InvokeContinuation(state.returnContinuation, [value]));
-    _current = null;
+    for (SubbuildFunction buildFinallyBlock in state.finallyBlocks.reversed) {
+      buildFinallyBlock(this);
+      if (!isOpen) break;
+    }
+    if (isOpen) {
+      add(new ir.InvokeContinuation(state.returnContinuation, [value]));
+      _current = null;
+    }
   }
 
   /// Create a blocks of [statements] by applying [build] to all reachable
@@ -2636,4 +2726,4 @@ class SwitchCaseInfo {
   SwitchCaseInfo(this.buildBody);
 
   void addConstant(ir.Primitive constant) => constants.add(constant);
-}
+}