Author: [email protected]
Date: Mon Mar 23 11:45:31 2009
New Revision: 5070
Modified:
trunk/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java
trunk/user/test/com/google/gwt/dev/jjs/test/JStaticEvalTest.java
Log:
Fixes a bug where Simplifier.conditional() could recurse back into
itself and cause an NPE. It should actually tolerate null as the
first argument.
Review by: scottb
Modified: trunk/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java
==============================================================================
--- trunk/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java
(original)
+++ trunk/dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java Mon Mar
23 11:45:31 2009
@@ -145,17 +145,15 @@
} else if (thenExpr instanceof JBooleanLiteral) {
if (((JBooleanLiteral) thenExpr).getValue()) {
// e.g. (cond ? true : else) -> cond || else
- JBinaryOperation binOp = new JBinaryOperation(program,
- original.getSourceInfo(), original.getType(),
JBinaryOperator.OR,
- condExpr, elseExpr);
+ JBinaryOperation binOp = new JBinaryOperation(program, sourceInfo,
+ type, JBinaryOperator.OR, condExpr, elseExpr);
return binOp;
} else {
// e.g. (cond ? false : else) -> !cond && else
JPrefixOperation notCondExpr = new JPrefixOperation(program,
condExpr.getSourceInfo(), JUnaryOperator.NOT, condExpr);
- JBinaryOperation binOp = new JBinaryOperation(program,
- original.getSourceInfo(), original.getType(),
JBinaryOperator.AND,
- notCondExpr, elseExpr);
+ JBinaryOperation binOp = new JBinaryOperation(program, sourceInfo,
+ type, JBinaryOperator.AND, notCondExpr, elseExpr);
return binOp;
}
} else if (elseExpr instanceof JBooleanLiteral) {
@@ -163,23 +161,21 @@
// e.g. (cond ? then : true) -> !cond || then
JPrefixOperation notCondExpr = new JPrefixOperation(program,
condExpr.getSourceInfo(), JUnaryOperator.NOT, condExpr);
- JBinaryOperation binOp = new JBinaryOperation(program,
- original.getSourceInfo(), original.getType(),
JBinaryOperator.OR,
- notCondExpr, thenExpr);
+ JBinaryOperation binOp = new JBinaryOperation(program, sourceInfo,
+ type, JBinaryOperator.OR, notCondExpr, thenExpr);
return binOp;
} else {
// e.g. (cond ? then : false) -> cond && then
- JBinaryOperation binOp = new JBinaryOperation(program,
- original.getSourceInfo(), original.getType(),
JBinaryOperator.AND,
- condExpr, thenExpr);
+ JBinaryOperation binOp = new JBinaryOperation(program, sourceInfo,
+ type, JBinaryOperator.AND, condExpr, thenExpr);
return binOp;
}
} else {
// e.g. (!cond ? then : else) -> (cond ? else : then)
JExpression unflipped = maybeUnflipBoolean(condExpr);
if (unflipped != null) {
- return new JConditional(program, original.getSourceInfo(),
- original.getType(), unflipped, elseExpr, thenExpr);
+ return new JConditional(program, sourceInfo, type, unflipped,
elseExpr,
+ thenExpr);
}
}
Modified: trunk/user/test/com/google/gwt/dev/jjs/test/JStaticEvalTest.java
==============================================================================
--- trunk/user/test/com/google/gwt/dev/jjs/test/JStaticEvalTest.java
(original)
+++ trunk/user/test/com/google/gwt/dev/jjs/test/JStaticEvalTest.java Mon
Mar 23 11:45:31 2009
@@ -36,6 +36,7 @@
}
private volatile double fieldDoubleFive = 5.0;
+ private volatile boolean fieldFalse = false;
private volatile float fieldFloatFive = 5.0F;
private volatile int[] fieldIntArray = new int[10];
private volatile int fieldIntFive = 5;
@@ -49,6 +50,31 @@
}
/**
+ * Tests simplifications on ternary conditional expressions.
+ */
+ public void testConditionalExpressions() {
+ assertEquals(1, returnTrue() ? 1 : 2);
+ assertEquals(2, returnFalse() ? 1 : 2);
+ assertEquals(true, fieldTrue ? true : fieldFalse);
+ assertEquals(false, fieldTrue ? false : fieldTrue);
+ assertEquals(true, fieldTrue ? fieldTrue : false);
+ assertEquals(false, fieldTrue ? fieldFalse : true);
+ assertEquals(2, !fieldTrue ? 1 : 2);
+ assertEquals(2, !fieldTrue ? 1 : 2);
+
+ /*
+ * This example causes Simplifier to recurse into itself, which in
previous
+ * versions could cause a NullPointerException. The sequence of
+ * simplifications is:
+ */
+ // (fieldFalse = false, !fieldFalse) ? 1 : 2 // inlining
+ // (fieldFalse = false, !fieldfalse ? 1 : 2) // move multi outward
+ // (fieldFalse = false, fieldFalse ? 2 : 1) // flip negative condition
+ int res = doSomethingAndReturnTrue() ? 1 : 2;
+ assertEquals(1, res);
+ }
+
+ /**
* Test "true == booleanField" and permutations, as well as "true ==
false"
* and permutations.
*/
@@ -276,6 +302,14 @@
if (fieldObject == null) {
fail();
}
+ }
+
+ /**
+ * This method will inline as a multi.
+ */
+ private boolean doSomethingAndReturnTrue() {
+ fieldTrue = true;
+ return !fieldFalse;
}
// All of these returnFoo() methods exist so that the
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
