dart2js cps: Do not propagate expressions into foreign code.

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

Index: pkg/compiler/lib/src/tree_ir/tree_ir_builder.dart
diff --git a/pkg/compiler/lib/src/tree_ir/tree_ir_builder.dart b/pkg/compiler/lib/src/tree_ir/tree_ir_builder.dart
index 5eddb3784787bc3778b6537df7c6a1a7236e27f6..c42cab864cc1829465dbfedbb809631dae5de01c 100644
--- a/pkg/compiler/lib/src/tree_ir/tree_ir_builder.dart
+++ b/pkg/compiler/lib/src/tree_ir/tree_ir_builder.dart
@@ -8,6 +8,7 @@ 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';
 
@@ -406,11 +407,22 @@ class Builder implements cps_ir.Visitor/*<NodeCallback|Node>*/ {
   }
 
   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);
@@ -420,7 +432,7 @@ class Builder implements cps_ir.Visitor/*<NodeCallback|Node>*/ {
         return new ForeignStatement(
             node.codeTemplate,
             node.type,
-            node.arguments.map(getVariableUse).toList(growable: false),
+            arguments,
             node.nativeBehavior,
             node.dependency);
       };
@@ -704,3 +716,28 @@ class Builder implements cps_ir.Visitor/*<NodeCallback|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).

+    }
+  }
+}