Split getClosureRepresentationInfo into MemberEntity and (ir/ast) nodes

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 16cfea58c6bd51109c12b82fbe8312d233aaf629..b4ea669a0c869b932eb98a0bc5c80181d4b0e09b 100644
--- a/pkg/compiler/lib/src/js_model/closure.dart
+++ b/pkg/compiler/lib/src/js_model/closure.dart
@@ -70,11 +70,15 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
   final Map<MemberEntity, ScopeModel> _closureModels;
 
   /// Map of the scoping information that corresponds to a particular entity.
-  Map<Entity, ScopeInfo> _scopeMap = <Entity, ScopeInfo>{};
+  Map<MemberEntity, ScopeInfo> _scopeMap = <MemberEntity, ScopeInfo>{};
   Map<ir.Node, CapturedScope> _capturedScopesMap = <ir.Node, CapturedScope>{};
 
-  Map<Entity, ClosureRepresentationInfo> _closureRepresentationMap =
-      <Entity, ClosureRepresentationInfo>{};
+  Map<MemberEntity, ClosureRepresentationInfo> _memberClosureRepresentationMap =
+      <MemberEntity, ClosureRepresentationInfo>{};
+
+  // The key is either a [ir.FunctionDeclaration] or [ir.FunctionExpression].
+  Map<ir.Node, ClosureRepresentationInfo> _localClosureRepresentationMap =
+      <ir.Node, ClosureRepresentationInfo>{};
 
   KernelClosureConversionTask(Measurer measurer, this._elementMap,
       this._globalLocalsMap, this._closureModels)
@@ -138,20 +142,20 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
 
     // We want the original declaration where that function is used to point
     // to the correct closure class.
-    _closureRepresentationMap[closureClass.callMethod] = closureClass;
-    Entity entity;
+    _memberClosureRepresentationMap[closureClass.callMethod] = closureClass;
     if (node.parent is ir.Member) {
-      entity = _elementMap.getMember(node.parent);
+      assert(_elementMap.getMember(node.parent) == member);
+      _memberClosureRepresentationMap[member] = closureClass;
     } else {
-      entity = localsMap.getLocalFunction(node.parent);
+      assert(node.parent is ir.FunctionExpression ||
+          node.parent is ir.FunctionDeclaration);
+      _localClosureRepresentationMap[node.parent] = closureClass;
     }
-    assert(entity != null);
-    _closureRepresentationMap[entity] = closureClass;
     return closureClass;
   }
 
   @override
-  ScopeInfo getScopeInfo(Entity entity) {
+  ScopeInfo getScopeInfo(MemberEntity 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].
@@ -160,7 +164,7 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
       entity = constructorBody.constructor;
     }
 
-    return _scopeMap[entity] ?? getClosureRepresentationInfo(entity);
+    return _scopeMap[entity] ?? getMemberRepresentationInfo(entity);
   }
 
   // TODO(efortuna): Eventually capturedScopesMap[node] should always
@@ -186,19 +190,35 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
       _capturedScopesMap[loopNode] ?? const CapturedLoopScope();
 
   @override
-  ClosureRepresentationInfo getClosureRepresentationInfo(Entity entity) {
-    var closure = _closureRepresentationMap[entity];
+  ClosureRepresentationInfo getMemberRepresentationInfo(MemberEntity entity) {
+    var closure = _memberClosureRepresentationMap[entity];
     assert(
         closure != null,
         "Corresponding closure class not found for $entity. "
-        "Closures found for ${_closureRepresentationMap.keys}");
+        "Closures found for ${_memberClosureRepresentationMap.keys}");
+    return closure;
+  }
+
+  @override
+  ClosureRepresentationInfo getClosureRepresentationInfo(ir.Node node) {
+    var closure = _localClosureRepresentationMap[node];
+    assert(
+        closure != null,
+        "Corresponding closure class not found for $node. "
+        "Closures found for ${_localClosureRepresentationMap.keys}");
     return closure;
   }
 
   @override
+  ClosureRepresentationInfo getMemberRepresentationInfoForTesting(
+      MemberEntity entity) {
+    return _memberClosureRepresentationMap[entity];
+  }
+
+  @override
   ClosureRepresentationInfo getClosureRepresentationInfoForTesting(
-      Entity member) {
-    return _closureRepresentationMap[member];
+      ir.Node node) {
+    return _localClosureRepresentationMap[node];
   }
 }
 
@@ -355,6 +375,11 @@ class KernelClosureClass extends JsScopeInfo
       KernelToLocalsMap localsMap)
       : closureEntity = closureSourceNode.parent is ir.Member
             ? null
+            // TODO(johnniwinther,efortuna): This is the only place we call

[Emily Fortuna, 2017/08/18 21:04:43]

Hey Johnni, can you please clarify this comment? I don't understand what you're
suggesting we do in the future.

[Johnni Winther, 2017/08/19 08:28:28]

If `closureSourceNode.parent` is an [ir.FunctionDeclaration] we should use the
local for its variable. If `closureSourceNode.parent` is an
[ir.FunctionExpression], we should create a fresh local.

+            // [getLocalFunction]. Therefore the [closureEntity] doesn't need
+            // to be derived from the node. If anything, the local should be the
+            // one associated with variable on an [ir.FunctionDeclaration],
+            // `closureSourceNode.parent` is not an [ir.FunctionExpression].
             : localsMap.getLocalFunction(closureSourceNode.parent),
         thisLocal =
             info.hasThisLocal ? new ThisLocal(localsMap.currentMember) : null,