Distinguish between actual closure scopes and non-closure scopes.

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

 // Copyright (c) 2017, 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.
 
 import 'package:kernel/ast.dart' as ir;
 
 import '../closure.dart';
 import '../common/tasks.dart';
 import '../elements/entities.dart';
 import '../kernel/element_map.dart';
@@ skipping 13 matching lines @@
 /// For a general explanation of how closure conversion works at a high level,
 /// check out:
 /// http://siek.blogspot.com/2012/07/essence-of-closure-conversion.html or
 /// http://matt.might.net/articles/closure-conversion/.
 // TODO(efortuna): Change inheritance hierarchy so that the
 // ClosureConversionTask doesn't inherit from ClosureTask because it's just a
 // glorified timer.
 class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
   final KernelToElementMapForBuilding _elementMap;
   final GlobalLocalsMap _globalLocalsMap;
+
+  /// Map of the scoping information that corresponds to a particular entity.
+  /// This particular map contains ScopeInfos only for entities that are *not*
+  /// closures. Entities that are closures are stored in the
+  /// [_closureScopeMap].

[sra1, 2017/07/12 23:57:41]

Say why are they separate. Is it so these can be GC-ed after the closure task
but the others cannot?

[Emily Fortuna, 2017/07/13 21:03:39]

see comment below.

+  Map<Entity, ScopeInfo> _scopeMap = <Entity, ScopeInfo>{};
   Map<ir.Node, ClosureScope> _closureScopeMap = <ir.Node, ClosureScope>{};

[sra1, 2017/07/12 23:57:41]

Can this be Entity too?

[Emily Fortuna, 2017/07/13 21:03:39]

yes, ended up combining with the above, since these don't always get garbage
collected.

 
   Map<Entity, ClosureRepresentationInfo> _closureRepresentationMap =
       <Entity, ClosureRepresentationInfo>{};
 
   /// Should only be used at the very beginning to ensure we are looking at the
   /// right kind of elements.
   // TODO(efortuna): Remove this map once we have one kernel backend strategy.
   final JsToFrontendMap _kToJElementMap;
 
   KernelClosureConversionTask(Measurer measurer, this._elementMap,
@@ skipping 31 matching lines @@
   }
 
   /// Inspect members and mark if those members capture any state that needs to
   /// be marked as free variables.
   void _buildClosureModel(
       MemberEntity entity,
       Map<ir.TreeNode, ScopeInfo> closuresToGenerate,
       ClosedWorldRefiner closedWorldRefiner) {
     ir.Node node = _elementMap.getMemberNode(entity);
     if (_closureScopeMap.keys.contains(node)) return;
-    ClosureScopeBuilder translator = new ClosureScopeBuilder(_closureScopeMap,
-        closuresToGenerate, _globalLocalsMap.getLocalsMap(entity), _elementMap);
+    ClosureScopeBuilder translator = new ClosureScopeBuilder(
+        _closureScopeMap,
+        _scopeMap,
+        closuresToGenerate,
+        _globalLocalsMap.getLocalsMap(entity),
+        _elementMap);
     if (entity.isField) {
       if (node is ir.Field && node.initializer != null) {
         translator.translateLazyInitializer(node);
       }
     } else {
       assert(node is ir.Procedure || node is ir.Constructor);
       translator.translateConstructorOrProcedure(node);
     }
   }
 
@@ skipping 24 matching lines @@
 
     _closureRepresentationMap[entity] = closureClass;
 
     // Register that a new class has been created.
     closedWorldRefiner.registerClosureClass(
         closureClass, node is ir.Member && node.isInstanceMember);
   }
 
   @override
   ScopeInfo getScopeInfo(Entity entity) {
-    return getClosureRepresentationInfo(entity);
+    // TODO(johnniwinther): Remove this check when constructor bodies a created
+    // eagerly with the J-model; a constructor body should have it's own
+    // [ClosureRepresentationInfo].
+    if (entity is ConstructorBodyEntity) {
+      ConstructorBodyEntity constructorBody = entity;
+      entity = constructorBody.constructor;
+    }
+
+    return _scopeMap[entity] ?? getClosureRepresentationInfo(entity);
   }
 
   // TODO(efortuna): Eventually closureScopeMap[node] should always be non-null,
   // and we should just test that with an assert.
   ClosureScope _getClosureScope(ir.Node node) =>
       _closureScopeMap[node] ?? const ClosureScope();
 
   @override
   ClosureScope getClosureScope(MemberEntity entity) {
     return _getClosureScope(_elementMap.getMemberNode(entity));
   }
 
   @override
   // TODO(efortuna): Eventually closureScopeMap[node] should always be non-null,
   // and we should just test that with an assert.
   LoopClosureScope getLoopClosureScope(ir.Node loopNode) =>
       _closureScopeMap[loopNode] ?? const LoopClosureScope();
 
   @override
   // TODO(efortuna): Eventually closureRepresentationMap[node] should always be
   // non-null, and we should just test that with an assert.
   ClosureRepresentationInfo getClosureRepresentationInfo(Entity entity) {
-    // TODO(johnniwinther): Remove this check when constructor bodies a created
-    // eagerly with the J-model; a constructor body should have it's own
-    // [ClosureRepresentationInfo].
-    if (entity is ConstructorBodyEntity) {
-      ConstructorBodyEntity constructorBody = entity;
-      entity = constructorBody.constructor;
-    }
     return _closureRepresentationMap[entity] ??
         const ClosureRepresentationInfo();
   }
 }
 
 class KernelScopeInfo extends ScopeInfo {
   final Set<Local> localsUsedInTryOrSync;
   final Local thisLocal;
   final Set<Local> boxedVariables;
 
@@ skipping 101 matching lines @@
   // that (and the subsequent adjustment here) will follow soon.
   bool get isClosure => false;
 
   bool get isAbstract => false;
 
   // TODO(efortuna): Talk to Johnni.
   JLibrary get library => null;
 
   String toString() => '${jsElementPrefix}class($name)';
 }