Fix the locals lookup of variables and partial implementation of boxing of variables.

pkg/compiler/lib/src/js_model/closure.dart

Index: pkg/compiler/lib/src/js_model/closure.dart
diff --git a/pkg/compiler/lib/src/js_model/closure.dart b/pkg/compiler/lib/src/js_model/closure.dart
index 1e086df0ee1e031e229e06446932c0be6b491f6b..bf4fad2c23e435e4caea867d47b2f3041d2dd06b 100644
--- a/pkg/compiler/lib/src/js_model/closure.dart
+++ b/pkg/compiler/lib/src/js_model/closure.dart
@@ -96,25 +96,41 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
       JsClosedWorld closedWorldRefiner) {
     closureModels.forEach((MemberEntity member, ScopeModel model) {
       KernelToLocalsMap localsMap = _globalLocalsMap.getLocalsMap(member);
-      if (model.scopeInfo != null) {
-        _scopeMap[member] = new JsScopeInfo.from(model.scopeInfo, localsMap);
-      }
-
+      assert(model.scopeInfo != null);
+      var boxedVariables =
+          _elementMap.makeRecordContainer(model.scopeInfo, member, localsMap);
+      _scopeMap[member] =
+          new JsScopeInfo.from(boxedVariables, model.scopeInfo, localsMap);
+      print('created the following scope info $member ${_scopeMap[member]}');
+
+      Set<JsCapturedScope> capturedScopes = new Set<JsCaptuerdScope>();
       model.capturedScopesMap
           .forEach((ir.Node node, KernelCapturedScope scope) {
+        boxedVariables = _elementMap.makeRecordContainer(
+            model.scopeInfo,
+            _elementMap.getMember(node),
+            localsMap); // TODO: (maybe don't need to call this again.)
         if (scope is KernelCapturedLoopScope) {
           _capturedScopesMap[node] =
-              new JsCapturedLoopScope.from(scope, localsMap);
+              new JsCapturedLoopScope.from(boxedVariables, scope, localsMap);
         } else {
-          _capturedScopesMap[node] = new JsCapturedScope.from(scope, localsMap);
+          _capturedScopesMap[node] =
+              new JsCapturedScope.from(boxedVariables, scope, localsMap);
         }
+        capturedScopes.add(_capturedScopesMap[node]);
+        print('captured scope info $node ${_capturedScopesMap[node]}');
       });
 
       Map<ir.FunctionNode, KernelScopeInfo> closuresToGenerate =
           model.closuresToGenerate;
       for (ir.FunctionNode node in closuresToGenerate.keys) {
-        _produceSyntheticElements(
-            member, node, closuresToGenerate[node], closedWorldRefiner);
+        KernelClosureClass closureClass = _produceSyntheticElements(member,
+            node, closuresToGenerate[node], capturedScopes, closedWorldRefiner);
+        // Add one for each call method.
+        KernelToLocalsMap localsMap =
+            _globalLocalsMap.getLocalsMap(closureClass.callMethod);
+        _scopeMap[closureClass.callMethod] = closureClass;
+        print('then we made $closureClass starting from ${node.parent}');
       }
     });
   }
@@ -125,11 +141,22 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
   /// the closure accesses a variable that gets accessed at some point), then
   /// boxForCapturedVariables stores the local context for those variables.
   /// If no variables are captured, this parameter is null.
-  void _produceSyntheticElements(MemberEntity member, ir.FunctionNode node,
-      KernelScopeInfo info, JsClosedWorld closedWorldRefiner) {
+  KernelClosureClass _produceSyntheticElements(
+      MemberEntity member,
+      ir.FunctionNode node,
+      KernelScopeInfo info,
+      Set<JsCaptuerdScope> capturedScopes,
+      JsClosedWorld closedWorldRefiner) {
     KernelToLocalsMap localsMap = _globalLocalsMap.getLocalsMap(member);
+
     KernelClosureClass closureClass = closedWorldRefiner.buildClosureClass(
-        member, node, member.library, info, node.location, localsMap);
+        member,
+        node,
+        member.library,
+        capturedScopes,
+        info,
+        node.location,
+        localsMap);
 
     // We want the original declaration where that function is used to point
     // to the correct closure class.
@@ -142,6 +169,7 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
     }
     assert(entity != null);
     _closureRepresentationMap[entity] = closureClass;
+    return closureClass;
   }
 
   @override
@@ -157,8 +185,6 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
     return _scopeMap[entity] ?? getClosureRepresentationInfo(entity);
   }
 
-  // TODO(efortuna): Eventually capturedScopesMap[node] should always
-  // be non-null, and we should just test that with an assert.
   @override
   CapturedScope getCapturedScope(MemberEntity entity) {
     MemberDefinition definition = _elementMap.getMemberDefinition(entity);
@@ -174,8 +200,6 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
   }
 
   @override
-  // TODO(efortuna): Eventually capturedScopesMap[node] should always
-  // be non-null, and we should just test that with an assert.
   CapturedLoopScope getCapturedLoopScope(ir.Node loopNode) =>
       _capturedScopesMap[loopNode] ?? const CapturedLoopScope();
 
@@ -208,6 +232,11 @@ class KernelScopeInfo {
   /// this scope.
   Set<ir.VariableDeclaration> freeVariables = new Set<ir.VariableDeclaration>();
 
+  /// The set of scopes that this scope captures. In practice we only populate
+  /// it if this scope uses a particular variable that is defined in another
+  /// scope, and this information is only really useful for closures.
+  final Set<ir.Node> capturedScopes = new Set<ir.Node>();
+
   KernelScopeInfo(this.hasThisLocal)
       : localsUsedInTryOrSync = new Set<ir.VariableDeclaration>(),
         boxedVariables = new Set<ir.VariableDeclaration>(),
@@ -228,7 +257,10 @@ class KernelScopeInfo {
   String toString() {
     StringBuffer sb = new StringBuffer();
     sb.write('this=$hasThisLocal,');
-    sb.write('localsUsedInTryOrSync={${localsUsedInTryOrSync.join(', ')}}');
+    sb.write('localsUsedInTryOrSync={${localsUsedInTryOrSync.join(', ')}},');
+    sb.write('freeVariables={${freeVariables.join(', ')}},');
+    sb.write('boxedVariables={${boxedVariables.join(', ')}},');
+    sb.write('capturedVariablesAccessor=$capturedVariablesAccessor');
     return sb.toString();
   }
 }
@@ -236,29 +268,24 @@ class KernelScopeInfo {
 class JsScopeInfo extends ScopeInfo {
   final Set<Local> localsUsedInTryOrSync;
   final Local thisLocal;
-  final Set<Local> boxedVariables;
+  final Map<Local, JRecord> boxedVariables;
 
   /// The set of variables that were defined in another scope, but are used in
   /// this scope.
   final Set<Local> freeVariables;
 
-  JsScopeInfo(this.thisLocal, this.localsUsedInTryOrSync, this.boxedVariables,
-      this.freeVariables);
-
-  JsScopeInfo.from(KernelScopeInfo info, KernelToLocalsMap localsMap)
+  JsScopeInfo.from(
+      this.boxedVariables, KernelScopeInfo info, KernelToLocalsMap localsMap)
       : this.thisLocal =
             info.hasThisLocal ? new ThisLocal(localsMap.currentMember) : null,
         this.localsUsedInTryOrSync =
             info.localsUsedInTryOrSync.map(localsMap.getLocalVariable).toSet(),
-        this.boxedVariables =
-            info.boxedVariables.map(localsMap.getLocalVariable).toSet(),
         this.freeVariables =
             info.freeVariables.map(localsMap.getLocalVariable).toSet();
 
   void forEachBoxedVariable(f(Local local, FieldEntity field)) {
-    boxedVariables.forEach((Local l) {
-      // TODO(efortuna): add FieldEntities as created.
-      f(l, null);
+    boxedVariables.forEach((Local l, JRecord box) {
+      f(l, box);
     });
   }
 
@@ -267,21 +294,21 @@ class JsScopeInfo extends ScopeInfo {
 
   String toString() {
     StringBuffer sb = new StringBuffer();
+    sb.write('JsScopeInfo: ');
     sb.write('this=$thisLocal,');
     sb.write('localsUsedInTryOrSync={${localsUsedInTryOrSync.join(', ')}}');
+    sb.write('freeVariables={${freeVariables.join(', ')}}');
+    sb.write('boxedVariables={$boxedVariables}');
     return sb.toString();
   }
 
-  bool isBoxed(Local variable) => boxedVariables.contains(variable);
+  bool isBoxed(Local variable) => boxedVariables.containsKey(variable);
 }
 
 class KernelCapturedScope extends KernelScopeInfo {
-  final ir.TreeNode context;
-
   KernelCapturedScope(
       Set<ir.VariableDeclaration> boxedVariables,
       NodeBox capturedVariablesAccessor,
-      this.context,
       Set<ir.VariableDeclaration> localsUsedInTryOrSync,
       Set<ir.VariableDeclaration> freeVariables,
       bool hasThisLocal)
@@ -292,14 +319,25 @@ class KernelCapturedScope extends KernelScopeInfo {
 }
 
 class JsCapturedScope extends JsScopeInfo implements CapturedScope {
-  final Local context;
+  final BoxLocal context;
 
-  JsCapturedScope.from(
+  JsCapturedScope.from(Map<Local, JRecord> boxedVariables,
       KernelCapturedScope capturedScope, KernelToLocalsMap localsMap)
-      : this.context = localsMap.getLocalVariable(capturedScope.context),
-        super.from(capturedScope, localsMap);
+      : this.context =
+            boxedVariables.isNotEmpty ? boxedVariables.values.first.box : null,
+        super.from(boxedVariables, capturedScope, localsMap);
 
   bool get requiresContextBox => boxedVariables.isNotEmpty;
+
+  String toString() {
+    StringBuffer sb = new StringBuffer();
+    sb.write('this=$thisLocal,');
+    sb.write('localsUsedInTryOrSync={${localsUsedInTryOrSync.join(', ')}}');
+    sb.write('freeVariables={${freeVariables.join(', ')}}');
+    sb.write('boxedVariables={$boxedVariables}');
+    sb.write('context={$context}');
+    return sb.toString();
+  }
 }
 
 class KernelCapturedLoopScope extends KernelCapturedScope {
@@ -309,12 +347,11 @@ class KernelCapturedLoopScope extends KernelCapturedScope {
       Set<ir.VariableDeclaration> boxedVariables,
       NodeBox capturedVariablesAccessor,
       this.boxedLoopVariables,
-      ir.TreeNode context,
       Set<ir.VariableDeclaration> localsUsedInTryOrSync,
       Set<ir.VariableDeclaration> freeVariables,
       bool hasThisLocal)
-      : super(boxedVariables, capturedVariablesAccessor, context,
-            localsUsedInTryOrSync, freeVariables, hasThisLocal);
+      : super(boxedVariables, capturedVariablesAccessor, localsUsedInTryOrSync,
+            freeVariables, hasThisLocal);
 
   bool get hasBoxedLoopVariables => boxedLoopVariables.isNotEmpty;
 }
@@ -322,36 +359,36 @@ class KernelCapturedLoopScope extends KernelCapturedScope {
 class JsCapturedLoopScope extends JsCapturedScope implements CapturedLoopScope {
   final List<Local> boxedLoopVariables;
 
-  JsCapturedLoopScope.from(
+  JsCapturedLoopScope.from(Map<Local, JRecord> boxedVariables,
       KernelCapturedLoopScope capturedScope, KernelToLocalsMap localsMap)
       : this.boxedLoopVariables = capturedScope.boxedLoopVariables
             .map(localsMap.getLocalVariable)
             .toList(),
-        super.from(capturedScope, localsMap);
+        super.from(boxedVariables, capturedScope, localsMap);
 
   bool get hasBoxedLoopVariables => boxedLoopVariables.isNotEmpty;
 }
 
 // TODO(johnniwinther): Add unittest for the computed [ClosureClass].
 class KernelClosureClass extends JsScopeInfo
-    implements ClosureRepresentationInfo, JClass {
-  final String name;
-  final JLibrary library;
+    implements ClosureRepresentationInfo {
   JFunction callMethod;
   final Local closureEntity;
   final Local thisLocal;
-
-  /// Index into the classData, classList and classEnvironment lists where this
-  /// entity is stored in [JsToFrontendMapImpl].
-  final int classIndex;
+  final ClassEntity closureClassEntity;
 
   final Map<Local, JField> localToFieldMap = new Map<Local, JField>();
 
+  /// The set of scopes that this scope captures. In practice we only populate
+  /// it if this scope uses a particular variable that is defined in another
+  /// scope, and this information is only really useful for closures.
+  final Set<JsCapturedScope> capturedScopes;
+
   KernelClosureClass.fromScopeInfo(
+      this.closureClassEntity,
       ir.FunctionNode closureSourceNode,
-      this.name,
-      this.classIndex,
-      this.library,
+      Map<Local, JRecord> boxedVariables,
+      this.capturedScopes,
       KernelScopeInfo info,
       KernelToLocalsMap localsMap)
       : closureEntity = closureSourceNode.parent is ir.Member
@@ -359,9 +396,7 @@ class KernelClosureClass extends JsScopeInfo
             : localsMap.getLocalFunction(closureSourceNode.parent),
         thisLocal =
             info.hasThisLocal ? new ThisLocal(localsMap.currentMember) : null,
-        super.from(info, localsMap);
-
-  ClassEntity get closureClassEntity => this;
+        super.from(boxedVariables, info, localsMap);
 
   List<Local> get createdFieldEntities => localToFieldMap.keys.toList();
 
@@ -374,25 +409,36 @@ class KernelClosureClass extends JsScopeInfo
   @override
   void forEachBoxedVariable(f(Local local, JField field)) {
     for (Local l in localToFieldMap.keys) {
-      if (localToFieldMap[l] is JBoxedField) f(l, localToFieldMap[l]);
+      if (localToFieldMap[l] is JRecord) f(l, localToFieldMap[l]);
+    }
+    for (JsCapturedScope scope in capturedScopes) {
+      scope.forEachBoxedVariable(f);
     }
   }
 
   void forEachFreeVariable(f(Local variable, JField field)) {
     for (Local l in localToFieldMap.keys) {
       var jField = localToFieldMap[l];
-      if (jField is! JBoxedField && jField is! BoxLocal) f(l, jField);
+      if (jField is! BoxLocal) f(l, jField);
     }
   }
 
-  bool isVariableBoxed(Local variable) =>
-      localToFieldMap.keys.contains(variable);
-
-  bool get isClosure => true;
+  bool isVariableBoxed(Local variable) {
+    print(
+        'RRRRRRRRRRR ${localToFieldMap.keys.contains(variable)} ${capturedScopes}');
+    return localToFieldMap.keys.contains(variable) ||
+        capturedScopes.any((JsCapturedScope scope) => scope.isBoxed(variable));
+  }
 
-  bool get isAbstract => false;
+  @override
+  bool isBoxed(Local variable) {
+    print(
+        'RRRRRRRRRRR ${localToFieldMap.keys.contains(variable)} ${capturedScopes}');
+    return localToFieldMap.keys.contains(variable) ||
+        capturedScopes.any((JsCapturedScope scope) => scope.isBoxed(variable));
+  }
 
-  String toString() => '${jsElementPrefix}class($name)';
+  bool get isClosure => true;
 }
 
 /// A local variable to disambiguate between a variable that has been captured
@@ -404,23 +450,62 @@ class NodeBox {
   NodeBox(this.name, this.executableContext);
 }
 
+class JClosureClass extends JClass {
+  // TODO(efortuna): omg this is so horrible.
+  final KernelToLocalsMap localsMap;
+
+  JClosureClass(this.localsMap, JLibrary library, int classIndex, String name)
+      : super(library, classIndex, name, isAbstract: false);
+
+  @override
+  bool get isClosure => true;
+
+  String toString() => '${jsElementPrefix}closure_class($name)';
+}
+
 class JClosureField extends JField {
   JClosureField(String name, int memberIndex,
       KernelClosureClass containingClass, bool isConst, bool isAssignable)
-      : super(memberIndex, containingClass.library, containingClass,
-            new Name(name, containingClass.library),
-            isAssignable: isAssignable, isConst: isConst, isStatic: false);
+      : super(
+            memberIndex,
+            containingClass.closureClassEntity.library,
+            containingClass.closureClassEntity,
+            new Name(name, containingClass.closureClassEntity.library),
+            isAssignable: isAssignable,
+            isConst: isConst,
+            isStatic: false);
+}
+
+/// A container for variables declared in a particular scope that are accessed
+/// elsewhere.
+// TODO(efortuna, johnniwinther): Don't implement JClass. This isn't actually a
+// class.
+class JRecordContainer implements JClass {
+  final JLibrary library;
+  final String name;
+
+  /// Index into the classData, classList and classEnvironment lists where this
+  /// entity is stored in [JsToFrontendMapImpl].
+  final int classIndex;
+
+  JRecordContainer(this.library, this.classIndex, this.name);
+
+  bool get isAbstract => false;
+
+  bool get isClosure => false;
+
+  String toString() => '${jsElementPrefix}record_container($name)';
 }
 
-/// A ClosureField that has been "boxed" to prevent name shadowing with the
+/// A variable that has been "boxed" to prevent name shadowing with the
 /// original variable and ensure that this variable is updated/read with the
 /// most recent value.
 /// This corresponds to BoxFieldElement; we reuse BoxLocal from the original
 /// algorithm to correspond to the actual name of the variable.
-class JBoxedField extends JField {
+class JRecord extends JField {
   final BoxLocal box;
-  JBoxedField(String name, int memberIndex, this.box,
-      KernelClosureClass containingClass, bool isConst, bool isAssignable)
+  JRecord(String name, int memberIndex, this.box, JClass containingClass,
+      bool isConst, bool isAssignable)
       : super(memberIndex, containingClass.library, containingClass,
             new Name(name, containingClass.library),
             isAssignable: isAssignable, isConst: isConst);
@@ -434,11 +519,9 @@ class ClosureClassDefinition implements ClassDefinition {
 
   ClassKind get kind => ClassKind.closure;
 
-  ir.Node get node =>
-      throw new UnsupportedError('ClosureClassDefinition.node for $cls');
+  ir.Node get node => throw new UnsupportedError('JRecord.node for $cls');
 
-  String toString() =>
-      'ClosureClassDefinition(kind:$kind,cls:$cls,location:$location)';
+  String toString() => 'JRecord(kind:$kind,cls:$cls,location:$location)';
 }
 
 class ClosureMemberData implements MemberData {
@@ -517,6 +600,21 @@ class ClosureMemberDefinition implements MemberDefinition {
       'ClosureMemberDefinition(kind:$kind,member:$member,location:$location)';
 }
 
+class RecordContainerDefinition implements ClassDefinition {
+  final ClassEntity cls;
+  final SourceSpan location;
+
+  RecordContainerDefinition(this.cls, this.location);
+
+  ClassKind get kind => ClassKind.container;
+
+  ir.Node get node =>
+      throw new UnsupportedError('RecordContainerDefinition.node for $cls');
+
+  String toString() =>
+      'RecordContainerDefinition(kind:$kind,cls:$cls,location:$location)';
+}
+
 /// Collection of scope data collected for a single member.
 class ScopeModel {
   /// Collection [ScopeInfo] data for the member.