dart2js cps: Do not propagate expressions into foreign code.

pkg/compiler/lib/src/tree_ir/optimization/statement_rewriter.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.optimization.statement_rewriter;
 
 import 'optimization.dart' show Pass;
 import '../tree_ir_nodes.dart';
 import '../../io/source_information.dart';
 import '../../elements/elements.dart';
@@ skipping 260 matching lines @@
   @override
   Expression visitVariableUse(VariableUse node) {
     // Count of number of unseen uses remaining.
     unseenUses.putIfAbsent(node.variable, () => node.variable.readCount);
     --unseenUses[node.variable];
 
     // We traverse the tree right-to-left, so when we have seen all uses,
     // it means we are looking at the first use.
     assert(unseenUses[node.variable] < node.variable.readCount);
     assert(unseenUses[node.variable] >= 0);
+
+    // We cannot reliably find the first dynamic use of a variable that is
+    // accessed from a JS function in a foreign code fragment.
+    if (node.variable.isCaptured) return node;
+
     bool isFirstUse = unseenUses[node.variable] == 0;
 
     // Propagate constant to use site.
     Expression constant = constantEnvironment[node.variable];
     if (constant != null && !hasUnsafeVariableUse(constant)) {
       --node.variable.readCount;
       return visitExpression(constant);
     }
 
     // Try to propagate another expression into this variable use.
@@ skipping 841 matching lines @@
   }
 
   Expression makeCondition(Expression e, bool polarity) {
     return polarity ? e : new Not(e);
   }
 
   Statement getBranch(If node, bool polarity) {
     return polarity ? node.thenStatement : node.elseStatement;
   }
 
+  void handleForeignCode(ForeignCode node) {
+    // Arguments will get inserted in a JS code template.  The arguments will
+    // not always be evaluated (e.g. if the template is '# && #').
+    // TODO(asgerf): We could analyze the JS AST to see if arguments are
+    //               definitely evaluated left-to-right.

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

We need to do this. The cost on code quality of not doing so is too high (adds
nearly 6k to swarm).

      get$length: function(receiver) {
        return this._html$_element.classList.length;
      },
-->
      get$length: function(receiver) {
        var v0 = this._html$_element;
        v0 = v0.classList;
        return v0.length;
      },

What would be the most convenient form of the information?

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

I think we can encode the whole thing with an integer N, meaning the first N
arguments will definitely be evaluated exactly once, in left-to-right order,
without intermediate side-effects or heap dependencies, before any of the
remaining arguments are evaluated.

So for example, if the template is "foo(#,#) && bar(#,#)", N=2 because the first
two arguments are always evaluated.

The requirement about side effects and heap dependencies might be a problem. For
example, if the template was "#.length > #", and the second argument is x.pop(),
we must evaluate the pop() before getting the length, so it can't propagate. N=1
is the only safe answer there.

We could assume that the side-effect spec string of the whole template also
summarizes the side-effects in-between the arguments. So if the entire
expression has no side effects/dependencies, we can assume there are also no
side effects/dependencies separating the arguments. It won't change N=1 in the
previous example, but it saves us from doing an essentially impossible
side-effect analysis on the JS AST.

+    inEmptyEnvironment(() {
+      _rewriteList(node.arguments);
+    });
+  }
+
   @override
   Expression visitForeignExpression(ForeignExpression node) {
-    _rewriteList(node.arguments);
+    handleForeignCode(node);
     return node;
   }
 
   @override
   Statement visitForeignStatement(ForeignStatement node) {
-    _rewriteList(node.arguments);
+    handleForeignCode(node);
     return node;
   }
 
   @override
   Expression visitAwait(Await node) {
     node.input = visitExpression(node.input);
     return node;
   }
 
   @override
@@ skipping 92 matching lines @@
   VariableUseVisitor(this.callback);
 
   visitVariableUse(VariableUse use) => callback(use);
 
   visitInnerFunction(FunctionDefinition node) {}
 
   static void visit(Expression node, VariableUseCallback callback) {
     new VariableUseVisitor(callback).visitExpression(node);
   }
 }