Separate out ClosureAnalysisInfo to separate interfaces between what

pkg/compiler/lib/src/closure.dart

Index: pkg/compiler/lib/src/closure.dart
diff --git a/pkg/compiler/lib/src/closure.dart b/pkg/compiler/lib/src/closure.dart
index 5611c40773068cd12b9d6c64556f793164f7c80d..fc7b749446fba73e93cdfa6a2cd380da419b0978 100644
--- a/pkg/compiler/lib/src/closure.dart
+++ b/pkg/compiler/lib/src/closure.dart
@@ -25,12 +25,40 @@ import 'tree/tree.dart';
 import 'util/util.dart';
 import 'world.dart' show ClosedWorldRefiner;
 
-abstract class ClosureClassMaps {
+/// Where T is ir.Node or Node.
+abstract class ClosureClassMaps<T> {
   ClosureClassMap getMemberMap(MemberEntity member);
   ClosureClassMap getLocalFunctionMap(Local localFunction);
+
+  /// Accessor to the information about closures that the SSA builder will use.
+  ClosureAnalysisInfo getClosureAnalysisInfo(T node);
+}
+
+/// Class that provides a black-box interface to information gleaned from
+/// analyzing a closure's characteristics, most commonly used to influence how
+/// code should be generated in SSA stage.
+class ClosureAnalysisInfo {

[Siggi Cherem (dart-lang), 2017/06/14 22:21:04]

nit: just realized that we commonly use "*Data" as opposed to "*Info" in other
places in the compiler. Maybe consider switching to use "data" here as well?

+  const ClosureAnalysisInfo();
+
+  /// If true, this closure accesses a variable that was defined in an outside
+  /// scope and this variable gets modified at some point (sometimes we say that
+  /// variable has been "captured"). In this situation, access to this variable
+  /// is controlled via a wrapper (box) so that updates to this variable
+  /// are done in a way that is in line with Dart's closure rules.
+  bool requiresBox() => false;

[Siggi Cherem (dart-lang), 2017/06/14 22:21:04]

nit: I'd turn this into a getter instead: `bool get requiresBox`

[Emily Fortuna, 2017/06/14 23:58:23]

See followup CL: https://codereview.chromium.org/2936323003

+
+  /// Accessor to the local environment in which a particular closure node is
+  /// executed. This will encapsulate the value of any variables that have been
+  /// scoped into this context from outside.
+  Local get context => null;

[Johnni Winther, 2017/06/09 10:12:40]

Mention that this is the aforementioned 'box'. Maybe rename to align [context]
and [requiresBox].

[Emily Fortuna, 2017/06/09 20:27:29]

good point. updated.

+
+  /// True if the specified variable has been mutated inside the scope of this
+  /// closure.
+  bool isCaptured(Local variable) => false;
 }
 
-class ClosureTask extends CompilerTask implements ClosureClassMaps {
+class ClosureTask extends CompilerTask implements ClosureClassMaps<Node> {
+  Map<Node, ClosureScope> _closureInfoMap = <Node, ClosureScope>{};
   Map<Element, ClosureClassMap> _closureMappingCache =
       <Element, ClosureClassMap>{};
   Compiler compiler;
@@ -42,6 +70,11 @@ class ClosureTask extends CompilerTask implements ClosureClassMaps {
 
   DiagnosticReporter get reporter => compiler.reporter;
 
+  ClosureAnalysisInfo getClosureAnalysisInfo(Node node) {
+    var value = _closureInfoMap[node];
+    return value == null ? const ClosureAnalysisInfo() : value;
+  }
+
   ClosureClassMap getMemberMap(MemberElement member) {
     return getClosureToClassMapping(member);
   }
@@ -94,7 +127,11 @@ class ClosureTask extends CompilerTask implements ClosureClassMaps {
         TreeElements elements = element.resolvedAst.elements;
 
         ClosureTranslator translator = new ClosureTranslator(
-            compiler, closedWorldRefiner, elements, _closureMappingCache);
+            compiler,
+            closedWorldRefiner,
+            elements,
+            _closureMappingCache,
+            _closureInfoMap);
 
         // The translator will store the computed closure-mappings inside the
         // cache. One for given node and one for each nested closure.
@@ -407,7 +444,7 @@ class SynthesizedCallMethodElementX extends BaseFunctionElementX
 
 // The box-element for a scope, and the captured variables that need to be
 // stored in the box.
-class ClosureScope {
+class ClosureScope implements ClosureAnalysisInfo {
   final BoxLocal boxElement;
   final Map<Local, BoxFieldElement> capturedVariables;
 
@@ -418,9 +455,15 @@ class ClosureScope {
 
   ClosureScope(this.boxElement, this.capturedVariables);
 
+  Local get context => boxElement;
+
+  bool requiresBox() => capturedVariables.keys.isNotEmpty;
+
+  List<Local> get boxedVariables => boxedLoopVariables;
+
   bool hasBoxedLoopVariables() => !boxedLoopVariables.isEmpty;
 
-  bool isCapturedVariable(Local variable) {
+  bool isCaptured(Local variable) {
     return capturedVariables.containsKey(variable);
   }
 
@@ -559,6 +602,7 @@ class ClosureTranslator extends Visitor {
   bool inTryStatement = false;
 
   final Map<Element, ClosureClassMap> closureMappingCache;
+  final Map<Node, ClosureScope> closureInfo;
 
   // Map of captured variables. Initially they will map to `null`. If
   // a variable needs to be boxed then the scope declaring the variable
@@ -585,7 +629,7 @@ class ClosureTranslator extends Visitor {
   bool insideClosure = false;
 
   ClosureTranslator(this.compiler, this.closedWorldRefiner, this.elements,
-      this.closureMappingCache);
+      this.closureMappingCache, this.closureInfo);
 
   DiagnosticReporter get reporter => compiler.reporter;
 
@@ -970,6 +1014,8 @@ class ClosureTranslator extends Visitor {
     if (!scopeMapping.isEmpty) {
       ClosureScope scope = new ClosureScope(box, scopeMapping);
       closureData.capturingScopes[node] = scope;
+      assert(closureInfo[node] == null);
+      closureInfo[node] = scope;
     }
   }