Split getClosureRepresentationInfo into MemberEntity and (ir/ast) nodes

pkg/compiler/lib/src/closure.dart

 // Copyright (c) 2012, 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 'common/names.dart' show Identifiers;
 import 'common/resolution.dart' show ParsingContext, Resolution;
 import 'common/tasks.dart' show CompilerTask, Measurer;
 import 'common.dart';
 import 'compiler.dart' show Compiler;
 import 'constants/expressions.dart';
@@ skipping 24 matching lines @@
       ClosedWorldRefiner closedWorldRefiner);
 }
 
 /// Class that provides information for how closures are rewritten/represented
 /// to preserve Dart semantics when compiled to JavaScript. Given a particular
 /// node to look up, it returns a information about the internal representation
 /// of how closure conversion is implemented. T is an ir.Node or Node.
 abstract class ClosureDataLookup<T> {
   /// Look up information about the variables that have been mutated and are
   /// used inside the scope of [node].
-  // TODO(johnniwinther): Split this up into two functions, one for members and
-  // one for local functions.
-  ScopeInfo getScopeInfo(covariant Entity member);
+  ScopeInfo getScopeInfo(MemberEntity member);
 
   /// This returns the same information as ScopeInfo, but can be called in
   /// situations when you are sure you are dealing with a closure specifically.
-  ClosureRepresentationInfo getClosureRepresentationInfo(
-      covariant Entity member);
+  // TODO(johnniwinther,efortuna): Can we use [getScopeInfo] instead?

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

you should be able to. In fact, getScopeInfo currently calls
getClosureRepresentationInfo if it's not in the first map. 

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

Ok, I'll look into it.

+  ClosureRepresentationInfo getMemberRepresentationInfo(MemberEntity member);

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

consider calling these:
getClosureInfoForMember
getClosureInfo
getClosureInfoForMemberTesting

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

Done.

+
+  ClosureRepresentationInfo getClosureRepresentationInfo(T localFunction);
+
+  ClosureRepresentationInfo getMemberRepresentationInfoForTesting(
+      MemberEntity member);
 
   ClosureRepresentationInfo getClosureRepresentationInfoForTesting(
-      covariant Entity member);
+      T localFunction);
 
   /// Look up information about a loop, in case any variables it declares need
   /// to be boxed/snapshotted.
   CapturedLoopScope getCapturedLoopScope(T loopNode);
 
   /// Accessor to the information about scopes that closures capture. Used by
   /// the SSA builder.
   CapturedScope getCapturedScope(MemberEntity entity);
 }
 
@@ skipping 183 matching lines @@
   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.
   bool get isClosure => false;
 }
 
 class ClosureTask extends ClosureConversionTask<Node> {
   Map<Node, CapturedScopeImpl> _closureInfoMap = <Node, CapturedScopeImpl>{};
-  Map<Element, ClosureClassMap> _closureMappingCache =
-      <Element, ClosureClassMap>{};
+  Map<MemberElement, ClosureClassMap> _closureMemberMappingCache =
+      <MemberElement, ClosureClassMap>{};
+  Map<FunctionExpression, ClosureClassMap> _closureNodeMappingCache =
+      <FunctionExpression, ClosureClassMap>{};
   Compiler compiler;
   ClosureTask(Compiler compiler)
       : compiler = compiler,
         super(compiler.measurer);
 
   String get name => "Closure Simplifier";
 
   DiagnosticReporter get reporter => compiler.reporter;
 
   void convertClosures(Iterable<MemberEntity> processedEntities,
       ClosedWorldRefiner closedWorldRefiner) {
     createClosureClasses(closedWorldRefiner);
   }
 
   CapturedScope _getCapturedScope(Node node) {
     var value = _closureInfoMap[node];
     return value == null ? const CapturedScope() : value;
   }
 
   CapturedScope getCapturedScope(covariant MemberElement member) {
     ResolvedAst resolvedAst = member.resolvedAst;
     if (resolvedAst.kind != ResolvedAstKind.PARSED)
       return const CapturedScope();
     return _getCapturedScope(resolvedAst.node);
   }
 
-  ScopeInfo getScopeInfo(Element member) {
-    return getClosureToClassMapping(member);
+  ScopeInfo getScopeInfo(MemberEntity member) {
+    return _getMemberMapping(member);
   }
 
-  ClosureRepresentationInfo getClosureRepresentationInfo(Element member) {
-    return getClosureToClassMapping(member);
+  ClosureRepresentationInfo getMemberRepresentationInfo(MemberEntity member) {
+    return _getMemberMapping(member);
+  }
+
+  ClosureRepresentationInfo getClosureRepresentationInfo(
+      covariant FunctionExpression node) {
+    return _getClosureMapping(node);
+  }
+
+  ClosureRepresentationInfo getMemberRepresentationInfoForTesting(
+      MemberEntity member) {
+    return getMemberRepresentationInfo(member);
   }
 
   ClosureRepresentationInfo getClosureRepresentationInfoForTesting(
-      Element member) {
-    return getClosureRepresentationInfo(member);
+      covariant FunctionExpression node) {
+    return getClosureRepresentationInfo(node);
   }
 
   CapturedLoopScope getCapturedLoopScope(Node loopNode) {
     var value = _closureInfoMap[loopNode];
     return value == null ? const CapturedLoopScope() : value;
   }
 
-  /// Returns the [ClosureClassMap] computed for [resolvedAst].
-  ClosureClassMap getClosureToClassMapping(Element element) {
+  /// Returns the [ClosureClassMap] computed for [element].
+  ClosureClassMap _getMemberMapping(MemberElement element) {
     return measure(() {
       if (element.isGenerativeConstructorBody) {
         ConstructorBodyElement constructorBody = element;
         element = constructorBody.constructor;
       }
-      ClosureClassMap closureClassMap = _closureMappingCache[element];
+      ClosureClassMap closureClassMap = _closureMemberMappingCache[element];
       assert(closureClassMap != null,
           failedAt(element, "No ClosureClassMap computed for ${element}."));
       return closureClassMap;
     });
   }
 
+  /// Returns the [ClosureClassMap] computed for [node].
+  ClosureClassMap _getClosureMapping(FunctionExpression node) {
+    return measure(() {
+      ClosureClassMap closureClassMap = _closureNodeMappingCache[node];
+      assert(closureClassMap != null,
+          failedAt(node, "No ClosureClassMap computed for ${node}."));
+      return closureClassMap;
+    });
+  }
+
   /// Create [ClosureClassMap]s for all live members.
   void createClosureClasses(ClosedWorldRefiner closedWorldRefiner) {
     compiler.enqueuer.resolution.processedEntities
         .forEach((MemberEntity _element) {
       MemberElement element = _element;
       ResolvedAst resolvedAst = element.resolvedAst;
       if (element.isAbstract) return;
       if (element.isField &&
           !element.isInstanceMember &&
           resolvedAst.body == null) {
         // Skip top-level/static fields without an initializer.
         return;
       }
       computeClosureToClassMapping(element, closedWorldRefiner);
     });
   }
 
   ClosureClassMap computeClosureToClassMapping(
       MemberElement element, ClosedWorldRefiner closedWorldRefiner) {
     return measure(() {
-      ClosureClassMap cached = _closureMappingCache[element];
+      ClosureClassMap cached = _closureMemberMappingCache[element];
       if (cached != null) return cached;
       if (element.resolvedAst.kind != ResolvedAstKind.PARSED) {
-        return _closureMappingCache[element] =
+        return _closureMemberMappingCache[element] =
             new ClosureClassMap(null, null, null, new ThisLocal(element));
       }
       return reporter.withCurrentElement(element.implementation, () {
         Node node = element.resolvedAst.node;
         TreeElements elements = element.resolvedAst.elements;
 
         ClosureTranslator translator = new ClosureTranslator(
             compiler,
             closedWorldRefiner,
             elements,
-            _closureMappingCache,
+            _closureMemberMappingCache,
+            _closureNodeMappingCache,
             _closureInfoMap);
 
         // The translator will store the computed closure-mappings inside the
         // cache. One for given node and one for each nested closure.
         if (node is FunctionExpression) {
           translator.translateFunction(element, node);
         } else if (element.isSynthesized) {
           reporter.internalError(
               element, "Unexpected synthesized element: $element");
-          _closureMappingCache[element] =
+          _closureMemberMappingCache[element] =
               new ClosureClassMap(null, null, null, new ThisLocal(element));
         } else {
           assert(element.isField,
               failedAt(element, "Expected $element to be a field."));
           Node initializer = element.resolvedAst.body;
           if (initializer != null) {
             // The lazy initializer of a static.
             translator.translateLazyInitializer(element, node, initializer);
           } else {
             assert(
                 element.isInstanceMember,
                 failedAt(
                     element,
                     "Expected $element (${element.runtimeType}) "
                     "to be an instance field."));
-            _closureMappingCache[element] =
+            _closureMemberMappingCache[element] =
                 new ClosureClassMap(null, null, null, new ThisLocal(element));
           }
         }
-        assert(_closureMappingCache[element] != null,
+        assert(_closureMemberMappingCache[element] != null,
             failedAt(element, "No ClosureClassMap computed for ${element}."));
-        return _closureMappingCache[element];
+        return _closureMemberMappingCache[element];
       });
     });
   }
 }
 
 // TODO(ahe): These classes continuously cause problems.  We need to
 // find a more general solution.
 class ClosureFieldElement extends ElementX
     implements FieldElement, PrivatelyNamedJSEntity {
   /// The [BoxLocal] or [LocalElement] being accessed through the field.
@@ skipping 454 matching lines @@
 }
 
 class ClosureTranslator extends Visitor {
   final Compiler compiler;
   final ClosedWorldRefiner closedWorldRefiner;
   final TreeElements elements;
   int closureFieldCounter = 0;
   int boxedFieldCounter = 0;
   bool inTryStatement = false;
 
-  final Map<Element, ClosureClassMap> closureMappingCache;
+  final Map<MemberElement, ClosureClassMap> memberMappingCache;
+  final Map<FunctionExpression, ClosureClassMap> nodeMappingCache;
   final Map<Node, CapturedScopeImpl> closureInfo;
 
   // Map of captured variables. Initially they will map to `null`. If
   // a variable needs to be boxed then the scope declaring the variable
   // will update this to mapping to the capturing [BoxFieldElement].
   Map<Local, BoxFieldElement> _capturedVariableMapping =
       new Map<Local, BoxFieldElement>();
 
   // List of encountered closures.
-  List<LocalFunctionElement> closures = <LocalFunctionElement>[];
+  List<FunctionExpression> closures = <FunctionExpression>[];
 
   // The local variables that have been declared in the current scope.
   List<LocalVariableElement> scopeVariables;
 
   // Keep track of the mutated local variables so that we don't need to box
   // non-mutated variables.
   Set<LocalVariableElement> mutatedVariables = new Set<LocalVariableElement>();
 
   MemberElement outermostElement;
   ExecutableElement executableContext;
 
   // The closureData of the currentFunctionElement.
   ClosureClassMap closureData;
 
   bool insideClosure = false;
 
   ClosureTranslator(this.compiler, this.closedWorldRefiner, this.elements,
-      this.closureMappingCache, this.closureInfo);
+      this.memberMappingCache, this.nodeMappingCache, this.closureInfo);
 
   DiagnosticReporter get reporter => compiler.reporter;
 
   RuntimeTypesNeed get rtiNeed => closedWorldRefiner.closedWorld.rtiNeed;
 
   /// Generate a unique name for the [id]th closure field, with proposed name
   /// [name].
   ///
   /// The result is used as the name of [ClosureFieldElement]s, and must
   /// therefore be unique to avoid breaking an invariant in the element model
@@ skipping 59 matching lines @@
     visitInvokable(element, node, () {
       visit(initializer);
     });
     updateClosures();
   }
 
   // This function runs through all of the existing closures and updates their
   // free variables to the boxed value. It also adds the field-elements to the
   // class representing the closure.
   void updateClosures() {
-    for (LocalFunctionElement closure in closures) {
+    for (FunctionExpression closure in closures) {
       // The captured variables that need to be stored in a field of the closure
       // class.
       Set<Local> fieldCaptures = new Set<Local>();
       Set<BoxLocal> boxes = new Set<BoxLocal>();
-      ClosureClassMap data = closureMappingCache[closure];
+      ClosureClassMap data = nodeMappingCache[closure];
       // We get a copy of the keys and iterate over it, to avoid modifications
       // to the map while iterating over it.
       Iterable<Local> freeVariables = data.freeVariables.toList();
       freeVariables.forEach((Local fromElement) {
         assert(data.isFreeVariable(fromElement));
         assert(data.freeVariableMap[fromElement] == null);
         assert(isCapturedVariable(fromElement));
         BoxFieldElement boxFieldElement =
             getCapturedVariableBoxField(fromElement);
         if (boxFieldElement == null) {
@@ skipping 423 matching lines @@
     bool oldInsideClosure = insideClosure;
     Element oldFunctionElement = executableContext;
     ClosureClassMap oldClosureData = closureData;
 
     insideClosure = outermostElement != null;
     LocalFunctionElement closure;
     executableContext = element;
     bool needsRti = false;
     if (insideClosure) {
       closure = element;
-      closures.add(closure);
-      closureData = globalizeClosure(node, closure);
+      closures.add(node);
+      nodeMappingCache[node] = closureData = globalizeClosure(node, closure);
       needsRti = compiler.options.enableTypeAssertions ||
           rtiNeed.localFunctionNeedsRti(closure);
     } else {
       outermostElement = element;
       ThisLocal thisElement = null;
       if (element.isInstanceMember || element.isGenerativeConstructor) {
         MemberElement member = element;
         thisElement = new ThisLocal(member);
       }
       closureData = new ClosureClassMap(null, null, null, thisElement);
+      memberMappingCache[element] = closureData;
+      memberMappingCache[element.declaration] = closureData;
       if (element is MethodElement) {
         needsRti = compiler.options.enableTypeAssertions ||
             rtiNeed.methodNeedsRti(element);
       }
     }
-    closureMappingCache[element] = closureData;
-    closureMappingCache[element.declaration] = closureData;
     if (closureData.callMethod != null) {
-      closureMappingCache[closureData.callMethod] = closureData;
+      memberMappingCache[closureData.callMethod] = closureData;
     }
 
     inNewScope(node, () {
       // If the method needs RTI, or checked mode is set, we need to
       // escape the potential type variables used in that closure.
       if (needsRti) {
         analyzeTypeVariables(element.type);
       }
 
       visitChildren();
@@ skipping 99 matching lines @@
 ///
 /// Move the below classes to a JS model eventually.
 ///
 abstract class JSEntity implements MemberEntity {
   Local get declaredEntity;
 }
 
 abstract class PrivatelyNamedJSEntity implements JSEntity {
   Entity get rootOfScope;
 }