Make the mutable variables local to an IR builder instead of global.

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 decab2ca7dc25ea63cd4b2380e39fbb9d82d27be..51c737d1b1b245171d3e1448e5a1cc1d6dfed077 100644
--- a/pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart
+++ b/pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart
@@ -376,7 +376,7 @@ abstract class IrBuilderMixin<N> {
 }
 
 /// Shared state between delimited IrBuilders within the same function.
-class IrBuilderDelimitedState {
+class IrBuilderSharedState {
   final ConstantSystem constantSystem;
 
   /// A stack of collectors for breaks.
@@ -395,7 +395,7 @@ class IrBuilderDelimitedState {
 
   final List<ir.Definition> functionParameters = <ir.Definition>[];
 
-  IrBuilderDelimitedState(this.constantSystem, this.currentElement);
+  IrBuilderSharedState(this.constantSystem, this.currentElement);
 
   ir.Parameter get thisParameter => _thisParameter;
   void set thisParameter(ir.Parameter value) {
@@ -418,20 +418,6 @@ class ThisParameterLocal implements Local {
 abstract class IrBuilder {
   IrBuilder _makeInstance();
 
-  /// True if [local] should currently be accessed from a [ir.MutableVariable].
-  bool isInMutableVariable(Local local);
-
-  /// Creates a [ir.MutableVariable] for the given local.
-  void makeMutableVariable(Local local);
-
-  /// Remove an [ir.MutableVariable] for a local.
-  ///
-  /// Subsequent access to the local will be direct rather than through the
-  /// mutable variable.  This is used for variables that do not spend their
-  /// entire lifetime as mutable variables (e.g., variables that are boxed
-  /// in mutable variables for a try block).
-  void removeMutableVariable(Local local);
-
   void declareLocalVariable(LocalVariableElement element,
                             {ir.Primitive initialValue});
 
@@ -482,7 +468,7 @@ abstract class IrBuilder {
 
   final List<ir.Parameter> _parameters = <ir.Parameter>[];
 
-  IrBuilderDelimitedState state;
+  IrBuilderSharedState state;
 
   /// A map from variable indexes to their values.
   ///
@@ -490,6 +476,36 @@ abstract class IrBuilder {
   /// in the map; look up their [BoxLocal] instead.
   Environment environment;
 
+  /// A map from mutable local variables to their [ir.MutableVariable]s.
+  ///
+  /// Mutable variables are treated as boxed.  Writes to them are observable
+  /// side effects.
+  Map<Local, ir.MutableVariable> mutableVariables;
+
+  /// True if [local] should currently be accessed from a [ir.MutableVariable].
+  bool isInMutableVariable(Local local) {
+    return mutableVariables.containsKey(local);
+  }
+
+  /// Creates a [ir.MutableVariable] for the given local.
+  void makeMutableVariable(Local local) {
+    mutableVariables[local] =
+        new ir.MutableVariable(local.executableContext, local);
+  }
+
+  /// Remove an [ir.MutableVariable] for a local.
+  ///
+  /// Subsequent access to the local will be direct rather than through the
+  /// mutable variable.
+  void removeMutableVariable(Local local) {
+    mutableVariables.remove(local);
+  }
+
+  /// Gets the [MutableVariable] containing the value of [local].
+  ir.MutableVariable getMutableVariable(Local local) {
+    return mutableVariables[local];
+  }
+
   // The IR builder maintains a context, which is an expression with a hole in
   // it.  The hole represents the focus where new expressions can be added.
   // The context is implemented by 'root' which is the root of the expression
@@ -519,8 +535,9 @@ abstract class IrBuilder {
 
   /// Initialize a new top-level IR builder.
   void _init(ConstantSystem constantSystem, ExecutableElement currentElement) {
-    state = new IrBuilderDelimitedState(constantSystem, currentElement);
+    state = new IrBuilderSharedState(constantSystem, currentElement);
     environment = new Environment.empty();
+    mutableVariables = <Local, ir.MutableVariable>{};
   }
 
   /// Construct a delimited visitor for visiting a subtree.
@@ -535,25 +552,29 @@ abstract class IrBuilder {
   IrBuilder makeDelimitedBuilder([Environment env = null]) {
     return _makeInstance()
         ..state = state
-        ..environment = env != null ? env : new Environment.from(environment);
+        ..environment = env != null ? env : new Environment.from(environment)
+        ..mutableVariables = mutableVariables;
   }
 
   /// Construct a builder for making constructor field initializers.
   IrBuilder makeInitializerBuilder() {
     return _makeInstance()
-        ..state = new IrBuilderDelimitedState(state.constantSystem,
-                                              state.currentElement)
-        ..environment = new Environment.from(environment);
+        ..state = new IrBuilderSharedState(state.constantSystem,
+                                           state.currentElement)
+        ..environment = new Environment.from(environment)
+        ..mutableVariables = mutableVariables;
   }
 
   /// Construct a builder for an inner function.
   IrBuilder makeInnerFunctionBuilder(ExecutableElement currentElement) {
-    IrBuilderDelimitedState innerState =
-        new IrBuilderDelimitedState(state.constantSystem, currentElement)
+    IrBuilderSharedState innerState =
+        new IrBuilderSharedState(state.constantSystem, currentElement)
           ..enclosingMethodThisParameter = state.enclosingMethodThisParameter;
     return _makeInstance()
         ..state = innerState
-        ..environment = new Environment.empty();
+        ..environment = new Environment.empty()
+        ..mutableVariables =
+            new Map<Local, ir.MutableVariable>.from(mutableVariables);

[Kevin Millikin (Google), 2015/04/20 10:32:20]

Inner function builders need to inherit the mutable variables of the outer
builder.  (In JS there are no references to any of them --- if there were a
reference from an inner function then it would be boxed in the heap and not a
mutable variable.)

   }
 
   bool get isOpen => _root == null || _current != null;
@@ -1715,12 +1736,10 @@ abstract class IrBuilder {
     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.  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).
+    // Variables treated as mutable in a try are not mutable outside of it.
+    // Work with a copy of the outer builder's mutable variables.
+    tryCatchBuilder.mutableVariables =
+        new Map<Local, ir.MutableVariable>.from(mutableVariables);
     for (LocalVariableElement variable in tryStatementInfo.boxedOnEntry) {
       assert(!tryCatchBuilder.isInMutableVariable(variable));
       ir.Primitive value = tryCatchBuilder.buildLocalVariableGet(variable);
@@ -1751,10 +1770,8 @@ abstract class IrBuilder {
     for (LocalVariableElement variable in tryStatementInfo.boxedOnEntry) {
       assert(catchBuilder.isInMutableVariable(variable));
       ir.Primitive value = catchBuilder.buildLocalVariableGet(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.
+      // After this point, the variables that were boxed on entry to the try
+      // are no longer treated as mutable.
       catchBuilder.removeMutableVariable(variable);
       catchBuilder.environment.update(variable, value);
     }
@@ -2041,23 +2058,8 @@ abstract class IrBuilder {
 
 /// 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>{};
-
-  /// Creates a [MutableVariable] for the given local.
-  void makeMutableVariable(Local local) {
-    ir.MutableVariable variable =
-        new ir.MutableVariable(local.executableContext, local);
-    local2mutable[local] = variable;
-  }
-
   /// [MutableVariable]s that should temporarily be treated as registers.
   final Set<Local> registerizedMutableVariables = new Set<Local>();
-
-  DartIrBuilderSharedState(Set<Local> capturedVariables) {
-    capturedVariables.forEach(makeMutableVariable);
-  }
 }
 
 /// Dart-specific subclass of [IrBuilder].
@@ -2076,28 +2078,17 @@ class DartIrBuilder extends IrBuilder {
   DartIrBuilder(ConstantSystem constantSystem,
                 ExecutableElement currentElement,
                 Set<Local> capturedVariables)
-      : dartState = new DartIrBuilderSharedState(capturedVariables) {
+      : dartState = new DartIrBuilderSharedState() {

[Kevin Millikin (Google), 2015/04/20 10:32:20]

There should be only one of these, not one per builder.  As you noted, it
contains only the registerized variables that we don't really need any more.

     _init(constantSystem, currentElement);
+    capturedVariables.forEach(makeMutableVariable);
   }
 
+  @override
   bool isInMutableVariable(Local local) {
-    return dartState.local2mutable.containsKey(local) &&
+    return mutableVariables.containsKey(local) &&
            !dartState.registerizedMutableVariables.contains(local);
   }
 
-  void makeMutableVariable(Local local) {
-    dartState.makeMutableVariable(local);
-  }
-
-  void removeMutableVariable(Local local) {
-    dartState.local2mutable.remove(local);
-  }
-
-  /// Gets the [MutableVariable] containing the value of [local].
-  ir.MutableVariable getMutableVariable(Local local) {
-    return dartState.local2mutable[local];
-  }
-
   void _enterScope(ClosureScope scope) {
     assert(scope == null);
   }
@@ -2110,7 +2101,7 @@ class DartIrBuilder extends IrBuilder {
                                 List<LocalElement> loopVariables) {
     assert(scope == null);
     for (LocalElement loopVariable in loopVariables) {
-      if (dartState.local2mutable.containsKey(loopVariable)) {
+      if (mutableVariables.containsKey(loopVariable)) {
         // Temporarily keep the loop variable in a primitive.
         // The loop variable will be added to environment when
         // [declareLocalVariable] is called.
@@ -2123,7 +2114,7 @@ class DartIrBuilder extends IrBuilder {
                          List<LocalElement> loopVariables) {
     assert(scope == null);
     for (LocalElement loopVariable in loopVariables) {
-      if (dartState.local2mutable.containsKey(loopVariable)) {
+      if (mutableVariables.containsKey(loopVariable)) {
         // Move from [Primitive] into [MutableVariable].
         dartState.registerizedMutableVariables.remove(loopVariable);
         add(new ir.LetMutable(getMutableVariable(loopVariable),
@@ -2292,12 +2283,6 @@ class JsIrBuilder extends IrBuilder {
     _init(constantSystem, currentElement);
   }
 
-  Map<ast.TryStatement, TryStatementInfo> get tryStatements => null;
-  Set<Local> get mutableCapturedVariables => null;
-  bool isInMutableVariable(Local local) => false;
-  void makeMutableVariable(Local local) {}
-  void removeMutableVariable(Local local) {}
-
   void enterInitializers() {
     assert(jsState.inInitializers == false);
     jsState.inInitializers = true;
@@ -2391,6 +2376,9 @@ class JsIrBuilder extends IrBuilder {
       add(new ir.SetField(environment.lookup(location.box),
                           location.field,
                           initialValue));
+    } else if (isInMutableVariable(variableElement)) {
+      add(new ir.LetMutable(getMutableVariable(variableElement),
+                            initialValue));
     } else {
       initialValue.useElementAsHint(variableElement);
       environment.extend(variableElement, initialValue);