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;
+
+  // For function parameters, we need to distinquish the following states:
+  //
+  // - captured inside initializers, not used in body (INSIDE_INITIALIZER)
+  // - only used in body (OUTSIDE_INITIALIZER)
+  // - captured inside initializers and used in body (OUTSIDE_INITIALIZER |
+  //   INSIDE_INITIALIZER)
+  //

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

Please, remove the empty comment line here.

[sjindel, 2017/08/01 14:58:25]

Done.

+  final Map<VariableDeclaration, int> variables = <VariableDeclaration, int>{};

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

How about adding helper functions to hide the internals of the flags?  For
example, like this:

```
static bool isCapturedInInitializer(int flags) {
  return (flags & INSIDE_INITIALIZER) != 0;
}
static bool isUsedInBody(int flags) {
  return (flags & OUTSIDE_INITIALIZER) != 0;
}
static bool isCausingRedirection(int flags) {
  return flags == (INSIDE_INITIALIZER | OUTSIDE_INITIALIZER);
}
```

Or even creating a helper class [CapturedVariable] and an enum for flags?

```
enum CapturingFlag {
  capturedInInitializer,
  usedInBody,
}

class CapturedVariable {
  VariableDeclaration variable;
  int _flags;
  CapturedVariable(this.variable) : _flags = 0;
  void addFlag(CapturingFlag flag) {
    _flags |= (1 << flag.index);
  }
  // ... helper functions ...
}
```

[sjindel, 2017/08/01 14:58:25]

I would prefer the second approach over the first, but personally I feel that
"boilerplate" code like this should be kept to a minimum, especially when it
doesn't significantly simplify the places where the flag is used.

However, I'll change it if you strongly disagree.

[Dmitry Stefantsov, 2017/08/03 09:28:26]

I guess it depends on the definition of "significantly" :)  To me, manipulations
with flags always bring too many parens which makes it harder to read the
conditions on flags.

Feel free to leave the code as it is.

+
   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);
     });
     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] |= captureFlags;
     }
     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] |= 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;
     }
   }
 }