Reviewers: scottb,
Description:
Operations like i += d where i is an int and d is a double are not
properly
truncated (narrowed) to the LHS type. This patch forces i += d to be
written as
i = i + d, and applies a narrowing cast as needed, e.g. i = (int)(i +
d);
Updated to fix String concat issue:
Please review this at http://gwt-code-reviews.appspot.com/1385803/
Affected files:
M dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
M
dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java
M
dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java
Index: dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
===================================================================
--- dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
(revision 9856)
+++ dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
(working copy)
@@ -292,10 +292,10 @@
// (5) "Normalize" the high-level Java tree into a lower-level tree
more
// suited for JavaScript code generation. Don't go reordering these
// willy-nilly because there are some subtle interdependencies.
- LongCastNormalizer.exec(jprogram);
JsoDevirtualizer.exec(jprogram);
CatchBlockNormalizer.exec(jprogram);
PostOptimizationCompoundAssignmentNormalizer.exec(jprogram);
+ LongCastNormalizer.exec(jprogram);
LongEmulationNormalizer.exec(jprogram);
CastNormalizer.exec(jprogram, options.isCastCheckingDisabled());
ArrayNormalizer.exec(jprogram);
Index:
dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java
===================================================================
---
dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java
(revision 9856)
+++
dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java
(working copy)
@@ -161,8 +161,10 @@
new JMultiExpression(x.getSourceInfo()));
JExpression newLhs = replacer.accept(x.getLhs());
- JBinaryOperation operation = new JBinaryOperation(x.getSourceInfo(),
+ JExpression operation = new JBinaryOperation(x.getSourceInfo(),
newLhs.getType(), op.getNonAssignmentOf(), newLhs, x.getRhs());
+ operation = modifyResultOperation((JBinaryOperation) operation);
+
// newLhs is cloned below because it was used in operation
JBinaryOperation asg = new JBinaryOperation(x.getSourceInfo(),
newLhs.getType(),
JBinaryOperator.ASG, cloner.cloneExpression(newLhs),
@@ -285,6 +287,16 @@
return lhs;
}
+ /**
+ * Decide what expression to return when breaking up a compound
assignment of
+ * the form <code>lhs op= rhs</code>. The breakup creates an expression
of
+ * the form <code>lhs = lhs op rhs</code>, and the right hand side of the
+ * newly created expression is passed to this method.
+ */
+ protected JExpression modifyResultOperation(JBinaryOperation op) {
+ return op;
+ }
+
protected abstract boolean shouldBreakUp(JBinaryOperation x);
protected abstract boolean shouldBreakUp(JPostfixOperation x);
Index:
dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java
===================================================================
---
dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java
(revision 9856)
+++
dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java
(working copy)
@@ -17,10 +17,13 @@
import com.google.gwt.dev.jjs.ast.JBinaryOperation;
import com.google.gwt.dev.jjs.ast.JBinaryOperator;
+import com.google.gwt.dev.jjs.ast.JCastOperation;
+import com.google.gwt.dev.jjs.ast.JExpression;
import com.google.gwt.dev.jjs.ast.JPostfixOperation;
import com.google.gwt.dev.jjs.ast.JPrefixOperation;
import com.google.gwt.dev.jjs.ast.JPrimitiveType;
import com.google.gwt.dev.jjs.ast.JProgram;
+import com.google.gwt.dev.jjs.ast.JType;
/**
* Normalize compound assignments as needed after optimization. Integer
division
@@ -36,6 +39,21 @@
}
@Override
+ protected JExpression modifyResultOperation(JBinaryOperation op) {
+ JType lhsType = op.getLhs().getType();
+ JType rhsType = op.getRhs().getType();
+ if (lhsType != rhsType) {
+ // first widen binary op to encompass both sides, then add narrow
cast
+ return new JCastOperation(op.getSourceInfo(), lhsType,
+ new JBinaryOperation(op.getSourceInfo(),
+ widenType(lhsType, rhsType),
+ op.getOp(),
+ op.getLhs(), op.getRhs()));
+ }
+ return op;
+ }
+
+ @Override
protected boolean shouldBreakUp(JBinaryOperation x) {
if (x.getType() == JPrimitiveType.LONG) {
return true;
@@ -43,6 +61,13 @@
if (x.getOp() == JBinaryOperator.ASG_DIV
&& x.getType() != JPrimitiveType.FLOAT
&& x.getType() != JPrimitiveType.DOUBLE) {
+ return true;
+ }
+
+ JType lhsType = x.getLhs().getType();
+ JType rhsType = x.getRhs().getType();
+ // break up so that result may be coerced to LHS type
+ if (isIntegral(lhsType) && !isIntegral(rhsType)) {
return true;
}
return false;
@@ -63,4 +88,25 @@
}
return false;
}
+
+ private boolean isIntegral(JType type) {
+ // if it can't be widened into a long, it must be float
+ return type instanceof JPrimitiveType &&
+ widenType(type, JPrimitiveType.LONG) == JPrimitiveType.LONG;
+ }
+
+ private JType widenType(JType lhsType, JType rhsType) {
+ if (lhsType == JPrimitiveType.DOUBLE ||
+ rhsType == JPrimitiveType.DOUBLE) {
+ return JPrimitiveType.DOUBLE;
+ } else if (lhsType == JPrimitiveType.FLOAT ||
+ rhsType == JPrimitiveType.FLOAT) {
+ return JPrimitiveType.FLOAT;
+ } else if (lhsType == JPrimitiveType.LONG ||
+ rhsType == JPrimitiveType.LONG) {
+ return JPrimitiveType.LONG;
+ } else {
+ return JPrimitiveType.INT;
+ }
+ }
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
