Fix duplicate context creation when closures appear in initializers.

pkg/kernel/lib/transformations/closure/info.dart

Index: pkg/kernel/lib/transformations/closure/info.dart
diff --git a/pkg/kernel/lib/transformations/closure/info.dart b/pkg/kernel/lib/transformations/closure/info.dart
index dd867f45f1d9d608b32efdb58bade9571a754a2b..0f66b5aad1c6919fa940b4e9b164281dcf02f906 100644
--- a/pkg/kernel/lib/transformations/closure/info.dart
+++ b/pkg/kernel/lib/transformations/closure/info.dart
@@ -25,11 +25,18 @@ import '../../visitor.dart' show RecursiveVisitor;
 
 class ClosureInfo extends RecursiveVisitor {
   FunctionNode currentFunction;
+
+  static const int OUTSIDE_INITIALIZER = 1;
+  static const int INSIDE_INITIALIZER = 2;
+
+  int captureFlags = OUTSIDE_INITIALIZER;
+
+  final Map<VariableDeclaration, int /*captured flags*/ > variables =

[Dmitry Stefantsov, 2017/07/31 15:05:35]

I think a more elaborate comment about the flags, how they are set, and how they
are used later is better than the in-line comment.

[sjindel, 2017/07/31 15:32:16]

Done.

+      <VariableDeclaration, int>{};
+
   final Map<VariableDeclaration, FunctionNode> function =
       <VariableDeclaration, FunctionNode>{};
 
-  final Set<VariableDeclaration> variables = new Set<VariableDeclaration>();
-
   /// Map from functions to set of type variables captured within them.
   final Map<FunctionNode, Set<TypeParameter>> typeVariables =
       <FunctionNode, Set<TypeParameter>>{};
@@ -107,8 +114,17 @@ class ClosureInfo extends RecursiveVisitor {
 
       visitList(node.annotations, this);
       node.name?.accept(this);
-      node.function?.accept(this);
+
+      visitList(node.function.typeParameters, this);
+      visitList(node.function.positionalParameters, this);
+      visitList(node.function.namedParameters, this);
+
+      assert(captureFlags == OUTSIDE_INITIALIZER);
+      captureFlags = INSIDE_INITIALIZER;
       visitList(node.initializers, this);
+      captureFlags = OUTSIDE_INITIALIZER;
+
+      node.function.accept(this);

[Dmitry Stefantsov, 2017/07/31 15:05:35]

To this point, we've already visited [typeParameters], [positionalParameters],
and [namedParameters].  Now we may just visit [returnType] and [body].

[sjindel, 2017/07/31 15:32:16]

Yes, but I wrote it this way so if a new field is added to FunctionNode and
FunctionNode::visitChildren, we won't ignore it.

[Dmitry Stefantsov, 2017/08/01 09:36:13]

I guess it's ok to leave it like this, but consider all those AST visitors that
need specific order of visiting the nodes.  They all should be updated when new
fields are added to the specially treated nodes.

     });
     endMember();
   }
@@ -181,14 +197,20 @@ class ClosureInfo extends RecursiveVisitor {
 
   visitVariableGet(VariableGet node) {
     if (function[node.variable] != currentFunction) {
-      variables.add(node.variable);
+      variables.putIfAbsent(node.variable, () => 0);
+    }
+    if (variables.containsKey(node.variable)) {
+      variables[node.variable] = variables[node.variable] | captureFlags;

[Dmitry Stefantsov, 2017/07/31 15:05:35]

How about using `|=` operation here and below?

[sjindel, 2017/07/31 15:32:16]

Done.

     }
     node.visitChildren(this);
   }
 
   visitVariableSet(VariableSet node) {
     if (function[node.variable] != currentFunction) {
-      variables.add(node.variable);
+      variables.putIfAbsent(node.variable, () => 0);
+    }
+    if (variables.containsKey(node.variable)) {
+      variables[node.variable] = variables[node.variable] | captureFlags;
     }
     node.visitChildren(this);
   }