Change the collection of continuation jumps.

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 fee4b77d92539de4c368501b8077f3b1da952063..fe2af5a2fe5e806bfd7e26bd1675f1c4fefbbe82 100644
--- a/pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart
+++ b/pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart
@@ -44,6 +44,20 @@ class Environment {
         index2variable = new List<Local>.from(other.index2variable),
         index2value = new List<ir.Primitive>.from(other.index2value);
 
+  /// Construct an environment that is shaped like another one but with a
+  /// fresh parameter for each variable.
+  ///
+  /// The mapping from elements to indexes is shared, not copied.
+  Environment.fresh(Environment other, List<ir.Parameter> parameters)

[Kevin Millikin (Google), 2015/04/07 09:14:55]

`parameters` is an output parameter here: the caller provides a list that is
filled in.  I can't think of another nice and simple way to do it.

I will strengthen the doc comment to make it clear what is going on.

[asgerf, 2015/04/07 13:42:42]

But it's a copy of index2value. Could you just copy index2value after creating
the environment?

[Kevin Millikin (Google), 2015/04/08 10:56:59]

Yeah, I can do that.  I was trying to make it enforced in the API and without
having the caller poke around at the internal bits of the environment.

Since there is currently only one call site and I don't imagine a whole lot more
than that, I'll do as you suggest.

+      : variable2index = other.variable2index,
+        index2variable = new List<Local>.from(other.index2variable),
+        index2value = other.index2variable.map((Local local) {
+          return new ir.Parameter(local);
+        }).toList() {
+    assert(parameters.isEmpty);
+    index2value.forEach(parameters.add);
+  }
+
   get length => index2variable.length;
 
   ir.Primitive operator [](int index) => index2value[index];
@@ -62,6 +76,14 @@ class Environment {
     index2value.add(value);
   }
 
+  void discard(int count) {
+    assert(count <= index2variable.length);
+    // The map from variables to their index are shared, so we cannot remove
+    // the mapping in `variable2index`.
+    index2variable.length -= count;
+    index2value.length -= count;
+  }
+
   ir.Primitive lookup(Local element) {
     assert(invariant(element, variable2index.containsKey(element),
                      message: "Unknown variable: $element."));
@@ -92,39 +114,110 @@ class Environment {
   }
 }
 
-/// A class to collect breaks or continues.
-///
-/// When visiting a potential target of breaks or continues, any breaks or
-/// continues are collected by a JumpCollector and processed later, on demand.
-/// The site of the break or continue is represented by a continuation
-/// invocation that will have its target and arguments filled in later.
-///
-/// The environment of the builder at that point is captured and should not
-/// be subsequently mutated until the jump is resolved.
-class JumpCollector {
+/// The abstract base class of objects that emit jumps to a continuation and
+/// give a handle to the continuation and its environment.
+abstract class JumpCollector {
   final JumpTarget target;
-  final List<ir.InvokeContinuation> _invocations = <ir.InvokeContinuation>[];
-  final List<Environment> _environments = <Environment>[];
-  final List<Iterable<LocalVariableElement>> boxedTryVariables =
+
+  ir.Continuation _continuation = null;
+  final Environment _environment;
+
+  final List<Iterable<LocalVariableElement>> _boxedTryVariables =
       <Iterable<LocalVariableElement>>[];
 
-  JumpCollector(this.target);
+  JumpCollector(this._environment, this.target);
 
-  bool get isEmpty => _invocations.isEmpty;
-  int get length => _invocations.length;
-  List<ir.InvokeContinuation> get invocations => _invocations;
-  List<Environment> get environments => _environments;
+  /// True if the collector has recorded any jumps to its continuation.

[asgerf, 2015/04/07 13:42:42]

has *not* recorded

[Kevin Millikin (Google), 2015/04/08 10:56:59]

Done.

+  bool get isEmpty;
 
-  void addJump(IrBuilder builder) {
-    // Unbox all variables that were boxed on entry to try blocks between the
-    // jump and the target.
-    for (Iterable<LocalVariableElement> boxedOnEntry in boxedTryVariables) {
+  /// The continuation encapsulated by this collector.
+  ir.Continuation get continuation;
+
+  /// The compile-time environment to be used for translating code in the body
+  /// of the continuation.
+  Environment get environment;
+
+  /// Emit a jump to the continuation for a given [IrBuilder].
+  void addJump(IrBuilder builder);
+
+  /// Add a set of variables that were boxed on entry to a try block.
+  ///
+  /// Jumps from a try block to targets outside have to unbox the 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.

[asgerf, 2015/04/07 13:42:41]

Could you start the comment with "All jumps" instead of "Jumps"?

I had to re-read the sentence a few times until I realized that Jumps is a noun
and not a verb.

[Kevin Millikin (Google), 2015/04/08 10:56:59]

Done.

+  void enterTry(Iterable<LocalVariableElement> boxedOnEntry) {
+    // The boxed variables are maintained as a stack to make leaving easy.
+    _boxedTryVariables.add(boxedOnEntry);
+  }
+
+  /// Remove the most recently added set of variables boxed on entry to a try
+  /// block.
+  ///
+  /// Call [enterTry] before translating a try block and call this function
+  /// after translating it.
+  void leaveTry() {
+    _boxedTryVariables.removeLast();
+  }
+
+  void _buildTryExit(IrBuilder builder) {
+    for (Iterable<LocalVariableElement> boxedOnEntry in _boxedTryVariables) {
       for (LocalVariableElement variable in boxedOnEntry) {
         assert(builder.isInMutableVariable(variable));
         ir.Primitive value = builder.buildLocalGet(variable);
         builder.environment.update(variable, value);
       }
     }
+  }
+}
+
+/// A class to collect 'forward' jumps.
+///
+/// A forward jump to a continuation in the sense of the CPS translation is
+/// a jump where the jump is emitted before any code in the body of the
+/// continuation is translated.  They have the property that continuation
+/// parameters and the environment for the translation of the body can be
+/// determined based on the invocations, before translating the body.  A
+/// [ForwardJumpCollector] can encapsulate a continuation where all the
+/// jumps are forward ones.
+///
+/// Examples of forward jumps in the translation are join points of
+/// if-then-else and breaks from loops.
+///
+/// The implementation strategy is that the collector collects invocation
+/// sites and the environments at those sites.  Then it constructs a
+/// continuation 'on demand' after all the jumps are seen.  It determines
+/// continuation parameters, the environment for the translation of code in
+/// the continuation body, and the arguments at the invocation site only
+/// after all the jumps to the continuation are seen.
+class ForwardJumpCollector extends JumpCollector {
+  final List<ir.InvokeContinuation> _invocations = <ir.InvokeContinuation>[];
+  final List<Environment> _environments = <Environment>[];

[asgerf, 2015/04/07 13:42:42]

Could we rename this to _invocationEnvironments? 

It makes the code in _setContinuation more readable. We currently have
'_environments' and '_environment' which does nothing to emphasize what is what.

As a side effect, it also helps emphasize that the two lists declared here are
paired.

[Kevin Millikin (Google), 2015/04/08 10:56:59]

Done.

+
+  /// Construct a collector with a given base environment.
+  ///
+  /// The base environment is the one in scope at the site that the
+  /// continuation represented by this collector will be bound.  The
+  /// environment is copied by the collector.  Subsequent mutation of the
+  /// original environment will not effect the collector.

[asgerf, 2015/04/07 13:42:41]

affect

[Kevin Millikin (Google), 2015/04/08 10:56:59]

Done.

+  ForwardJumpCollector(Environment environment, {JumpTarget target: null})
+      : super(new Environment.from(environment), target);
+
+  bool get isEmpty => _invocations.isEmpty;
+
+  ir.Continuation get continuation {
+    if (_continuation == null) _setContinuation();
+    return _continuation;
+  }
+
+  Environment get environment {
+    if (_continuation == null) _setContinuation();
+    return _environment;
+  }
+
+  void addJump(IrBuilder builder) {
+    assert(_continuation == null);
+    _buildTryExit(builder);
     ir.InvokeContinuation invoke = new ir.InvokeContinuation.uninitialized();
     builder.add(invoke);
     _invocations.add(invoke);
@@ -134,24 +227,111 @@ class JumpCollector {
     // less likely to mutate it?
   }
 
-  /// Add a set of variables that were boxed on entry to a try block.
-  ///
-  /// Jumps from a try block to targets outside have to unbox the 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) {
-    // The boxed variables are maintained as a stack to make leaving easy.
-    boxedTryVariables.add(boxedOnEntry);
+  void _setContinuation() {
+    assert(_continuation == null);
+    // We have seen all invocations of this continuation, and recorded the
+    // environment in effect at each invocation site.
+
+    // Compute the union of the assigned variables reaching the continuation.
+    //
+    // There is a continuation parameter for each environment variable
+    // that has a different value (from the environment in scope at the
+    // continuation binding) on some path.  `_environment` is initially a copy
+    // of the environment in scope at the continuation binding.  Compute the
+    // continuation parameters and add them to `_environment` so it will become
+    // the one in scope for the continuation body.

[asgerf, 2015/04/07 13:42:41]

'_environment' and 'env' (below) are bad names when we are dealing with multiple
environments.

Possibly rename '_environment' to '_targetEnvironment'? Or introduce a local
alias with a better name.

As for 'env' below, how about 'jumpEnv' or 'jumpSiteEnv', 'invocationEnv',
'invocationSiteEnv' or something like that?

[Kevin Millikin (Google), 2015/04/08 10:56:59]

Done.

+    List<ir.Parameter> parameters = <ir.Parameter>[];
+    if (_environments.isNotEmpty) {
+      for (int varIndex = 0; varIndex < _environment.length; ++varIndex) {
+        for (Environment env in _environments) {
+          if (env[varIndex] != _environment[varIndex]) {
+            ir.Parameter parameter =
+                new ir.Parameter(_environment.index2variable[varIndex]);
+            _environment.index2value[varIndex] = parameter;
+            parameters.add(parameter);
+            break;
+          }
+        }
+      }
+    }
+    _continuation = new ir.Continuation(parameters);
+
+    // Compute the intersection of the parameters with the environments at
+    // each continuation invocation.  Initialize the invocations.
+    for (int jumpIndex = 0; jumpIndex < _invocations.length; ++jumpIndex) {
+      Environment currentEnvironment = _environments[jumpIndex];

[asgerf, 2015/04/07 13:42:42]

Again, something like 'jumpEnvironment' would be better than
'currentEnvironment'.

+      List<ir.Reference> arguments = <ir.Reference>[];
+      if (parameters.isNotEmpty) {

[asgerf, 2015/04/07 13:42:42]

I think we should make 'parameters' the outer loop and lose all this bailout
code. E.g:

int varIndex = 0;
for (ir.Parameter param in parameters) {
  varIndex = bindingEnvironment.index2value.indexOf(param, varIndex);
  arguments.add(new ir.Reference(jumpEnvironment[varIndex]));
}

[Kevin Millikin (Google), 2015/04/08 10:56:59]

Done.  It is probably better, though not simpler, to compute the parameter
indexes as a list of indexes above, then just iterate that rather than
repeatedly searching for their index.

+        arguments.length = parameters.length;
+        int argIndex = 0;
+        for (int varIndex = 0; varIndex < _environment.length; ++varIndex) {
+          if (_environment[varIndex] == parameters[argIndex]) {
+            arguments[argIndex++] =
+                new ir.Reference(currentEnvironment[varIndex]);
+            if (argIndex == parameters.length) break;
+          }
+        }
+      }
+      ir.InvokeContinuation invocation = _invocations[jumpIndex];
+      invocation.continuation = new ir.Reference(_continuation);
+      invocation.arguments = arguments;
+    }
   }
+}
 
-  /// Remove the most recently added set of variables boxed on entry to a try
-  /// block.
+/// A class to collect 'backward' jumps.
+///
+/// A backward jump to a continuation in the sense of the CPS translation is
+/// a jump where some code in the body of the continuation is translated
+/// before the jump is emitted.  They have the property that the
+/// continuation parameters and the environment for the translation of the
+/// body must be determined before emitting all the invocations.  A
+/// [BackwardJumpCollector] can ecapsulate a continuation where some jumps
+/// are backward ones.
+///
+/// Examples of backward jumps in the translation are the recursive
+/// invocations of loop continuations.
+///
+/// The implementation strategy is that the collector inserts a continuation
+/// parameter for each variable in scope at the entry to the continuation,
+/// before emitting any jump to the continuation.  When a jump is added, it
+/// is given an argument for each continuation parameter.
+class BackwardJumpCollector extends JumpCollector {
+  List<ir.Parameter> _parameters;
+
+  /// Construct a collector with a given base environment.
   ///
-  /// Call [enterTry] before translating a try block and call this function
-  /// after translating it.
-  void leaveTry() {
-    boxedTryVariables.removeLast();
+  /// The base environment is the one in scope at the site that the
+  /// continuation represented by this collector will be bound.  The
+  /// translation of the continuation body will use an environment with the
+  /// same shape, but with fresh continuation parameters for each variable.
+  factory BackwardJumpCollector(Environment environment,
+                                {JumpTarget target: null}) {
+    List<ir.Parameter> parameters = <ir.Parameter>[];
+    Environment fresh = new Environment.fresh(environment, parameters);
+    return new BackwardJumpCollector._internal(fresh, parameters, target);
+  }
+
+  BackwardJumpCollector._internal(Environment environment,
+                                  this._parameters,
+                                  JumpTarget target)
+      : super(environment, target) {
+    _continuation = new ir.Continuation(_parameters, isRecursive: true);
+  }
+
+  bool isEmpty = true;
+
+  ir.Continuation get continuation => _continuation;
+  Environment get environment => _environment;
+
+  void addJump(IrBuilder builder) {
+    assert(_parameters.length <= builder.environment.length);
+    isEmpty = false;
+    _buildTryExit(builder);
+    builder.add(new ir.InvokeContinuation(_continuation,
+        builder.environment.index2value.take(_parameters.length).toList(),
+        isRecursive: true));
+    builder._current = null;
   }
 }
 
@@ -366,14 +546,17 @@ abstract class IrBuilder {
 
   /// Construct a delimited visitor for visiting a subtree.
   ///
-  /// The delimited visitor has its own compile-time environment mapping
-  /// local variables to their values, which is initially a copy of the parent
-  /// environment.  It has its own context for building an IR expression, so
-  /// the built expression is not plugged into the parent's context.
-  IrBuilder makeDelimitedBuilder() {
+  /// Build a subterm that is not (yet) connected to the CPS term.  The
+  /// delimited visitor has its own has its own context for building an IR
+  /// expression, so the built expression is not plugged into the parent's
+  /// context.  It has its own compile-time environment mapping local
+  /// variables to their values.  If an optional environment argument is
+  /// supplied, it is used as the builder's initial environment.  Otherwise
+  /// the environment is initially a copy of the parent builder's environment.
+  IrBuilder makeDelimitedBuilder([Environment env = null]) {
     return _makeInstance()
         ..state = state
-        ..environment = new Environment.from(environment);
+        ..environment = env != null ? env : new Environment.from(environment);
   }
 
   /// Construct a builder for making constructor field initializers.
@@ -580,7 +763,6 @@ abstract class IrBuilder {
       ir.Primitive condition,
       ir.Primitive buildThenExpression(IrBuilder builder),
       ir.Primitive buildElseExpression(IrBuilder builder)) {
-
     assert(isOpen);
 
     // The then and else expressions are delimited.
@@ -591,16 +773,23 @@ abstract class IrBuilder {
 
     // Treat the values of the subexpressions as named values in the
     // environment, so they will be treated as arguments to the join-point
-    // continuation.
+    // continuation.  We know the environments are the right size because
+    // expressions cannot introduce variable bindings.
     assert(environment.length == thenBuilder.environment.length);
     assert(environment.length == elseBuilder.environment.length);
+    // Optimistically assume that the value in the incoming environment is the
+    // value of the first subexpression, which might not even be in scope
+    // because it's bound in the first subexpression.  However, if that is the
+    // case, it will necessarily differ from the value of the other
+    // subexpression and cause the introduction of a join-point continuation
+    // parameter.  If the two values do happen to be the same, this will
+    // avoid inserting a useless continuation parameter.

[asgerf, 2015/04/07 13:42:42]

I really like using the environment like this. Before this CL I hadn't even
realized we used the environment for this purpose, but now it's a lot more
clear.

With that said, at a first glance, the lengthy comment suggests that this is an
important optimization and that the following lines have something to do with
that optimization.

Could you emphasize that we just need to extend the environment with an
arbitrary placeholder value? Using thenValue sort of happens to generate a
better result, but that's not the most important thing happening here.

[Kevin Millikin (Google), 2015/04/08 10:56:59]

I've mentioned that first, though I thought the comment before the asserts was
serving that purpose.  I've left the long-winded comment in place.  It's not
intended to make this sound like an important optimization, but I do think it's
necessary to justify its safety.

[asgerf, 2015/04/08 11:10:21]

You are right, I overlooked the comment before the asserts.

+    environment.extend(null, thenValue);
     thenBuilder.environment.extend(null, thenValue);
     elseBuilder.environment.extend(null, elseValue);
-    JumpCollector jumps = new JumpCollector(null);
-    jumps.addJump(thenBuilder);
-    jumps.addJump(elseBuilder);
-    ir.Continuation joinContinuation =
-        createJoin(environment.length + 1, jumps);
+    JumpCollector join = new ForwardJumpCollector(environment);
+    thenBuilder.jumpTo(join);
+    elseBuilder.jumpTo(join);
 
     // Build the term
     //   let cont join(x, ..., result) = [] in
@@ -612,15 +801,17 @@ abstract class IrBuilder {
     ir.Continuation elseContinuation = new ir.Continuation([]);
     thenContinuation.body = thenBuilder._root;
     elseContinuation.body = elseBuilder._root;
-    add(new ir.LetCont(joinContinuation,
+    add(new ir.LetCont(join.continuation,
             new ir.LetCont.many(<ir.Continuation>[thenContinuation,
                                                   elseContinuation],
                 new ir.Branch(new ir.IsTrue(condition),
                               thenContinuation,
                               elseContinuation))));
+    environment = join.environment;
+    environment.discard(1);
     return (thenValue == elseValue)
         ? thenValue
-        : joinContinuation.parameters.last;
+        : join.continuation.parameters.last;
   }
 
   /**
@@ -887,16 +1078,15 @@ abstract class IrBuilder {
                           thenContinuation,
                           elseContinuation));
 
-    ir.Continuation joinContinuation;  // Null if there is no join.
+    JumpCollector join;  // Null if there is no join.
     if (thenBuilder.isOpen && elseBuilder.isOpen) {
       // There is a join-point continuation.  Build the term
       // 'let cont join(x, ...) = [] in Result' and plug invocations of the
       // join-point continuation into the then and else continuations.
-      JumpCollector jumps = new JumpCollector(null);
-      jumps.addJump(thenBuilder);
-      jumps.addJump(elseBuilder);
-      joinContinuation = createJoin(environment.length, jumps);
-      result = new ir.LetCont(joinContinuation, result);
+      join = new ForwardJumpCollector(environment);
+      thenBuilder.jumpTo(join);
+      elseBuilder.jumpTo(join);
+      result = new ir.LetCont(join.continuation, result);
     }
 
     // The then or else term root could be null, but not both.  If there is
@@ -910,7 +1100,7 @@ abstract class IrBuilder {
     elseContinuation.body = elseBuilder._root;
 
     add(result);
-    if (joinContinuation == null) {
+    if (join == null) {
       // At least one subexpression is closed.
       if (thenBuilder.isOpen) {
         if (thenBuilder._root != null) _current = thenBuilder._current;
@@ -921,43 +1111,21 @@ abstract class IrBuilder {
       } else {
         _current = null;
       }
+    } else {
+      environment = join.environment;
     }
   }
 
-  void jumpTo(ir.Continuation continuation) {
-    assert(isOpen);
-    assert(environment.length >= continuation.parameters.length);
-    ir.InvokeContinuation jump = new ir.InvokeContinuation.uninitialized();
-    jump.continuation = new ir.Reference(continuation);
-    jump.arguments = new List<ir.Reference>.generate(
-        continuation.parameters.length, (i) {
-      return new ir.Reference(environment[i]);
-    });
-    add(jump);
-    _current = null;
+  void jumpTo(JumpCollector collector) {
+    collector.addJump(this);
   }
 
-  /// Invoke a join-point continuation that contains arguments for all local
-  /// variables.
-  ///
-  /// Given the continuation and a list of uninitialized invocations, fill
-  /// in each invocation with the continuation and appropriate arguments.
-  void invokeFullJoin(ir.Continuation join,
-                      JumpCollector jumps,
-                      {recursive: false}) {
-    // TODO(kmillikin): If the JumpCollector collected open IrBuilders instead
-    // of pairs of invocations and environments, we could use IrBuilder.jumpTo
-    // here --- the code is almost the same.
-    join.isRecursive = recursive;
-    for (int i = 0; i < jumps.length; ++i) {
-      Environment currentEnvironment = jumps.environments[i];
-      ir.InvokeContinuation invoke = jumps.invocations[i];
-      invoke.continuation = new ir.Reference(join);
-      invoke.arguments = new List<ir.Reference>.generate(
-          join.parameters.length,
-          (i) => new ir.Reference(currentEnvironment[i]));
-      invoke.isRecursive = recursive;
-    }
+  void addRecursiveContinuation(BackwardJumpCollector collector) {
+    assert(environment.length == collector.environment.length);
+    add(new ir.LetCont(collector.continuation,
+            new ir.InvokeContinuation(collector.continuation,
+                                      environment.index2value)));
+    environment = collector.environment;
   }
 
   /// Creates a for loop in which the initializer, condition, body, update are
@@ -991,7 +1159,7 @@ abstract class IrBuilder {
     // [[initializer]];
     // let cont loop(x, ...) =
     //     let prim cond = [[condition]] in
-    //     let cont break() = [[successor]] in
+    //     let cont break(x, ...) = [[successor]] in
     //     let cont exit() = break(v, ...) in
     //     let cont body() =
     //         _enterForLoopBody();
@@ -1009,28 +1177,33 @@ abstract class IrBuilder {
     // statement occurs in the exit continuation).  If there is only one
     // invocation of the continue continuation (i.e., no continues in the
     // body), the continue continuation is inlined in the body.
-
     _enterForLoopInitializer(closureScope, loopVariables);
-
     buildInitializer(this);
 
-    IrBuilder condBuilder = makeRecursiveBuilder();
-    ir.Primitive condition = buildCondition(condBuilder);
+    JumpCollector loop = new BackwardJumpCollector(environment);
+    addRecursiveContinuation(loop);
+
+    ir.Primitive condition = buildCondition(this);
     if (condition == null) {
       // If the condition is empty then the body is entered unconditionally.
-      condition = condBuilder.buildBooleanLiteral(true);
+      condition = buildBooleanLiteral(true);
     }
+    JumpCollector breakCollector =
+        new ForwardJumpCollector(environment, target: target);
 
-    JumpCollector breakCollector = new JumpCollector(target);
-    JumpCollector continueCollector = new JumpCollector(target);
-    state.breakCollectors.add(breakCollector);
-    state.continueCollectors.add(continueCollector);
-
-    IrBuilder outerBodyBuilder = condBuilder.makeDelimitedBuilder();
+    // Use a pair of builders for the body, one for the entry code if any
+    // and one for the body itself.  We only decide whether to insert a
+    // continue continuation until after translating the body and there is no
+    // way to insert such a continuation between the entry code and the body
+    // if they are translated together.
+    IrBuilder outerBodyBuilder = makeDelimitedBuilder();
     outerBodyBuilder._enterForLoopBody(closureScope, loopVariables);
+    JumpCollector continueCollector =
+        new ForwardJumpCollector(outerBodyBuilder.environment, target: target);
 
     IrBuilder innerBodyBuilder = outerBodyBuilder.makeDelimitedBuilder();
-
+    state.breakCollectors.add(breakCollector);
+    state.continueCollectors.add(continueCollector);
     buildBody(innerBodyBuilder);
     assert(state.breakCollectors.last == breakCollector);
     assert(state.continueCollectors.last == continueCollector);
@@ -1040,17 +1213,34 @@ abstract class IrBuilder {
     // The binding of the continue continuation should occur as late as
     // possible, that is, at the nearest common ancestor of all the continue
     // sites in the body.  However, that is difficult to compute here, so it
-    // is instead placed just outside the body of the body continuation.
+    // is instead placed just outside the translation of the loop body.  In
+    // the case where there are no continues in the body, the updates are
+    // translated immediately after the body.
     bool hasContinues = !continueCollector.isEmpty;
-    IrBuilder updateBuilder = hasContinues
-        ? outerBodyBuilder.makeRecursiveBuilder()
-        : innerBodyBuilder;
+    IrBuilder updateBuilder;
+    if (hasContinues) {
+      if (innerBodyBuilder.isOpen) innerBodyBuilder.jumpTo(continueCollector);
+      updateBuilder = makeDelimitedBuilder(continueCollector.environment);
+    } else {
+      updateBuilder = innerBodyBuilder;
+    }
     updateBuilder._enterForLoopUpdate(closureScope, loopVariables);
     buildUpdate(updateBuilder);
+    if (updateBuilder.isOpen) updateBuilder.jumpTo(loop);
+    // Connect the inner and outer body builders.  This is done only after
+    // it is guaranteed that the updateBuilder has a non-empty term.
+    if (hasContinues) {
+      outerBodyBuilder.add(new ir.LetCont(continueCollector.continuation,
+          innerBodyBuilder._root));
+      continueCollector.continuation.body = updateBuilder._root;
+    } else {
+      outerBodyBuilder.add(innerBodyBuilder._root);
+    }
 
-    // Create body entry and loop exit continuations and a branch to them.
-    ir.Continuation bodyContinuation = new ir.Continuation([]);
+    // Create loop exit and body entry continuations and a branch to them.
     ir.Continuation exitContinuation = new ir.Continuation([]);
+    ir.Continuation bodyContinuation = new ir.Continuation([]);
+    bodyContinuation.body = outerBodyBuilder._root;
     // Note the order of continuations: the first one is the one that will
     // be filled by LetCont.plug.
     ir.LetCont branch =
@@ -1060,57 +1250,19 @@ abstract class IrBuilder {
                           bodyContinuation,
                           exitContinuation));
     // If there are breaks in the body, then there must be a join-point
-    // continuation for the normal exit and the breaks.
+    // continuation for the normal exit and the breaks.  Otherwise, the
+    // successor is translated in the hole in the exit continuation.
     bool hasBreaks = !breakCollector.isEmpty;
-    ir.LetCont letJoin;
-    if (hasBreaks) {
-      letJoin = new ir.LetCont(null, branch);
-      condBuilder.add(letJoin);
-      condBuilder._current = branch;
-    } else {
-      condBuilder.add(branch);
-    }
-    ir.Continuation continueContinuation;
-    if (hasContinues) {
-      // If there are continues in the body, we need a named continue
-      // continuation as a join point.
-      continueContinuation = new ir.Continuation(updateBuilder._parameters);
-      if (innerBodyBuilder.isOpen) continueCollector.addJump(innerBodyBuilder);
-      invokeFullJoin(continueContinuation, continueCollector);
-    }
-    ir.Continuation loopContinuation =
-        new ir.Continuation(condBuilder._parameters);
-    if (updateBuilder.isOpen) {
-      JumpCollector backEdges = new JumpCollector(null);
-      backEdges.addJump(updateBuilder);
-      invokeFullJoin(loopContinuation, backEdges, recursive: true);
-    }
-
-    // Fill in the body and possible continue continuation bodies.  Do this
-    // only after it is guaranteed that they are not empty.
-    if (hasContinues) {
-      continueContinuation.body = updateBuilder._root;
-      outerBodyBuilder.add(new ir.LetCont(continueContinuation,
-                                          innerBodyBuilder._root));
-    } else {
-      outerBodyBuilder.add(innerBodyBuilder._root);
-    }
-    bodyContinuation.body = outerBodyBuilder._root;
-
-    loopContinuation.body = condBuilder._root;
-    add(new ir.LetCont(loopContinuation,
-            new ir.InvokeContinuation(loopContinuation,
-                environment.index2value)));
+    ir.LetCont letBreak;
     if (hasBreaks) {
-      _current = branch;
-      environment = condBuilder.environment;
-      breakCollector.addJump(this);
-      letJoin.continuations =
-          <ir.Continuation>[createJoin(environment.length, breakCollector)];
-      _current = letJoin;
+      IrBuilder exitBuilder = makeDelimitedBuilder();
+      exitBuilder.jumpTo(breakCollector);
+      exitContinuation.body = exitBuilder._root;
+      letBreak = new ir.LetCont(breakCollector.continuation, branch);
+      add(letBreak);
+      environment = breakCollector.environment;
     } else {
-      _current = condBuilder._current;
-      environment = condBuilder.environment;
+      add(branch);
     }
   }
 
@@ -1149,37 +1301,50 @@ abstract class IrBuilder {
     //   s;
     // }
 
-    // The condition and body are delimited.
-    IrBuilder condBuilder = makeRecursiveBuilder();
-
+    // Fill the current hole with:
+    // let prim expressionReceiver = [[e]] in
+    // let cont iteratorInvoked(iterator) =
+    //     [ ]
+    // in expressionReceiver.iterator () iteratorInvoked
     ir.Primitive expressionReceiver = buildExpression(this);
-    List<ir.Primitive> emptyArguments = new List<ir.Primitive>();
-
+    List<ir.Primitive> emptyArguments = <ir.Primitive>[];
     ir.Parameter iterator = new ir.Parameter(null);
     ir.Continuation iteratorInvoked = new ir.Continuation([iterator]);
     add(new ir.LetCont(iteratorInvoked,
         new ir.InvokeMethod(expressionReceiver,
-            new Selector.getter("iterator", null), iteratorInvoked,
+            new Selector.getter("iterator", null),
+            iteratorInvoked,
             emptyArguments)));
 
+    // Fill with:
+    // let cont loop(x, ...) =
+    //     let cont moveNextInvoked(condition) =
+    //         [ ]
+    //     in iterator.moveNext () moveNextInvoked
+    // in loop(v, ...)
+    JumpCollector loop = new BackwardJumpCollector(environment, target: target);
+    addRecursiveContinuation(loop);
     ir.Parameter condition = new ir.Parameter(null);
     ir.Continuation moveNextInvoked = new ir.Continuation([condition]);
-    condBuilder.add(new ir.LetCont(moveNextInvoked,
+    add(new ir.LetCont(moveNextInvoked,
         new ir.InvokeMethod(iterator,
             new Selector.call("moveNext", null, 0),
-            moveNextInvoked, emptyArguments)));
-
-    JumpCollector breakCollector = new JumpCollector(target);
-    JumpCollector continueCollector = new JumpCollector(target);
-    state.breakCollectors.add(breakCollector);
-    state.continueCollectors.add(continueCollector);
+            moveNextInvoked,
+            emptyArguments)));
 
-    IrBuilder bodyBuilder = condBuilder.makeDelimitedBuilder();
+    // As a delimited term, build:
+    // <<BODY>> =
+    //   _enterScope();
+    //   [[variableDeclaration]]
+    //   let cont currentInvoked(currentValue) =
+    //       [[a = currentValue]];
+    //       [ ]
+    //   in iterator.current () currentInvoked
+    IrBuilder bodyBuilder = makeDelimitedBuilder();
     bodyBuilder._enterScope(closureScope);
     if (buildVariableDeclaration != null) {
       buildVariableDeclaration(bodyBuilder);
     }
-
     ir.Parameter currentValue = new ir.Parameter(null);
     ir.Continuation currentInvoked = new ir.Continuation([currentValue]);
     bodyBuilder.add(new ir.LetCont(currentInvoked,
@@ -1198,15 +1363,27 @@ abstract class IrBuilder {
       bodyBuilder.buildDynamicSet(receiver, variableSelector, currentValue);
     }
 
+    // Translate the body in the hole in the delimited term above, and add
+    // a jump to the loop if control flow is live after the body.
+    JumpCollector breakCollector =
+        new ForwardJumpCollector(environment, target: target);
+    state.breakCollectors.add(breakCollector);
+    state.continueCollectors.add(loop);
     buildBody(bodyBuilder);
     assert(state.breakCollectors.last == breakCollector);
-    assert(state.continueCollectors.last == continueCollector);
+    assert(state.continueCollectors.last == loop);
     state.breakCollectors.removeLast();
     state.continueCollectors.removeLast();
+    if (bodyBuilder.isOpen) bodyBuilder.jumpTo(loop);
 
     // Create body entry and loop exit continuations and a branch to them.
-    ir.Continuation bodyContinuation = new ir.Continuation([]);
+    //
+    // let cont exit() = [ ]
+    //      and body() = <<BODY>>
+    // in branch condition (body, exit)
     ir.Continuation exitContinuation = new ir.Continuation([]);
+    ir.Continuation bodyContinuation = new ir.Continuation([]);
+    bodyContinuation.body = bodyBuilder._root;
     // Note the order of continuations: the first one is the one that will
     // be filled by LetCont.plug.
     ir.LetCont branch =
@@ -1216,37 +1393,19 @@ abstract class IrBuilder {
                           bodyContinuation,
                           exitContinuation));
     // If there are breaks in the body, then there must be a join-point
-    // continuation for the normal exit and the breaks.
+    // continuation for the normal exit and the breaks.  Otherwise, the
+    // successor is translated in the hole in the exit continuation.
     bool hasBreaks = !breakCollector.isEmpty;
-    ir.LetCont letJoin;
+    ir.LetCont letBreak;
     if (hasBreaks) {
-      letJoin = new ir.LetCont(null, branch);
-      condBuilder.add(letJoin);
-      condBuilder._current = branch;
+      IrBuilder exitBuilder = makeDelimitedBuilder();
+      exitBuilder.jumpTo(breakCollector);
+      exitContinuation.body = exitBuilder._root;
+      letBreak = new ir.LetCont(breakCollector.continuation, branch);
+      add(letBreak);
+      environment = breakCollector.environment;
     } else {
-      condBuilder.add(branch);
-    }
-    ir.Continuation loopContinuation =
-        new ir.Continuation(condBuilder._parameters);
-    if (bodyBuilder.isOpen) continueCollector.addJump(bodyBuilder);
-    invokeFullJoin(
-        loopContinuation, continueCollector, recursive: true);
-    bodyContinuation.body = bodyBuilder._root;
-
-    loopContinuation.body = condBuilder._root;
-    add(new ir.LetCont(loopContinuation,
-            new ir.InvokeContinuation(loopContinuation,
-                                      environment.index2value)));
-    if (hasBreaks) {
-      _current = branch;
-      environment = condBuilder.environment;
-      breakCollector.addJump(this);
-      letJoin.continuations =
-          <ir.Continuation>[createJoin(environment.length, breakCollector)];
-      _current = letJoin;
-    } else {
-      _current = condBuilder._current;
-      environment = condBuilder.environment;
+      add(branch);
     }
   }
 
@@ -1268,34 +1427,39 @@ abstract class IrBuilder {
     //     let prim cond = [[condition]] in
     //     let cont break(x, ...) = [[successor]] in
     //     let cont exit() = break(v, ...)
-    //          and body() = [[body]]; continue(v, ...)
+    //          and body() =
+    //                _enterScope();
+    //                [[body]];
+    //                continue(v, ...)
     //     in branch cond (body, exit)
     // in continue(v, ...)
     //
     // If there are no breaks in the body, the break continuation is inlined
     // in the exit continuation (i.e., the translation of the successor
     // statement occurs in the exit continuation).
+    JumpCollector loop = new BackwardJumpCollector(environment, target: target);
+    addRecursiveContinuation(loop);
 
-    // The condition and body are delimited.
-    IrBuilder condBuilder = makeRecursiveBuilder();
-    ir.Primitive condition = buildCondition(condBuilder);
+    ir.Primitive condition = buildCondition(this);
 
-    JumpCollector breakCollector = new JumpCollector(target);
-    JumpCollector continueCollector = new JumpCollector(target);
-    state.breakCollectors.add(breakCollector);
-    state.continueCollectors.add(continueCollector);
+    JumpCollector breakCollector =
+        new ForwardJumpCollector(environment, target: target);
 
-    IrBuilder bodyBuilder = condBuilder.makeDelimitedBuilder();
+    IrBuilder bodyBuilder = makeDelimitedBuilder();
     bodyBuilder._enterScope(closureScope);
+    state.breakCollectors.add(breakCollector);
+    state.continueCollectors.add(loop);
     buildBody(bodyBuilder);
     assert(state.breakCollectors.last == breakCollector);
-    assert(state.continueCollectors.last == continueCollector);
+    assert(state.continueCollectors.last == loop);
     state.breakCollectors.removeLast();
     state.continueCollectors.removeLast();
+    if (bodyBuilder.isOpen) bodyBuilder.jumpTo(loop);
 
     // Create body entry and loop exit continuations and a branch to them.
-    ir.Continuation bodyContinuation = new ir.Continuation([]);
     ir.Continuation exitContinuation = new ir.Continuation([]);
+    ir.Continuation bodyContinuation = new ir.Continuation([]);
+    bodyContinuation.body = bodyBuilder._root;
     // Note the order of continuations: the first one is the one that will
     // be filled by LetCont.plug.
     ir.LetCont branch =
@@ -1305,36 +1469,19 @@ abstract class IrBuilder {
                           bodyContinuation,
                           exitContinuation));
     // If there are breaks in the body, then there must be a join-point
-    // continuation for the normal exit and the breaks.
+    // continuation for the normal exit and the breaks.  Otherwise, the
+    // successor is translated in the hole in the exit continuation.
     bool hasBreaks = !breakCollector.isEmpty;
-    ir.LetCont letJoin;
+    ir.LetCont letBreak;
     if (hasBreaks) {
-      letJoin = new ir.LetCont(null, branch);
-      condBuilder.add(letJoin);
-      condBuilder._current = branch;
+      IrBuilder exitBuilder = makeDelimitedBuilder();
+      exitBuilder.jumpTo(breakCollector);
+      exitContinuation.body = exitBuilder._root;
+      letBreak = new ir.LetCont(breakCollector.continuation, branch);
+      add(letBreak);
+      environment = breakCollector.environment;
     } else {
-      condBuilder.add(branch);
-    }
-    ir.Continuation loopContinuation =
-        new ir.Continuation(condBuilder._parameters);
-    if (bodyBuilder.isOpen) continueCollector.addJump(bodyBuilder);
-    invokeFullJoin(loopContinuation, continueCollector, recursive: true);
-    bodyContinuation.body = bodyBuilder._root;
-
-    loopContinuation.body = condBuilder._root;
-    add(new ir.LetCont(loopContinuation,
-            new ir.InvokeContinuation(loopContinuation,
-                                      environment.index2value)));
-    if (hasBreaks) {
-      _current = branch;
-      environment = condBuilder.environment;
-      breakCollector.addJump(this);
-      letJoin.continuations =
-          <ir.Continuation>[createJoin(environment.length, breakCollector)];
-      _current = letJoin;
-    } else {
-      _current = condBuilder._current;
-      environment = condBuilder.environment;
+      add(branch);
     }
   }
 
@@ -1362,99 +1509,66 @@ abstract class IrBuilder {
     //       in branch cond (repeat, exit)
     //   in [[body]]; continue(v, ...)
     // in loop(v, ...)
-    IrBuilder bodyBuilder = makeRecursiveBuilder();
-    IrBuilder continueBuilder = bodyBuilder.makeRecursiveBuilder();
+    IrBuilder loopBuilder = makeDelimitedBuilder();
+    JumpCollector loop =
+        new BackwardJumpCollector(loopBuilder.environment, target: target);
+    loopBuilder.addRecursiveContinuation(loop);
+
+    // Translate the body.
+    JumpCollector breakCollector =
+        new ForwardJumpCollector(environment, target: target);
+    JumpCollector continueCollector =
+        new ForwardJumpCollector(loopBuilder.environment, target: target);
+    IrBuilder bodyBuilder = loopBuilder.makeDelimitedBuilder();
+    bodyBuilder._enterScope(closureScope);
+    state.breakCollectors.add(breakCollector);
+    state.continueCollectors.add(continueCollector);
+    buildBody(bodyBuilder);
+    assert(state.breakCollectors.last == breakCollector);
+    assert(state.continueCollectors.last == continueCollector);
+    state.breakCollectors.removeLast();
+    state.continueCollectors.removeLast();
+    if (bodyBuilder.isOpen) bodyBuilder.jumpTo(continueCollector);
 
-    // Construct the continue continuation (i.e., the condition).
+    // Construct the body of the continue continuation (i.e., the condition).
     // <Continue> =
     // let prim cond = [[condition]] in
     //   let cont exit() = break(v, ...)
     //        and repeat() = loop(v, ...)
     //   in branch cond (repeat, exit)
+    IrBuilder continueBuilder = loopBuilder.makeDelimitedBuilder();
+    continueBuilder.environment = continueCollector.environment;
     ir.Primitive condition = buildCondition(continueBuilder);
-    // Use a delimited IrBuilder for the exit continuation's body so that
-    // we can capture the break with the body's break collector.
+
     ir.Continuation exitContinuation = new ir.Continuation([]);
     IrBuilder exitBuilder = continueBuilder.makeDelimitedBuilder();
+    exitBuilder.jumpTo(breakCollector);
+    exitContinuation.body = exitBuilder._root;
     ir.Continuation repeatContinuation = new ir.Continuation([]);
-    ir.InvokeContinuation invokeLoop =
-        new ir.InvokeContinuation.uninitialized(recursive: true);
-    invokeLoop.arguments =
-        continueBuilder.environment.index2value.map(
-            (ir.Primitive value) => new ir.Reference(value)).toList();
-    repeatContinuation.body = invokeLoop;
+    IrBuilder repeatBuilder = continueBuilder.makeDelimitedBuilder();
+    repeatBuilder.jumpTo(loop);
+    repeatContinuation.body = repeatBuilder._root;
+
     continueBuilder.add(
         new ir.LetCont.many(<ir.Continuation>[exitContinuation,
                                               repeatContinuation],
             new ir.Branch(new ir.IsTrue(condition),
                           repeatContinuation,
                           exitContinuation)));
-    ir.Continuation continueContinuation =
-        new ir.Continuation(continueBuilder._parameters);
-    continueContinuation.body = continueBuilder._root;
+    continueCollector.continuation.body = continueBuilder._root;
 
     // Construct the loop continuation (i.e., the body and condition).
     // <Loop> =
     // let cont continue(x, ...) =
     //   <Continue>
     // in [[body]]; continue(v, ...)
-    JumpCollector breakCollector = new JumpCollector(target);
-    JumpCollector continueCollector = new JumpCollector(target);
-    state.breakCollectors.add(breakCollector);
-    state.continueCollectors.add(continueCollector);
-    bodyBuilder._enterScope(closureScope);
-    buildBody(bodyBuilder);
-    assert(state.breakCollectors.last == breakCollector);
-    assert(state.continueCollectors.last == continueCollector);
-    state.breakCollectors.removeLast();
-    state.continueCollectors.removeLast();
-    // Add the jump from the loop's exit to the break condition.  It is only
-    // here where the exitBuilder's root is non-null and we can set the
-    // exitContinuation's body.
-    breakCollector.addJump(exitBuilder);
-    exitContinuation.body = exitBuilder._root;
-    if (bodyBuilder.isOpen) {
-      continueCollector.addJump(bodyBuilder);
-    }
-    invokeFullJoin(continueContinuation, continueCollector, recursive: false);
-    ir.Continuation loopContinuation =
-        new ir.Continuation(bodyBuilder._parameters);
-    loopContinuation.isRecursive = true;
-    loopContinuation.body =
-        new ir.LetCont(continueContinuation, bodyBuilder._root);
-    invokeLoop.continuation =
-        new ir.Reference<ir.Continuation>(loopContinuation);
-    ir.LetCont letLoop =
-        new ir.LetCont(loopContinuation,
-            new ir.InvokeContinuation(loopContinuation,
-                                      environment.index2value));
-
-    // Add the break condition.
-    ir.Continuation breakContinuation;
-    if (breakCollector.length == 1) {
-      // createJoin only works when there is more than one jump to a join-point
-      // continuation.  This is to potentially catch errors in the case that
-      // a join was intended and at least one jump is missing.  Unfortunately
-      // we have the explicit code below for the (common?) case that the
-      // only break from the do-while is the implicit one when the condition
-      // is false.
-      List<ir.Parameter> parameters = <ir.Parameter>[];
-      List<ir.Reference> arguments = <ir.Reference>[];
-      for (int i = 0; i < environment.length; ++i) {
-        ir.Parameter parameter =
-            new ir.Parameter(environment.index2variable[i]);
-        parameters.add(parameter);
-        environment.index2value[i] = parameter;
-        arguments.add(new ir.Reference(breakCollector.environments.first[i]));
-      }
-      breakContinuation = new ir.Continuation(parameters);
-      breakCollector.invocations.first.arguments = arguments;
-      breakCollector.invocations.first.continuation =
-          new ir.Reference(breakContinuation);
-    } else {
-      breakContinuation = createJoin(environment.length, breakCollector);
-    }
-    add(new ir.LetCont(breakContinuation, letLoop));
+    loopBuilder.add(
+        new ir.LetCont(continueCollector.continuation,
+            bodyBuilder._root));
+
+    // And tie it all together.
+    add(new ir.LetCont(breakCollector.continuation, loopBuilder._root));
+    environment = breakCollector.environment;
   }
 
   /// Creates a try-statement.
@@ -1499,14 +1613,15 @@ 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 that are boxed due to being captured in a closure are boxed
     // for their entire lifetime, and so they do not need to be boxed on
-    // entry to any try block.  We check for them here because we can not
-    // identify all of them in the same pass where we identify the variables
-    // assigned in the try (the may be captured by a closure after the try
-    // statement).
+    // entry to any try block.  They are not filtered out before this because
+    // we can not identify all of them in the same pass where we identify the
+    // variables assigned in the try (they may be captured by a closure after
+    // the try statement).
     Iterable<LocalVariableElement> boxedOnEntry =
         tryStatementInfo.boxedOnEntry.where((LocalVariableElement variable) {
       return !tryCatchBuilder.mutableCapturedVariables.contains(variable);
@@ -1518,34 +1633,26 @@ abstract class IrBuilder {
       tryCatchBuilder.declareLocalVariable(variable, initialValue: value);
     }
 
-    IrBuilder catchBuilder = tryCatchBuilder.makeDelimitedBuilder();
     IrBuilder tryBuilder = tryCatchBuilder.makeDelimitedBuilder();
-    List<ir.Parameter> joinParameters =
-        new List<ir.Parameter>.generate(environment.length, (i) {
-      return new ir.Parameter(environment.index2variable[i]);
-    });
-    ir.Continuation joinContinuation = new ir.Continuation(joinParameters);
 
     void interceptJumps(JumpCollector collector) {
       collector.enterTry(boxedOnEntry);
     }
-    void restoreJumps(JumpCollector collector) {
-      collector.leaveTry();
-    }
     tryBuilder.state.breakCollectors.forEach(interceptJumps);
     tryBuilder.state.continueCollectors.forEach(interceptJumps);
     buildTryBlock(tryBuilder);
+    void restoreJumps(JumpCollector collector) {
+      collector.leaveTry();
+    }
     tryBuilder.state.breakCollectors.forEach(restoreJumps);
     tryBuilder.state.continueCollectors.forEach(restoreJumps);
     if (tryBuilder.isOpen) {
-      for (LocalVariableElement variable in boxedOnEntry) {
-        assert(tryBuilder.isInMutableVariable(variable));
-        ir.Primitive value = tryBuilder.buildLocalGet(variable);
-        tryBuilder.environment.update(variable, value);
-      }
-      tryBuilder.jumpTo(joinContinuation);
+      interceptJumps(join);
+      tryBuilder.jumpTo(join);
+      restoreJumps(join);
     }

[asgerf, 2015/04/07 13:42:42]

I may be nitpicking now, but could we please restore the outer collectors AFTER
this if? This seems to violate a stack discipline.

This also has the side benfit of moving the jumpTo() closer to the
buildTryBlock().

There is a recurring pattern of "build and then jump". This pattern is fairly
easy to understand, so it helps a lot if we can recognize easily.

In this instance it is surround by some stack-based state manipulation, which is
also fairly digestable, but not as much when you can't see at a glance that it's
a stack.

[Kevin Millikin (Google), 2015/04/08 10:56:59]

That's fine.  I can even restore the break and continue collectors in the
opposite order to make it really clear :)

However, in all the other sites, I have not done exactly what you describe: I
have pushed/popped the break and continue collector stacks immediately around
visiting the statement in question, and only after that added a jump to the end
of the body if necessary.  I consciously did it that way to emphasize that the
'scope' of the modified break and continue stacks was exactly the translation of
the body.  I was worried that the translation of some unrelated term would creep
in at the wrong place.

[asgerf, 2015/04/08 11:10:21]

Just don't do breakCollectors.reversed.forEach(restoreJumps). That would be
overkill.

 
+    IrBuilder catchBuilder = tryCatchBuilder.makeDelimitedBuilder();
     for (LocalVariableElement variable in boxedOnEntry) {
       assert(catchBuilder.isInMutableVariable(variable));
       ir.Primitive value = catchBuilder.buildLocalGet(variable);
@@ -1577,9 +1684,7 @@ abstract class IrBuilder {
         traceParameter = new ir.Parameter(null);
       }
       catchClauseInfo.buildCatchBlock(catchBuilder);
-      if (catchBuilder.isOpen) {
-        catchBuilder.jumpTo(joinContinuation);
-      }
+      if (catchBuilder.isOpen) catchBuilder.jumpTo(join);
       List<ir.Parameter> catchParameters =
           <ir.Parameter>[exceptionParameter, traceParameter];
       ir.Continuation catchContinuation = new ir.Continuation(catchParameters);
@@ -1590,10 +1695,8 @@ abstract class IrBuilder {
       tryCatchBuilder._current = null;
     }
 
-    add(new ir.LetCont(joinContinuation, tryCatchBuilder._root));
-    for (int i = 0; i < environment.length; ++i) {
-      environment.index2value[i] = joinParameters[i];
-    }
+    add(new ir.LetCont(join.continuation, tryCatchBuilder._root));
+    environment = join.environment;
   }
 
   /// Create a return statement `return value;` or `return;` if [value] is
@@ -1634,48 +1737,26 @@ abstract class IrBuilder {
     }
   }
 
-
   /// Creates a labeled statement
   void buildLabeledStatement({SubbuildFunction buildBody,
                               JumpTarget target}) {
-    JumpCollector jumps = new JumpCollector(target);
-    state.breakCollectors.add(jumps);
+    JumpCollector join = new ForwardJumpCollector(environment, target: target);
     IrBuilder innerBuilder = makeDelimitedBuilder();
+    innerBuilder.state.breakCollectors.add(join);
     buildBody(innerBuilder);
-    state.breakCollectors.removeLast();
-    bool hasBreaks = !jumps.isEmpty;
-    ir.Continuation joinContinuation;
+    innerBuilder.state.breakCollectors.removeLast();
+    bool hasBreaks = !join.isEmpty;
     if (hasBreaks) {
-      if (innerBuilder.isOpen) {
-        jumps.addJump(innerBuilder);
-      }
-
-      // All jumps to the break continuation must be in the scope of the
-      // continuation's binding.  The continuation is bound just outside the
-      // body to satisfy this property without extra analysis.
-      // As a consequence, the break continuation needs parameters for all
-      // local variables in scope at the exit from the body.
-      List<ir.Parameter> parameters =
-          new List<ir.Parameter>.generate(environment.length, (i) {
-        return new ir.Parameter(environment.index2variable[i]);
-      });
-      joinContinuation = new ir.Continuation(parameters);
-      invokeFullJoin(joinContinuation, jumps, recursive: false);
-      add(new ir.LetCont(joinContinuation, innerBuilder._root));
-      for (int i = 0; i < environment.length; ++i) {
-        environment.index2value[i] = parameters[i];
-      }
-    } else {
-      if (innerBuilder._root != null) {
-        add(innerBuilder._root);
-        _current = innerBuilder._current;
-        environment = innerBuilder.environment;
-      }
+      if (innerBuilder.isOpen) innerBuilder.jumpTo(join);
+      add(new ir.LetCont(join.continuation, innerBuilder._root));
+      environment = join.environment;
+    } else if (innerBuilder._root != null) {
+      add(innerBuilder._root);
+      _current = innerBuilder._current;
+      environment = innerBuilder.environment;
     }

[asgerf, 2015/04/07 13:42:41]

Could we have an else with a comment saying that the body was empty and so
nothing needs to be done? Right now it sort of looks like a forgotton case.

[Kevin Millikin (Google), 2015/04/08 10:56:59]

Done.

-    return null;
   }
 
-
   // Build(BreakStatement L, C) = C[InvokeContinuation(...)]
   //
   // The continuation and arguments are filled in later after translating
@@ -1697,7 +1778,7 @@ abstract class IrBuilder {
     assert(isOpen);
     for (JumpCollector collector in collectors) {
       if (target == collector.target) {
-        collector.addJump(this);
+        jumpTo(collector);
         return true;
       }
     }
@@ -1765,7 +1846,6 @@ abstract class IrBuilder {
     // e0 || e1 is translated as if e0 ? true : (e1 == true).
     // The translation must convert both e0 and e1 to booleans and handle
     // local variable assignments in e1.
-
     IrBuilder rightBuilder = makeDelimitedBuilder();
     ir.Primitive rightValue = buildRightValue(rightBuilder);
     // A dummy empty target for the branch on the left subexpression branch.
@@ -1792,18 +1872,19 @@ abstract class IrBuilder {
     assert(environment.length == emptyBuilder.environment.length);
     assert(environment.length == rightTrueBuilder.environment.length);
     assert(environment.length == rightFalseBuilder.environment.length);
+    // Treat the value of the expression as a local variable so it will get
+    // a continuation parameter.
+    environment.extend(null, null);

[asgerf, 2015/04/07 13:42:42]

I love this!

     emptyBuilder.environment.extend(null, leftBool);
     rightTrueBuilder.environment.extend(null, rightTrue);
     rightFalseBuilder.environment.extend(null, rightFalse);
 
     // Wire up two continuations for the left subexpression, two continuations
     // for the right subexpression, and a three-way join continuation.
-    JumpCollector jumps = new JumpCollector(null);
-    jumps.addJump(emptyBuilder);
-    jumps.addJump(rightTrueBuilder);
-    jumps.addJump(rightFalseBuilder);
-    ir.Continuation joinContinuation =
-        createJoin(environment.length + 1, jumps);
+    JumpCollector join = new ForwardJumpCollector(environment);
+    emptyBuilder.jumpTo(join);
+    rightTrueBuilder.jumpTo(join);
+    rightFalseBuilder.jumpTo(join);
     ir.Continuation leftTrueContinuation = new ir.Continuation([]);
     ir.Continuation leftFalseContinuation = new ir.Continuation([]);
     ir.Continuation rightTrueContinuation = new ir.Continuation([]);
@@ -1828,120 +1909,17 @@ abstract class IrBuilder {
       leftFalseContinuation.body = emptyBuilder._root;
     }
 
-    add(new ir.LetCont(joinContinuation,
+    add(new ir.LetCont(join.continuation,
             new ir.LetCont.many(<ir.Continuation>[leftTrueContinuation,
                                                   leftFalseContinuation],
                 new ir.Branch(new ir.IsTrue(leftValue),
                               leftTrueContinuation,
                               leftFalseContinuation))));
+    environment = join.environment;
+    environment.discard(1);
     // There is always a join parameter for the result value, because it
     // is different on at least two paths.
-    return joinContinuation.parameters.last;
-  }
-
-  /// Create a non-recursive join-point continuation.
-  ///
-  /// Given the environment length at the join point and a list of
-  /// jumps that should reach the join point, create a join-point
-  /// continuation.  The join-point continuation has a parameter for each
-  /// variable that has different values reaching on different paths.
-  ///
-  /// The jumps are uninitialized [ir.InvokeContinuation] expressions.
-  /// They are filled in with the target continuation and appropriate
-  /// arguments.
-  ///
-  /// As a side effect, the environment of this builder is updated to include
-  /// the join-point continuation parameters.
-  ir.Continuation createJoin(int environmentLength, JumpCollector jumps) {
-    assert(jumps.length >= 2);
-
-    // Compute which values are identical on all paths reaching the join.
-    // Handle the common case of a pair of contexts efficiently.
-    Environment first = jumps.environments[0];
-    Environment second = jumps.environments[1];
-    assert(environmentLength <= first.length);
-    assert(environmentLength <= second.length);
-    assert(first.sameDomain(environmentLength, second));
-    // A running count of the join-point parameters.
-    int parameterCount = 0;
-    // The null elements of common correspond to required parameters of the
-    // join-point continuation.
-    List<ir.Primitive> common =
-        new List<ir.Primitive>.generate(environmentLength,
-            (i) {
-              ir.Primitive candidate = first[i];
-              if (second[i] == candidate) {
-                return candidate;
-              } else {
-                ++parameterCount;
-                return null;
-              }
-            });
-    // If there is already a parameter for each variable, the other
-    // environments do not need to be considered.
-    if (parameterCount < environmentLength) {
-      for (int i = 0; i < environmentLength; ++i) {
-        ir.Primitive candidate = common[i];
-        if (candidate == null) continue;
-        for (Environment current in jumps.environments.skip(2)) {
-          assert(environmentLength <= current.length);
-          assert(first.sameDomain(environmentLength, current));
-          if (candidate != current[i]) {
-            common[i] = null;
-            ++parameterCount;
-            break;
-          }
-        }
-        if (parameterCount >= environmentLength) break;
-      }
-    }
-
-    // Create the join point continuation.
-    List<ir.Parameter> parameters = <ir.Parameter>[];
-    parameters.length = parameterCount;
-    int index = 0;
-    for (int i = 0; i < environmentLength; ++i) {
-      if (common[i] == null) {
-        parameters[index++] = new ir.Parameter(first.index2variable[i]);
-      }
-    }
-    assert(index == parameterCount);
-    ir.Continuation join = new ir.Continuation(parameters);
-
-    // Fill in all the continuation invocations.
-    for (int i = 0; i < jumps.length; ++i) {
-      Environment currentEnvironment = jumps.environments[i];
-      ir.InvokeContinuation invoke = jumps.invocations[i];
-      // Sharing this.environment with one of the invocations will not do
-      // the right thing (this.environment has already been mutated).
-      List<ir.Reference> arguments = <ir.Reference>[];
-      arguments.length = parameterCount;
-      int index = 0;
-      for (int i = 0; i < environmentLength; ++i) {
-        if (common[i] == null) {
-          arguments[index++] = new ir.Reference(currentEnvironment[i]);
-        }
-      }
-      invoke.continuation = new ir.Reference(join);
-      invoke.arguments = arguments;
-    }
-
-    // Mutate this.environment to be the environment at the join point.  Do
-    // this after adding the continuation invocations, because this.environment
-    // might be collected by the jump collector and so the old environment
-    // values are needed for the continuation invocation.
-    //
-    // Iterate to environment.length because environmentLength includes values
-    // outside the environment which are 'phantom' variables used for the
-    // values of expressions like &&, ||, and ?:.
-    index = 0;
-    for (int i = 0; i < environment.length; ++i) {
-      if (common[i] == null) {
-        environment.index2value[i] = parameters[index++];
-      }
-    }
-

[asgerf, 2015/04/07 13:42:42]

I think red is my new favorite color.

-    return join;
+    return join.continuation.parameters.last;
   }
 }