Change the collection of continuation jumps.

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

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library dart2js.ir_builder;
 
 import '../constants/expressions.dart';
 import '../constants/values.dart' show PrimitiveConstantValue;
 import '../dart_types.dart';
 import '../dart2jslib.dart';
@@ skipping 26 matching lines @@
         index2value = <ir.Primitive>[];
 
   /// Construct an environment that is a copy of another one.
   ///
   /// The mapping from elements to indexes is shared, not copied.
   Environment.from(Environment other)
       : variable2index = other.variable2index,
         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];
 
   void extend(Local element, ir.Primitive value) {
     // Assert that the name is not already in the environment.  `null` is used
     // as the name of anonymous variables.  Because the variable2index map is
     // shared, `null` can already occur.  This is safe because such variables
     // are not looked up by name.
     //
     // TODO(kmillikin): This is still kind of fishy.  Refactor to not share
     // name maps or else garbage collect unneeded names.
     assert(element == null || !variable2index.containsKey(element));
     variable2index[element] = index2variable.length;
     index2variable.add(element);
     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."));
     return index2value[variable2index[element]];
   }
 
   void update(Local element, ir.Primitive value) {
     index2value[variable2index[element]] = value;
   }
 
@@ skipping 10 matching lines @@
       // The variable maps to the same index in both environments.
       int index = variable2index[variable];
       if (index == null || index != other.variable2index[variable]) {
         return false;
       }
     }
     return true;
   }
 }
 
-/// 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);
     _environments.add(builder.environment);
     builder._current = null;
     // TODO(kmillikin): Can we set builder.environment to null to make it
     // 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.
 
-  /// 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();
+    // 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;
+    }
   }
 }
 
+/// 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.
+  ///
+  /// 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;
+  }
+}
+
 /// Function for building a node in the context of the current builder.
 typedef ir.Node BuildFunction(node);
 
 /// Function for building nodes in the context of the provided [builder].
 typedef ir.Node SubbuildFunction(IrBuilder builder);
 
 /// Mixin that provides encapsulated access to nested builders.
 abstract class IrBuilderMixin<N> {
   IrBuilder _irBuilder;
 
@@ skipping 191 matching lines @@
   ir.Expression _current = null;
 
   /// Initialize a new top-level IR builder.
   void _init(ConstantSystem constantSystem, ExecutableElement currentElement) {
     state = new IrBuilderDelimitedState(constantSystem, currentElement);
     environment = new Environment.empty();
   }
 
   /// 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.
   IrBuilder makeInitializerBuilder() {
     return _makeInstance()
         ..state = new IrBuilderDelimitedState(state.constantSystem,
                                               state.currentElement)
         ..environment = new Environment.from(environment);
   }
 
@@ skipping 186 matching lines @@
     return addPrimitive(new ir.LiteralMap(type, entries));
   }
 
   /// Creates a conditional expression with the provided [condition] where the
   /// then and else expression are created through the [buildThenExpression] and
   /// [buildElseExpression] functions, respectively.
   ir.Primitive buildConditional(
       ir.Primitive condition,
       ir.Primitive buildThenExpression(IrBuilder builder),
       ir.Primitive buildElseExpression(IrBuilder builder)) {
-
     assert(isOpen);
 
     // The then and else expressions are delimited.
     IrBuilder thenBuilder = makeDelimitedBuilder();
     IrBuilder elseBuilder = makeDelimitedBuilder();
     ir.Primitive thenValue = buildThenExpression(thenBuilder);
     ir.Primitive elseValue = buildElseExpression(elseBuilder);
 
     // 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
     //   let cont then() = [[thenPart]]; join(v, ...)
     //        and else() = [[elsePart]]; join(v, ...)
     //   in
     //     if condition (then, else)
     ir.Continuation thenContinuation = new ir.Continuation([]);
     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;
   }
 
   /**
    * Add an explicit `return null` for functions that don't have a return
    * statement on each branch. This includes functions with an empty body,
    * such as `foo(){ }`.
    */
   void _ensureReturn() {
     if (!isOpen) return;
     ir.Constant constant = buildNullLiteral();
@@ skipping 246 matching lines @@
     List<ir.Continuation> arms = !thenBuilder.isOpen && elseBuilder.isOpen
         ? <ir.Continuation>[elseContinuation, thenContinuation]
         : <ir.Continuation>[thenContinuation, elseContinuation];
 
     ir.Expression result =
         new ir.LetCont.many(arms,
             new ir.Branch(new ir.IsTrue(condition),
                           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
     // a join then an InvokeContinuation was just added to both of them.  If
     // there is no join, then at least one of them is closed and thus has a
     // non-null root by the definition of the predicate isClosed.  In the
     // case that one of them is null, it must be the only one that is open
     // and thus contains the new hole in the context.  This case is handled
     // after the branch is plugged into the current hole.
     thenContinuation.body = thenBuilder._root;
     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;
         environment = thenBuilder.environment;
       } else if (elseBuilder.isOpen) {
         if (elseBuilder._root != null) _current = elseBuilder._current;
         environment = elseBuilder.environment;
       } 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
   /// created by [buildInitializer], [buildCondition], [buildBody] and
   /// [buildUpdate], respectively.
   ///
   /// The jump [target] is used to identify which `break` and `continue`
   /// statements that have this `for` statement as their target.
   ///
   /// The [closureScope] identifies variables that should be boxed in this loop.
@@ skipping 13 matching lines @@
 
     // For loops use four named continuations: the entry to the condition,
     // the entry to the body, the loop exit, and the loop successor (break).
     // The CPS translation of
     // [[for (initializer; condition; update) body; successor]] is:
     //
     // _enterForLoopInitializer();
     // [[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();
     //         let cont continue(x, ...) =
     //             _enterForLoopUpdate();
     //             [[update]];
     //             loop(v, ...) in
     //         [[body]];
     //         continue(v, ...) in
     //     branch cond (body, exit) in
     // loop(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).  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);
+    // 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);
-
-    IrBuilder outerBodyBuilder = condBuilder.makeDelimitedBuilder();
-    outerBodyBuilder._enterForLoopBody(closureScope, loopVariables);
-
-    IrBuilder innerBodyBuilder = outerBodyBuilder.makeDelimitedBuilder();
-
     buildBody(innerBodyBuilder);
     assert(state.breakCollectors.last == breakCollector);
     assert(state.continueCollectors.last == continueCollector);
     state.breakCollectors.removeLast();
     state.continueCollectors.removeLast();
 
     // 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.
+    // Create loop exit and body entry continuations and a branch to them.
+    ir.Continuation exitContinuation = new ir.Continuation([]);
     ir.Continuation bodyContinuation = new ir.Continuation([]);
-    ir.Continuation exitContinuation = 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 =
         new ir.LetCont.many(<ir.Continuation>[exitContinuation,
                                               bodyContinuation],
             new ir.Branch(new ir.IsTrue(condition),
                           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 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)));
-    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);
     }
   }
 
   /// Creates a for-in loop, `for (v in e) b`.
   ///
   /// [buildExpression] creates the expression, `e`. The variable, `v`, can
   /// take one of three forms:
   ///     1) `v` can be declared within the for-in statement, like in
   ///        `for (var v in e)`, in which case, [buildVariableDeclaration]
   ///        creates its declaration and [variableElement] is the element for
@@ skipping 18 matching lines @@
     // for (a in e) s;
     //
     // Is compiled analogously to:
     //
     // it = e.iterator;
     // while (it.moveNext()) {
     //   var a = it.current;
     //   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)));
+            moveNextInvoked,
+            emptyArguments)));
 
-    JumpCollector breakCollector = new JumpCollector(target);
-    JumpCollector continueCollector = new JumpCollector(target);
-    state.breakCollectors.add(breakCollector);
-    state.continueCollectors.add(continueCollector);
-
-    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,
         new ir.InvokeMethod(iterator, new Selector.getter("current", null),
             currentInvoked, emptyArguments)));
     // TODO(sra): Does this cover all cases? The general setter case include
     // super.
     if (Elements.isLocal(variableElement)) {
       bodyBuilder.buildLocalSet(variableElement, currentValue);
     } else if (Elements.isStaticOrTopLevel(variableElement) ||
                Elements.isErroneous(variableElement)) {
       bodyBuilder.buildStaticSet(variableElement, currentValue);
     } else {
       ir.Primitive receiver = bodyBuilder.buildThis();
       assert(receiver != null);
       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.
+    //
+    // let cont exit() = [ ]
+    //      and body() = <<BODY>>
+    // in branch condition (body, exit)
+    ir.Continuation exitContinuation = new ir.Continuation([]);
     ir.Continuation bodyContinuation = new ir.Continuation([]);
-    ir.Continuation exitContinuation = 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 =
         new ir.LetCont.many(<ir.Continuation>[exitContinuation,
                                               bodyContinuation],
             new ir.Branch(new ir.IsTrue(condition),
                           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);
     }
   }
 
   /// Creates a while loop in which the condition and body are created by
   /// [buildCondition] and [buildBody], respectively.
   ///
   /// The jump [target] is used to identify which `break` and `continue`
   /// statements that have this `while` statement as their target.
   void buildWhile({SubbuildFunction buildCondition,
                    SubbuildFunction buildBody,
                    JumpTarget target,
                    ClosureScope closureScope}) {
     assert(isOpen);
     // While loops use four named continuations: the entry to the body, the
     // loop exit, the loop back edge (continue), and the loop exit (break).
     // The CPS translation of [[while (condition) body; successor]] is:
     //
     // let cont continue(x, ...) =
     //     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);
+    JumpCollector breakCollector =
+        new ForwardJumpCollector(environment, target: target);
+
+    IrBuilder bodyBuilder = makeDelimitedBuilder();
+    bodyBuilder._enterScope(closureScope);
     state.breakCollectors.add(breakCollector);
-    state.continueCollectors.add(continueCollector);
-
-    IrBuilder bodyBuilder = condBuilder.makeDelimitedBuilder();
-    bodyBuilder._enterScope(closureScope);
+    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 exitContinuation = new ir.Continuation([]);
     ir.Continuation bodyContinuation = new ir.Continuation([]);
-    ir.Continuation exitContinuation = 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 =
         new ir.LetCont.many(<ir.Continuation>[exitContinuation,
                                               bodyContinuation],
             new ir.Branch(new ir.IsTrue(condition),
                           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);
     }
   }
 
 
   /// Creates a do-while loop.
   ///
   /// The body and condition are created by [buildBody] and [buildCondition].
   /// The jump target [target] is the target of `break` and `continue`
   /// statements in the body that have the loop as their target.
   /// [closureScope] contains all the variables declared in the loop (but not
   /// declared in some inner closure scope).
   void buildDoWhile({SubbuildFunction buildBody,
                      SubbuildFunction buildCondition,
                      JumpTarget target,
                      ClosureScope closureScope}) {
     assert(isOpen);
     // The CPS translation of [[do body; while (condition); successor]] is:
     //
     // let cont break(x, ...) = [[successor]] in
     // let cont rec loop(x, ...) =
     //   let cont continue(x, ...) =
     //     let prim cond = [[condition]] in
     //       let cont exit() = break(v, ...)
     //            and repeat() = loop(v, ...)
     //       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);
 
-    // Construct the continue continuation (i.e., the condition).
+    // 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 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));
+    loopBuilder.add(
+        new ir.LetCont(continueCollector.continuation,
+            bodyBuilder._root));
 
-    // 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));
+    // And tie it all together.
+    add(new ir.LetCont(breakCollector.continuation, loopBuilder._root));
+    environment = breakCollector.environment;
   }
 
   /// Creates a try-statement.
   ///
   /// [tryInfo] provides information on local variables declared and boxed
   /// within this try statement.
   /// [buildTryBlock] builds the try block.
   /// [catchClauseInfos] provides access to the catch type, exception variable,
   /// and stack trace variable, and a function for building the catch block.
   void buildTry(
@@ skipping 24 matching lines @@
     //
     // In other words, both the try and catch block are in the scope of the
     // join-point continuation, and they are both in the scope of a sequence
     // of mutable bindings for the variables assigned in the try.  The join-
     // point continuation is not in the scope of these mutable bindings.
     // The tryBlock is in the scope of a binding for the catch handler.  Each
     // instruction (specifically, each call) in the tryBlock is in the dynamic
     // 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();
 
-    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);
     });
     for (LocalVariableElement variable in boxedOnEntry) {
       assert(!tryCatchBuilder.isInMutableVariable(variable));
       ir.Primitive value = tryCatchBuilder.buildLocalGet(variable);
       tryCatchBuilder.makeMutableVariable(variable);
       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);
     }
+    tryBuilder.state.breakCollectors.forEach(interceptJumps);
+    tryBuilder.state.continueCollectors.forEach(interceptJumps);
+    buildTryBlock(tryBuilder);
     void restoreJumps(JumpCollector collector) {
       collector.leaveTry();
     }
-    tryBuilder.state.breakCollectors.forEach(interceptJumps);
-    tryBuilder.state.continueCollectors.forEach(interceptJumps);
-    buildTryBlock(tryBuilder);
     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);
       // Note that we remove the variable from the set of mutable variables
       // here (and not above for the try body).  This is because the set of
       // mutable variables is global for the whole function and not local to
       // a delimited builder.
       catchBuilder.removeMutableVariable(variable);
       catchBuilder.environment.update(variable, value);
     }
@@ skipping 11 matching lines @@
       if (stackTraceVariable != null) {
         traceParameter = new ir.Parameter(stackTraceVariable);
         catchBuilder.environment.extend(stackTraceVariable, traceParameter);
       } else {
         // Use a dummy continuation parameter for the stack trace parameter.
         // This will ensure that all handlers have two parameters and so they
         // can be treated uniformly.
         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);
       catchContinuation.body = catchBuilder._root;
 
       tryCatchBuilder.add(
           new ir.LetHandler(catchContinuation, tryBuilder._root));
       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
   /// null.
   void buildReturn([ir.Primitive value]) {
     // Build(Return(e), C) = C'[InvokeContinuation(return, x)]
     //   where (C', x) = Build(e, C)
     //
     // Return without a subexpression is translated as if it were return null.
     assert(isOpen);
@@ skipping 20 matching lines @@
   /// The first node in the sequence does not need to be reachable.
   // TODO(johnniwinther): Type [nodes] as `Iterable` when `NodeList` uses
   // `List` instead of `Link`.
   void buildSequence(var nodes, BuildFunction build) {
     for (var node in nodes) {
       if (!isOpen) return;
       build(node);
     }
   }
 
-
   /// 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
   // the body containing the break.
   bool buildBreak(JumpTarget target) {
     return buildJumpInternal(target, state.breakCollectors);
   }
 
   // Build(ContinueStatement L, C) = C[InvokeContinuation(...)]
   //
   // The continuation and arguments are filled in later after translating
   // the body containing the continue.
   bool buildContinue(JumpTarget target) {
     return buildJumpInternal(target, state.continueCollectors);
   }
 
   bool buildJumpInternal(JumpTarget target,
                          Iterable<JumpCollector> collectors) {
     assert(isOpen);
     for (JumpCollector collector in collectors) {
       if (target == collector.target) {
-        collector.addJump(this);
+        jumpTo(collector);
         return true;
       }
     }
     return false;
   }
 
   /// Create a negation of [condition].
   ir.Primitive buildNegation(ir.Primitive condition) {
     // ! e is translated as e ? false : true
 
@@ skipping 47 matching lines @@
   /// operand and [buildRightValue] is called to process the value of the right
   /// operand in the context of its own [IrBuilder].
   ir.Primitive buildLogicalOperator(
       ir.Primitive leftValue,
       ir.Primitive buildRightValue(IrBuilder builder),
       {bool isLazyOr: false}) {
     // e0 && e1 is translated as if e0 ? (e1 == true) : false.
     // 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.
     // This enables using the same infrastructure for join-point continuations
     // as in visitIf and visitConditional.  It will hold a definition of the
     // appropriate constant and an invocation of the join-point continuation.
     IrBuilder emptyBuilder = makeDelimitedBuilder();
     // Dummy empty targets for right true and right false.  They hold
     // definitions of the appropriate constant and an invocation of the
     // join-point continuation.
     IrBuilder rightTrueBuilder = rightBuilder.makeDelimitedBuilder();
     IrBuilder rightFalseBuilder = rightBuilder.makeDelimitedBuilder();
 
     // If we don't evaluate the right subexpression, the value of the whole
     // expression is this constant.
     ir.Constant leftBool = emptyBuilder.buildBooleanLiteral(isLazyOr);
     // If we do evaluate the right subexpression, the value of the expression
     // is a true or false constant.
     ir.Constant rightTrue = rightTrueBuilder.buildBooleanLiteral(true);
     ir.Constant rightFalse = rightFalseBuilder.buildBooleanLiteral(false);
 
     // Treat the result values as named values in the environment, so they
     // will be treated as arguments to the join-point continuation.
     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([]);
     ir.Continuation rightFalseContinuation = new ir.Continuation([]);
     rightTrueContinuation.body = rightTrueBuilder._root;
     rightFalseContinuation.body = rightFalseBuilder._root;
     // The right subexpression has two continuations.
     rightBuilder.add(
         new ir.LetCont.many(<ir.Continuation>[rightTrueContinuation,
                                               rightFalseContinuation],
             new ir.Branch(new ir.IsTrue(rightValue),
                           rightTrueContinuation,
                           rightFalseContinuation)));
     // Depending on the operator, the left subexpression's continuations are
     // either the right subexpression or an invocation of the join-point
     // continuation.
     if (isLazyOr) {
       leftTrueContinuation.body = emptyBuilder._root;
       leftFalseContinuation.body = rightBuilder._root;
     } else {
       leftTrueContinuation.body = rightBuilder._root;
       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;
   }
 }
 
 /// Shared state between DartIrBuilders within the same method.
 class DartIrBuilderSharedState {
   /// Maps local variables to their corresponding [MutableVariable] object.
   final Map<Local, ir.MutableVariable> local2mutable =
       <Local, ir.MutableVariable>{};
 
   // Move this to the IrBuilderVisitor.
@@ skipping 630 matching lines @@
 }
 
 /// Synthetic parameter to a JavaScript factory method that takes the type
 /// argument given for the type variable [variable].
 class TypeInformationParameter implements Local {
   final TypeVariableElement variable;
   final ExecutableElement executableContext;
   TypeInformationParameter(this.variable, this.executableContext);
   String get name => variable.name;
 }