Fix duplicate context creation when closures appear in initializers.

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

 // Copyright (c) 2016, 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.
 
 library kernel.transformations.closure.info;
 
 import '../../ast.dart'
     show
         Class,
         Constructor,
         Field,
         FunctionDeclaration,
         FunctionNode,
         Member,
         Procedure,
         ThisExpression,
         TypeParameter,
         TypeParameterType,
         VariableDeclaration,
         VariableGet,
         VariableSet,
         visitList;
 
 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>>{};
 
   /// Map from members to synthetic variables for accessing `this` in a local
   /// function.
   final Map<FunctionNode, VariableDeclaration> thisAccess =
       <FunctionNode, VariableDeclaration>{};
 
   final Set<String> currentMemberLocalNames = new Set<String>();
@@ skipping 57 matching lines @@
     /// the parameters that may be used in the initializers.  If the nodes are
     /// visited in another order, the encountered parameters in initializers
     /// are treated as captured, because they are not yet associated with the
     /// function.
     beginMember(node, node.function);
     saveCurrentFunction(() {
       currentFunction = currentMemberFunction;
 
       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();
   }
 
   visitProcedure(Procedure node) {
     beginMember(node, node.function);
     super.visitProcedure(node);
     endMember();
   }
 
@@ skipping 52 matching lines @@
     }
   }
 
   visitVariableDeclaration(VariableDeclaration node) {
     function[node] = currentFunction;
     node.visitChildren(this);
   }
 
   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);
   }
 
   visitTypeParameterType(TypeParameterType node) {
     if (!isOuterMostContext && node.parameter.parent != currentFunction) {
       typeVariables
           .putIfAbsent(currentFunction, () => new Set<TypeParameter>())
           .add(node.parameter);
     }
   }
 
   visitThisExpression(ThisExpression node) {
     if (!isOuterMostContext) {
       thisAccess.putIfAbsent(
           currentMemberFunction, () => new VariableDeclaration("#self"));
     }
   }
 
   saveCurrentFunction(void f()) {
     var saved = currentFunction;
     try {
       f();
     } finally {
       currentFunction = saved;
     }
   }
 }