Don't allow DataflowOptimizer to perform constant transformation on expressions with side-effects.
Review at http://gwt-code-reviews.appspot.com/1467801
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10360 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/gflow/constants/ConstantsTransformationFunction.java b/dev/core/src/com/google/gwt/dev/jjs/impl/gflow/constants/ConstantsTransformationFunction.java
index 812c143..8fb4616 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/gflow/constants/ConstantsTransformationFunction.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/gflow/constants/ConstantsTransformationFunction.java
@@ -56,6 +56,11 @@
Preconditions.checkNotNull(condition,
"Null condition in %s: %s", node, node.getJNode());
+
+ if (condition.hasSideEffects()) {
+ return;
+ }
+
JValueLiteral evaluatedCondition =
ExpressionEvaluator.evaluate(condition, assumption);
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/DataflowOptimizerTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/DataflowOptimizerTest.java
index 060bc4e..60a1a00 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/DataflowOptimizerTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/gflow/DataflowOptimizerTest.java
@@ -2,6 +2,8 @@
import com.google.gwt.dev.jjs.ast.JMethod;
import com.google.gwt.dev.jjs.ast.JProgram;
+import com.google.gwt.dev.jjs.impl.DeadCodeElimination;
+import com.google.gwt.dev.jjs.impl.MethodInliner;
import com.google.gwt.dev.jjs.impl.OptimizerTestBase;
public class DataflowOptimizerTest extends OptimizerTestBase {
@@ -9,6 +11,10 @@
protected void setUp() throws Exception {
super.setUp();
+ /*
+ * TODO: Each of these snippets shouldn't be setup for every test, and thus should be moved
+ * to the individual test cases they are needed for (or to shared methods if needed).
+ */
addSnippetClassDecl("static void foo(int i) { }");
addSnippetClassDecl("static boolean bar() { return true; }");
addSnippetClassDecl("static void baz(boolean b) { }");
@@ -29,6 +35,9 @@
addSnippetClassDecl("static class Foo { int i; int j; int k; }");
addSnippetClassDecl("static Foo createFoo() {return null;}");
addSnippetClassDecl("static Foo staticFooInstance;");
+
+ runMethodInliner = false;
+ runDCE = false;
}
public void testLinearStatements1() throws Exception {
@@ -305,9 +314,60 @@
" long lng8 = lng << 8;",
" return lng8;");
}
+
+ /*
+ * This test is a regression for an issue where inlined multiexpressions were getting removed
+ * by the ConstantsTransformationFunction, based on the constant value of the multi-expression,
+ * despite there being side-effects of the multi-expression. So, we want to test that inlining
+ * proceeds, but not further constant transformation.
+ *
+ * TODO: This test may need to evolve over time, as the specifics of the optimizers change. One
+ * obvious todo is to allow some form of constant transformation to occur with inlined
+ * multi-expressions (see comment below).
+ */
+ public void testInlinedConstantExpressionWithSideEffects() throws Exception {
+
+ runDCE = true;
+ runMethodInliner = true;
+
+ addSnippetClassDecl("static void fail() {" +
+ " throw new RuntimeException();" +
+ "}");
+ addSnippetClassDecl("static Integer x;");
+ addSnippetClassDecl("static boolean copy(Number n) {" +
+ " x = (Integer) n;" +
+ " return true;" +
+ "}");
+
+ optimize("int",
+ "Integer n = new Integer(1);",
+ "if (!copy(n)) {",
+ " fail();",
+ "}",
+ "return x;")
+ // TODO: Allow the second line below to be transformed to just: "EntryPoint.x = n;"
+ .intoString("Integer n = new Integer(1);",
+ "((EntryPoint.x = n, true)) || EntryPoint.fail();",
+ "return EntryPoint.x.intValue();");
+
+ }
+
+ private boolean runDCE;
+ private boolean runMethodInliner;
@Override
protected boolean optimizeMethod(JProgram program, JMethod method) {
- return DataflowOptimizer.exec(program, method).didChange();
+ boolean didChange = false;
+
+ if (runDCE) {
+ didChange = DeadCodeElimination.exec(program).didChange() || didChange;
+ }
+
+ if (runMethodInliner) {
+ didChange = MethodInliner.exec(program).didChange() || didChange;
+ }
+
+ didChange = DataflowOptimizer.exec(program, method).didChange() || didChange;
+ return didChange;
}
}
