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..cc72c9095377c840fb82d0d3e140fdbf1565d9eb 100644
--- a/pkg/kernel/lib/transformations/closure/info.dart
+++ b/pkg/kernel/lib/transformations/closure/info.dart
@@ -25,11 +25,24 @@ 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>>{};
@@ -107,8 +120,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);
     });
     endMember();
   }
@@ -181,14 +203,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] |= 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);
   }