dart2js cps: Do not propagate expressions into foreign code.

pkg/compiler/lib/src/tree_ir/tree_ir_builder.dart

 // Copyright (c) 2014, 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 tree_ir_builder;
 
 import '../common.dart';
 import '../constants/values.dart';
 import '../cps_ir/cps_ir_nodes.dart' as cps_ir;
 import '../elements/elements.dart';
+import 'package:js_ast/js_ast.dart' as js;
 
 import 'tree_ir_nodes.dart';
 
 typedef Statement NodeCallback(Statement next);
 
 /**
  * Builder translates from CPS-based IR to direct-style Tree.
  *
  * A call `Invoke(fun, cont, args)`, where cont is a singly-referenced
  * non-exit continuation `Cont(v, body)` is translated into a direct-style call
@@ skipping 378 matching lines @@
     Expression invoke = new InvokeConstructor(
         node.dartType,
         node.target,
         node.selector,
         arguments,
         node.sourceInformation);
     return makeCallExpression(node, invoke);
   }
 
   NodeCallback visitForeignCode(cps_ir.ForeignCode node) {
+    List<Expression> arguments =
+        node.arguments.map(getVariableUse).toList(growable: false);
+    if (HasCapturedArguments.check(node.codeTemplate.ast)) {
+      for (Expression arg in arguments) {
+        if (arg is VariableUse) {
+          arg.variable.isCaptured = true;
+        } else {
+          // TODO(asgerf): Avoid capture of 'this'.
+        }
+      }
+    }
     if (node.codeTemplate.isExpression) {
       Expression foreignCode = new ForeignExpression(
           node.codeTemplate,
           node.type,
-          node.arguments.map(getVariableUse).toList(growable: false),
+          arguments,
           node.nativeBehavior,
           node.dependency);
       return makeCallExpression(node, foreignCode);
     } else {
       return (Statement next) {
         assert(next is Unreachable); // We are not using the `next` statement.
         return new ForeignStatement(
             node.codeTemplate,
             node.type,
-            node.arguments.map(getVariableUse).toList(growable: false),
+            arguments,
             node.nativeBehavior,
             node.dependency);
       };
     }
   }
 
   NodeCallback visitGetLazyStatic(cps_ir.GetLazyStatic node) {
     // In the tree IR, GetStatic handles lazy fields because we do not need
     // as fine-grained control over side effects.
     GetStatic value = new GetStatic(node.element, node.sourceInformation);
@@ skipping 263 matching lines @@
     internalError(CURRENT_ELEMENT_SPANNABLE, 'Unexpected IR node: $node');
   }
 
   visitFunctionDefinition(cps_ir.FunctionDefinition node) {
     unexpectedNode(node);
   }
   visitParameter(cps_ir.Parameter node) => unexpectedNode(node);
   visitContinuation(cps_ir.Continuation node) => unexpectedNode(node);
   visitMutableVariable(cps_ir.MutableVariable node) => unexpectedNode(node);
 }
+
+class HasCapturedArguments extends js.BaseVisitor {
+  static bool check(js.Node node) {
+    HasCapturedArguments visitor = new HasCapturedArguments();
+    node.accept(visitor);
+    return visitor.found;
+  }
+
+  int enclosingFunctions = 0;
+  bool found = false;
+
+  @override
+  visitFun(js.Fun node) {
+    ++enclosingFunctions;
+    node.visitChildren(this);
+    --enclosingFunctions;
+  }
+
+  @override
+  visitInterpolatedNode(js.InterpolatedNode node) {
+    if (enclosingFunctions > 0) {
+      found = true;

[sra1, 2015/11/12 02:30:09]

The documentation for JS says 'never use `#` in a function body'.

There is no way to get a correct result if the capture happens in a loop, or if
the captured variable is a re-used temporary.

We should reject this code outright, and force the use of 'let' (a JS function
application) to capture the values.

I'll take a look at what is involved.

[asgerf, 2015/11/12 12:09:46]

FWIW, this change was made to fix the issue with re-used temporaries. The
register allocation will never merge captured variables with any other variables
(actually a leftover from dart2dart, where variable capture was a real thing).

+    }
+  }
+}