Remove unnecessary contexts in closure conversion

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 2d9c2ca3c90f6f38ea3b78201741b63a3eaa7c94..7b9bd32454da6cd87d71f4ef96753839367e0fb4 100644
--- a/pkg/kernel/lib/transformations/closure/info.dart
+++ b/pkg/kernel/lib/transformations/closure/info.dart
@@ -9,13 +9,17 @@ import '../../ast.dart'
         Class,
         Constructor,
         Field,
+        FieldInitializer,
         FunctionDeclaration,
         FunctionNode,
+        LocalInitializer,
         Member,
         Name,
         Procedure,
         ProcedureKind,
         PropertyGet,
+        RedirectingInitializer,
+        SuperInitializer,
         ThisExpression,
         TypeParameter,
         TypeParameterType,
@@ -100,8 +104,31 @@ class ClosureInfo extends RecursiveVisitor {
   }
 
   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();
   }
 
@@ -158,10 +185,12 @@ class ClosureInfo extends RecursiveVisitor {
 
   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.
@@ -209,4 +238,13 @@ class ClosureInfo extends RecursiveVisitor {
     invokedGetters.add(node.name);
     super.visitPropertyGet(node);
   }
+
+  saveCurrentFunction(void f()) {
+    var saved = currentFunction;
+    try {
+      f();
+    } finally {
+      currentFunction = saved;
+    }
+  }
 }