Remove unnecessary contexts in closure conversion

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,
+        FieldInitializer,
         FunctionDeclaration,
         FunctionNode,
+        LocalInitializer,
         Member,
         Name,
         Procedure,
         ProcedureKind,
         PropertyGet,
+        RedirectingInitializer,
+        SuperInitializer,
         ThisExpression,
         TypeParameter,
         TypeParameterType,
         VariableDeclaration,
         VariableGet,
         VariableSet;
 
 import '../../visitor.dart' show RecursiveVisitor;
 
 class ClosureInfo extends RecursiveVisitor {
@@ skipping 64 matching lines @@
     currentMemberFunction = null;
   }
 
   visitClass(Class node) {
     currentClass = node;
     super.visitClass(node);
     currentClass = null;
   }
 
   visitConstructor(Constructor node) {
+    /// [currentFunction] should be set to [currentMemberFunction] before
+    /// visiting the [FunctionNode] of the constructor, because initializers may
+    /// use constructor parameters and it shouldn't be treated as capturing
+    /// them.  Consider the following code:
+    ///
+    ///     class A {
+    ///       int x;
+    ///       A(int x)  /* [x] is visible in initializers and body. */
+    ///         : this.x = x {  /* Initializer. */
+    ///         /* Constructor body. */
+    ///       }
+    ///     }
+    ///
+    /// Here the parameter shouldn't be captured into a context in the
+    /// initializer.  However, [currentFunction] is `null` if not set, and
+    /// `function[node.variable]` in this case points to the [FunctionNode] of
+    /// the constructor (which is not `null`).  It leads to `x` being treated as
+    /// captured, because it's seen as used outside of the function where it is
+    /// declared.  In turn, it leads to unnecessary context creation and usage.
     beginMember(node, node.function);
-    super.visitConstructor(node);
+    saveCurrentFunction(() {
+      currentFunction = currentMemberFunction;
+      super.visitConstructor(node);
+    });
     endMember();
   }
 
   visitProcedure(Procedure node) {
     beginMember(node, node.function);
     if (node.isInstanceMember && node.kind == ProcedureKind.Method) {
       // Ignore the `length` method of [File] subclasses for now, as they
       // will force us to rename the `length` getter (kernel issue #43).
       // TODO(ahe): remove this condition.
       Class parent = node.parent;
@@ skipping 36 matching lines @@
   }
 
   visitFunctionDeclaration(FunctionDeclaration node) {
     assert(!localNames.containsKey(node));
     localNames[node.function] = computeUniqueLocalName(node.variable.name);
     return super.visitFunctionDeclaration(node);
   }
 
   visitFunctionNode(FunctionNode node) {
     localNames.putIfAbsent(node, computeUniqueLocalName);
-    var saved = currentFunction;
-    currentFunction = node;
-    node.visitChildren(this);
-    currentFunction = saved;
+
+    saveCurrentFunction(() {
+      currentFunction = node;
+      node.visitChildren(this);
+    });
+
     Set<TypeParameter> capturedTypeVariables = typeVariables[node];
     if (capturedTypeVariables != null && !isOuterMostContext) {
       // Propagate captured type variables to enclosing function.
       typeVariables
           .putIfAbsent(currentFunction, () => new Set<TypeParameter>())
           .addAll(capturedTypeVariables);
     }
   }
 
   visitVariableDeclaration(VariableDeclaration node) {
@@ skipping 27 matching lines @@
     if (!isOuterMostContext) {
       thisAccess.putIfAbsent(
           currentMemberFunction, () => new VariableDeclaration("#self"));
     }
   }
 
   visitPropertyGet(PropertyGet node) {
     invokedGetters.add(node.name);
     super.visitPropertyGet(node);
   }
+
+  saveCurrentFunction(void f()) {
+    var saved = currentFunction;
+    try {
+      f();
+    } finally {
+      currentFunction = saved;
+    }
+  }
 }