Have JsInliner refuse to interpose a new expression
before a substituted argument.

Review at http://gwt-code-reviews.appspot.com/344801

Review by: scottb@google.com

git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@7913 8db76d5a-ed1c-0410-87a9-c151d255dfc7

dev/core/src/com/google/gwt/dev/js/JsInliner.java

diff --git a/dev/core/src/com/google/gwt/dev/js/JsInliner.java b/dev/core/src/com/google/gwt/dev/js/JsInliner.java
index 2c15ef1..1cef6f4 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsInliner.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsInliner.java
@@ -673,13 +673,30 @@
         maintainsOrder = false;
       }
     }
-
+    
     @Override
     public void endVisit(JsNameRef x, JsContext<JsExpression> ctx) {
       checkName(x.getName());
     }
 
     @Override
+    public void endVisit(JsNew x, JsContext<JsExpression> ctx) {
+      /*
+       * Unless all arguments have already been evaluated, assume
+       * that invoking the new expression might interfere with
+       * the evaluation of the argument.
+       *
+       * It would be possible to allow this if the invoked function
+       * either does nothing or does nothing that affects the 
+       * remaining arguments.  However, currently there is no
+       * analysis of the invoked function.
+       */
+      if (unevaluated.size() > 0) {
+        maintainsOrder = false;
+      }
+    }
+
+    @Override
     public void endVisit(JsThisRef x, JsContext<JsExpression> ctx) {
       checkName(THIS_NAME);
     }

dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java

diff --git a/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java b/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java
index 4debe87..cb1f37b 100644
--- a/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java
+++ b/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java
@@ -80,6 +80,29 @@
     compare(expected, input);
   }
 
+  /**
+   * Test that a new expression breaks argument ordering.
+   */
+  public void testOrderingNew() throws Exception {
+    StringBuffer code = new StringBuffer();
+    // A static variable x
+    code.append("var x;");
+
+    // foo() uses x
+    code.append("function foo() { alert('x = ' + x); }");
+
+    // callee does "new foo" before evaluating its argument
+    code.append("function callee(arg) { new foo(); return arg; }");
+
+    // caller invokes callee with a multi that initializes x
+    code.append("function caller() { callee((x=1,2)); }");
+
+    // bootstrap the program
+    code.append("caller();");
+
+    compare(code.toString(), code.toString());
+  }
+
   public void testSelfRecursion() throws Exception {
     String input = "function a1() { return blah && b1() }"
         + "function b1() { return bar && a1()}" + "function c() { a1() } c()";
@@ -94,7 +117,6 @@
     input = optimize(input, JsSymbolResolver.class, FixStaticRefsVisitor.class,
         JsInliner.class, JsUnusedFunctionRemover.class);
     expected = optimize(expected);
-    System.err.println("Input vs ");
     assertEquals(expected, input);
   }
 }