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) {
+    // For constructors we need to set [currentFunction] 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. */

[Dmitry Stefantsov, 2017/06/16 08:36:27]

I think I missed a spot here.  "x" should have been _x_ in the comment.  But,
probably, it's better to use `x` here and below to describe parts of the
example.

[karlklose, 2017/06/16 08:43:40]

Consider using '///' and '[x]' here and below (although you are technically in a
level two comment)

[Dmitry Stefantsov, 2017/06/16 08:55:28]

Done.

+    //         : this.x = x {  /* Initializer. */
+    //         /* Constructor body. */
+    //       }
+    //     }
+    //
+    // Here _x_ parameter shouldn't be captured into a context in the

[karlklose, 2017/06/16 08:43:41]

"_x_ parameter" -> "the parameter"?

[Dmitry Stefantsov, 2017/06/16 08:55:27]

Done.

+    // initializer.  However, if [currentFunction] is not set, it has _null_

[karlklose, 2017/06/16 08:43:41]

How about 'However, [currentFunction] is `null` if not set, ...'

[Dmitry Stefantsov, 2017/06/16 08:55:28]

Agree, that reads much better.

+    // value, and _function[node.variable]_ in this case points to the
+    // [FunctionNode] of the constructor (which is not _null_).  It leads to "x"

[karlklose, 2017/06/16 08:43:41]

`null`

[Dmitry Stefantsov, 2017/06/16 08:55:28]

Done.

+    // 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;
+    }
+  }
 }