Remove LocalsHandler._capturedAndBoxed

Change-Id: I1f91a59d1e64dedad61dec77dd74aa6cd8e360dd
Reviewed-on: https://dart-review.googlesource.com/c/84524
Reviewed-by: Stephen Adams <sra@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
diff --git a/pkg/compiler/lib/src/closure.dart b/pkg/compiler/lib/src/closure.dart
index 22b3fc7..41e51e6 100644
--- a/pkg/compiler/lib/src/closure.dart
+++ b/pkg/compiler/lib/src/closure.dart
@@ -113,7 +113,7 @@
   void forEachBoxedVariable(f(Local local, FieldEntity field)) {}
 
   /// True if [variable] has been mutated and is also used in another scope.
-  bool isBoxed(Local variable) => false;
+  bool isBoxedVariable(Local variable) => false;
 }
 
 /// Class representing the usage of a scope that has been captured in the
@@ -299,10 +299,6 @@
   /// scopes.
   void forEachFreeVariable(f(Local variable, FieldEntity field)) {}
 
-  /// Return true if [variable] has been captured and mutated (all other
-  /// variables do not require boxing).
-  bool isVariableBoxed(Local variable) => false;
-
   // TODO(efortuna): Remove this method. The old system was using
   // ClosureClassMaps for situations other than closure class maps, and that's
   // just confusing.
diff --git a/pkg/compiler/lib/src/inferrer/builder_kernel.dart b/pkg/compiler/lib/src/inferrer/builder_kernel.dart
index cbb7f3a..7c2d111 100644
--- a/pkg/compiler/lib/src/inferrer/builder_kernel.dart
+++ b/pkg/compiler/lib/src/inferrer/builder_kernel.dart
@@ -56,6 +56,7 @@
       <JumpTarget, List<LocalsHandler>>{};
   TypeInformation _returnType;
   final Set<Local> _capturedVariables = new Set<Local>();
+  final Map<Local, FieldEntity> _capturedAndBoxed;
 
   /// Whether we currently collect [IsCheck]s.
   bool _accumulateIsChecks = false;
@@ -69,7 +70,7 @@
 
   KernelTypeGraphBuilder(this._options, this._closedWorld, this._inferrer,
       this._analyzedMember, this._analyzedNode, this._localsMap,
-      [this._locals])
+      [this._locals, Map<Local, FieldEntity> capturedAndBoxed])
       : this._types = _inferrer.types,
         this._memberData = _inferrer.dataOfMember(_analyzedMember),
         // TODO(johnniwinther): Should side effects also be tracked for field
@@ -78,7 +79,10 @@
             ? _inferrer.inferredDataBuilder
                 .getSideEffectsBuilder(_analyzedMember)
             : new SideEffectsBuilder.free(_analyzedMember),
-        this._inGenerativeConstructor = _analyzedNode is ir.Constructor {
+        this._inGenerativeConstructor = _analyzedNode is ir.Constructor,
+        this._capturedAndBoxed = capturedAndBoxed != null
+            ? new Map<Local, FieldEntity>.from(capturedAndBoxed)
+            : <Local, FieldEntity>{} {
     if (_locals != null) return;
 
     FieldInitializationScope fieldScope =
@@ -170,7 +174,7 @@
     // previous analysis of [outermostElement].
     ScopeInfo scopeInfo = _closureDataLookup.getScopeInfo(_analyzedMember);
     scopeInfo.forEachBoxedVariable((variable, field) {
-      _locals.setCapturedAndBoxed(variable, field);
+      _capturedAndBoxed[variable] = field;
     });
 
     return visit(_analyzedNode);
@@ -225,8 +229,8 @@
   void handleParameter(ir.VariableDeclaration node, {bool isOptional}) {
     Local local = _localsMap.getLocalVariable(node);
     DartType type = _localsMap.getLocalType(_elementMap, local);
-    _locals.update(
-        _inferrer, local, _inferrer.typeOfParameter(local), node, type);
+    _locals.update(_inferrer, _capturedAndBoxed, local,
+        _inferrer.typeOfParameter(local), node, type);
     if (isOptional) {
       TypeInformation type;
       if (node.initializer != null) {
@@ -683,9 +687,11 @@
     Local local = _localsMap.getLocalVariable(node);
     DartType type = _localsMap.getLocalType(_elementMap, local);
     if (node.initializer == null) {
-      _locals.update(_inferrer, local, _types.nullType, node, type);
+      _locals.update(
+          _inferrer, _capturedAndBoxed, local, _types.nullType, node, type);
     } else {
-      _locals.update(_inferrer, local, visit(node.initializer), node, type);
+      _locals.update(_inferrer, _capturedAndBoxed, local,
+          visit(node.initializer), node, type);
     }
     if (node.initializer is ir.ThisExpression) {
       _markThisAsExposed();
@@ -696,7 +702,7 @@
   @override
   TypeInformation visitVariableGet(ir.VariableGet node) {
     Local local = _localsMap.getLocalVariable(node.variable);
-    TypeInformation type = _locals.use(_inferrer, local);
+    TypeInformation type = _locals.use(_inferrer, _capturedAndBoxed, local);
     assert(type != null, "Missing type information for $local.");
     return type;
   }
@@ -709,7 +715,7 @@
     }
     Local local = _localsMap.getLocalVariable(node.variable);
     DartType type = _localsMap.getLocalType(_elementMap, local);
-    _locals.update(_inferrer, local, rhsType, node, type);
+    _locals.update(_inferrer, _capturedAndBoxed, local, rhsType, node, type);
     return rhsType;
   }
 
@@ -821,7 +827,8 @@
         TypeInformation refinedType = _types
             .refineReceiver(selector, mask, receiverType, isConditional: false);
         DartType type = _localsMap.getLocalType(_elementMap, local);
-        _locals.update(_inferrer, local, refinedType, node, type);
+        _locals.update(
+            _inferrer, _capturedAndBoxed, local, refinedType, node, type);
         List<Refinement> refinements = _localRefinementMap[variable];
         if (refinements != null) {
           refinements.add(new Refinement(selector, mask));
@@ -904,12 +911,14 @@
       if (refinements.isNotEmpty) {
         Local local = _localsMap.getLocalVariable(alias);
         DartType type = _localsMap.getLocalType(_elementMap, local);
-        TypeInformation localType = _locals.use(_inferrer, local);
+        TypeInformation localType =
+            _locals.use(_inferrer, _capturedAndBoxed, local);
         for (Refinement refinement in refinements) {
           localType = _types.refineReceiver(
               refinement.selector, refinement.mask, localType,
               isConditional: true);
-          _locals.update(_inferrer, local, localType, node, type);
+          _locals.update(
+              _inferrer, _capturedAndBoxed, local, localType, node, type);
         }
       }
     }
@@ -962,8 +971,8 @@
 
     Local variable = _localsMap.getLocalVariable(node.variable);
     DartType variableType = _localsMap.getLocalType(_elementMap, variable);
-    _locals.update(
-        _inferrer, variable, currentType, node.variable, variableType);
+    _locals.update(_inferrer, _capturedAndBoxed, variable, currentType,
+        node.variable, variableType);
 
     JumpTarget target = _localsMap.getJumpTargetForForIn(node);
     return handleLoop(node, target, () {
@@ -1345,18 +1354,19 @@
       List<IsCheck> positiveTests, List<IsCheck> negativeTests) {
     for (IsCheck check in positiveTests) {
       if (check.type != null) {
-        _locals.narrow(_inferrer, check.local, check.type, check.node);
+        _locals.narrow(
+            _inferrer, _capturedAndBoxed, check.local, check.type, check.node);
       } else {
         DartType localType = _localsMap.getLocalType(_elementMap, check.local);
-        _locals.update(
-            _inferrer, check.local, _types.nullType, check.node, localType);
+        _locals.update(_inferrer, _capturedAndBoxed, check.local,
+            _types.nullType, check.node, localType);
       }
     }
     for (IsCheck check in negativeTests) {
       if (check.type != null) {
         // TODO(johnniwinther): Use negative type knowledge.
       } else {
-        _locals.narrow(_inferrer, check.local,
+        _locals.narrow(_inferrer, _capturedAndBoxed, check.local,
             _closedWorld.commonElements.objectType, check.node);
       }
     }
@@ -1493,8 +1503,8 @@
     // Record the types of captured non-boxed variables. Types of
     // these variables may already be there, because of an analysis of
     // a previous closure.
-    info.forEachFreeVariable((variable, field) {
-      if (!info.isVariableBoxed(variable)) {
+    info.forEachFreeVariable((Local variable, FieldEntity field) {
+      if (!info.isBoxedVariable(variable)) {
         if (variable == info.thisLocal) {
           _inferrer.recordTypeOfField(field, thisType);
         }
@@ -1512,7 +1522,8 @@
     if (variable != null) {
       Local local = _localsMap.getLocalVariable(variable);
       DartType type = _localsMap.getLocalType(_elementMap, local);
-      _locals.update(_inferrer, local, localFunctionType, node, type);
+      _locals.update(
+          _inferrer, _capturedAndBoxed, local, localFunctionType, node, type);
     }
 
     // We don't put the closure in the work queue of the
@@ -1527,7 +1538,8 @@
         info.callMethod,
         functionNode,
         _localsMap,
-        closureLocals);
+        closureLocals,
+        _capturedAndBoxed);
     visitor.run();
     _inferrer.recordReturnType(info.callMethod, visitor._returnType);
 
@@ -1634,14 +1646,15 @@
         mask = _types.dynamicType;
       }
       Local local = _localsMap.getLocalVariable(exception);
-      _locals.update(_inferrer, local, mask, node, const DynamicType());
+      _locals.update(
+          _inferrer, _capturedAndBoxed, local, mask, node, const DynamicType());
     }
     ir.VariableDeclaration stackTrace = node.stackTrace;
     if (stackTrace != null) {
       Local local = _localsMap.getLocalVariable(stackTrace);
       // TODO(johnniwinther): Use a mask based on [StackTrace].
-      _locals.update(
-          _inferrer, local, _types.dynamicType, node, const DynamicType());
+      _locals.update(_inferrer, _capturedAndBoxed, local, _types.dynamicType,
+          node, const DynamicType());
     }
     visit(node.body);
     return null;
diff --git a/pkg/compiler/lib/src/inferrer/locals_handler.dart b/pkg/compiler/lib/src/inferrer/locals_handler.dart
index b0b1fd8..a0ad009 100644
--- a/pkg/compiler/lib/src/inferrer/locals_handler.dart
+++ b/pkg/compiler/lib/src/inferrer/locals_handler.dart
@@ -246,7 +246,6 @@
  */
 class LocalsHandler {
   final VariableScope locals;
-  final Map<Local, FieldEntity> _capturedAndBoxed;
   final FieldInitializationScope fieldScope;
   LocalsHandler tryBlock;
   bool seenReturnOrThrow = false;
@@ -258,45 +257,42 @@
 
   bool get inTryBlock => tryBlock != null;
 
-  LocalsHandler.internal(ir.Node block, this.fieldScope, this.locals,
-      this._capturedAndBoxed, this.tryBlock);
+  LocalsHandler.internal(
+      ir.Node block, this.fieldScope, this.locals, this.tryBlock);
 
   LocalsHandler(ir.Node block, [this.fieldScope])
       : locals = new VariableScope(block, isTry: false),
-        _capturedAndBoxed = new Map<Local, FieldEntity>(),
         tryBlock = null;
 
   LocalsHandler.from(LocalsHandler other, ir.Node block,
       {bool isTry: false, bool useOtherTryBlock: true})
       : locals = new VariableScope(block, isTry: isTry, parent: other.locals),
-        fieldScope = new FieldInitializationScope.from(other.fieldScope),
-        _capturedAndBoxed = other._capturedAndBoxed {
+        fieldScope = new FieldInitializationScope.from(other.fieldScope) {
     tryBlock = useOtherTryBlock ? other.tryBlock : this;
   }
 
   LocalsHandler.deepCopyOf(LocalsHandler other)
       : locals = new VariableScope.deepCopyOf(other.locals),
         fieldScope = new FieldInitializationScope.from(other.fieldScope),
-        _capturedAndBoxed = other._capturedAndBoxed,
         tryBlock = other.tryBlock;
 
   LocalsHandler.topLevelCopyOf(LocalsHandler other)
       : locals = new VariableScope.topLevelCopyOf(other.locals),
         fieldScope = new FieldInitializationScope.from(other.fieldScope),
-        _capturedAndBoxed = other._capturedAndBoxed,
         tryBlock = other.tryBlock;
 
-  TypeInformation use(InferrerEngine inferrer, Local local) {
-    if (_capturedAndBoxed.containsKey(local)) {
-      FieldEntity field = _capturedAndBoxed[local];
+  TypeInformation use(InferrerEngine inferrer,
+      Map<Local, FieldEntity> capturedAndBoxed, Local local) {
+    FieldEntity field = capturedAndBoxed[local];
+    if (field != null) {
       return inferrer.typeOfMember(field);
     } else {
       return locals[local];
     }
   }
 
-  void update(InferrerEngine inferrer, Local local, TypeInformation type,
-      ir.Node node, DartType staticType,
+  void update(InferrerEngine inferrer, Map<Local, FieldEntity> capturedAndBoxed,
+      Local local, TypeInformation type, ir.Node node, DartType staticType,
       {bool isSetIfNull: false}) {
     assert(type != null);
     if (!inferrer.options.assignmentCheckPolicy.isIgnored) {
@@ -318,8 +314,9 @@
       locals[local] = type;
     }
 
-    if (_capturedAndBoxed.containsKey(local)) {
-      inferrer.recordTypeOfField(_capturedAndBoxed[local], type);
+    FieldEntity field = capturedAndBoxed[local];
+    if (field != null) {
+      inferrer.recordTypeOfField(field, type);
     } else if (inTryBlock) {
       // We don't know if an assignment in a try block
       // will be executed, so all assignments in that block are
@@ -343,16 +340,14 @@
     }
   }
 
-  void narrow(InferrerEngine inferrer, Local local, DartType type, ir.Node node,
+  void narrow(InferrerEngine inferrer, Map<Local, FieldEntity> capturedAndBoxed,
+      Local local, DartType type, ir.Node node,
       {bool isSetIfNull: false}) {
-    TypeInformation existing = use(inferrer, local);
+    TypeInformation existing = use(inferrer, capturedAndBoxed, local);
     TypeInformation newType =
         inferrer.types.narrowType(existing, type, isNullable: false);
-    update(inferrer, local, newType, node, type, isSetIfNull: isSetIfNull);
-  }
-
-  void setCapturedAndBoxed(Local local, FieldEntity field) {
-    _capturedAndBoxed[local] = field;
+    update(inferrer, capturedAndBoxed, local, newType, node, type,
+        isSetIfNull: isSetIfNull);
   }
 
   void mergeDiamondFlow(InferrerEngine inferrer, LocalsHandler thenBranch,
diff --git a/pkg/compiler/lib/src/js_model/closure.dart b/pkg/compiler/lib/src/js_model/closure.dart
index 791b525..f6d4e82 100644
--- a/pkg/compiler/lib/src/js_model/closure.dart
+++ b/pkg/compiler/lib/src/js_model/closure.dart
@@ -744,7 +744,7 @@
     return sb.toString();
   }
 
-  bool isBoxed(Local variable) => boxedVariables.containsKey(variable);
+  bool isBoxedVariable(Local variable) => boxedVariables.containsKey(variable);
 
   factory JsScopeInfo.readFromDataSource(DataSource source) {
     source.begin(tag);
@@ -1045,19 +1045,11 @@
 
   FieldEntity get thisFieldEntity => localToFieldMap[thisLocal];
 
-  @override
-  void forEachBoxedVariable(f(Local local, JField field)) {
-    boxedVariables.forEach(f);
-  }
-
   void forEachFreeVariable(f(Local variable, JField field)) {
     localToFieldMap.forEach(f);
     boxedVariables.forEach(f);
   }
 
-  bool isVariableBoxed(Local variable) =>
-      boxedVariables.keys.contains(variable);
-
   bool get isClosure => true;
 }
 
diff --git a/pkg/compiler/lib/src/ssa/builder_kernel.dart b/pkg/compiler/lib/src/ssa/builder_kernel.dart
index 07a0cdf..25993e7 100644
--- a/pkg/compiler/lib/src/ssa/builder_kernel.dart
+++ b/pkg/compiler/lib/src/ssa/builder_kernel.dart
@@ -1149,7 +1149,9 @@
     void _handleParameter(ir.VariableDeclaration variable) {
       Local local = localsMap.getLocalVariable(variable);
       if (nodeIsConstructorBody &&
-          closureDataLookup.getCapturedScope(targetElement).isBoxed(local)) {
+          closureDataLookup
+              .getCapturedScope(targetElement)
+              .isBoxedVariable(local)) {
         // If local is boxed, then `variable` will be a field inside the box
         // passed as the last parameter, so no need to update our locals
         // handler or check types at this point.
@@ -5319,7 +5321,8 @@
     bool hasBox = false;
     forEachOrderedParameter(closedWorld.globalLocalsMap, _elementMap, function,
         (Local parameter) {
-      if (forGenerativeConstructorBody && scopeData.isBoxed(parameter)) {
+      if (forGenerativeConstructorBody &&
+          scopeData.isBoxedVariable(parameter)) {
         // The parameter will be a field in the box passed as the last
         // parameter. So no need to have it.
         hasBox = true;
diff --git a/pkg/compiler/lib/src/ssa/locals_handler.dart b/pkg/compiler/lib/src/ssa/locals_handler.dart
index 57f2d1d..98e128f 100644
--- a/pkg/compiler/lib/src/ssa/locals_handler.dart
+++ b/pkg/compiler/lib/src/ssa/locals_handler.dart
@@ -202,7 +202,7 @@
 
     parameters.forEach((Local local, AbstractValue typeMask) {
       if (isGenerativeConstructorBody) {
-        if (scopeData.isBoxed(local)) {
+        if (scopeData.isBoxedVariable(local)) {
           // The parameter will be a field in the box passed as the
           // last parameter. So no need to have it.
           return;
diff --git a/tests/compiler/dart2js/closure/closure_test.dart b/tests/compiler/dart2js/closure/closure_test.dart
index fc68583..3cc81e2 100644
--- a/tests/compiler/dart2js/closure/closure_test.dart
+++ b/tests/compiler/dart2js/closure/closure_test.dart
@@ -215,7 +215,7 @@
     } else {
       //Expect.isFalse(capturedScope.localIsUsedInTryOrSync(local));
     }
-    if (capturedScope.isBoxed(local)) {
+    if (capturedScope.isBoxedVariable(local)) {
       features.add('boxed');
     }
     if (capturedScope.context == local) {