Revision: 9903
Author: cromwell...@google.com
Date: Mon Mar 28 09:42:49 2011
Log: 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:
Review at http://gwt-code-reviews.appspot.com/1385803
Review by: sco...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=9903
Added:
/trunk/dev/core/javatests
/trunk/dev/core/test/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizerTest.java
Modified:
/trunk/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java
=======================================
--- /dev/null
+++ /trunk/dev/core/javatests Mon Mar 28 09:42:49 2011
@@ -0,0 +1,1 @@
+test
=======================================
--- /dev/null
+++
/trunk/dev/core/test/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizerTest.java
Mon Mar 28 09:42:49 2011
@@ -0,0 +1,63 @@
+/*
+ * Copyright 2008 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may
not
+ * use this file except in compliance with the License. You may obtain a
copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
under
+ * the License.
+ */
+package com.google.gwt.dev.jjs.impl;
+
+import com.google.gwt.core.ext.UnableToCompleteException;
+import com.google.gwt.dev.jjs.ast.JMethod;
+import com.google.gwt.dev.jjs.ast.JProgram;
+
+/**
+ * Tests {@link PostOptimizationCompoundAssignmentNormalizer}.
+ */
+public class PostOptimizationCompoundAssignmentNormalizerTest
+ extends OptimizerTestBase {
+
+ public void testIntegralFloatCoercion() throws Exception {
+ // long op= float
+ optimize("void", "long x=2L; float d=3; x += d;").into(
+ "long x=2L; float d=3; x = (long)((float)x + d);");
+ // long op= long
+ optimize("void", "long x=2L; long d=3L; x += d;").into(
+ "long x=2L; long d=3L; x = x + d;");
+ // don't touch int op= int
+ optimize("void", "int x=2; int d=3; x += d;").into(
+ "int x=2; int d=3; x += d;");
+ // don't touch, integral types with lhs wider than rhs
+ optimize("void", "int x=2; short d=3; x += d;").into(
+ "int x=2; short d=3; x += d;");
+ // different integral types, but should narrow result
+ optimize("void", "int x=2; short d=3; d += x;").into(
+ "int x=2; short d=3; d = (short)(d + x);");
+ // integral with long, should break up
+ optimize("void", "int x=2; long d=3L; x += d;").into(
+ "int x=2; long d=3L; x = (int)((long)x + d);");
+ // integral with float
+ optimize("void", "int x=2; float d=3.0f; x += d;").into(
+ "int x=2; float d=3.0f; x = (int)(x + d);");
+ // integral with double
+ optimize("void", "int x=2; double d=3.0; x += d;").into(
+ "int x=2; double d=3.0; x = (int)(x + d);");
+ // float and double, don't touch
+ optimize("void", "float x=2; double d=3.0; x += d;").into(
+ "float x=2; double d=3.0; x += d;");
+ }
+
+ protected boolean optimizeMethod(JProgram program, JMethod method) {
+ PostOptimizationCompoundAssignmentNormalizer.exec(program);
+ LongCastNormalizer.exec(program);
+ return true;
+ }
+}
=======================================
---
/trunk/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
Fri Mar 25 12:11:48 2011
+++
/trunk/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
Mon Mar 28 09:42:49 2011
@@ -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);
=======================================
---
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java
Fri May 28 06:43:34 2010
+++
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java
Mon Mar 28 09:42:49 2011
@@ -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);
=======================================
---
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java
Fri May 28 06:43:34 2010
+++
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java
Mon Mar 28 09:42:49 2011
@@ -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
@@ -34,6 +37,21 @@
protected PostOptimizationCompoundAssignmentNormalizer() {
}
+
+ @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) {
@@ -45,6 +63,19 @@
&& x.getType() != JPrimitiveType.DOUBLE) {
return true;
}
+
+ JType lhsType = x.getLhs().getType();
+ JType rhsType = x.getRhs().getType();
+
+ // don't bother with float op= double since we don't float == double
in JS
+ if (lhsType == JPrimitiveType.FLOAT && rhsType ==
JPrimitiveType.DOUBLE) {
+ return false;
+ }
+ // break up so that result may be coerced to LHS type
+ if (lhsType instanceof JPrimitiveType && rhsType instanceof
JPrimitiveType
+ && widenType(lhsType, rhsType) != lhsType) {
+ return true;
+ }
return false;
}
@@ -63,4 +94,22 @@
}
return false;
}
-}
+
+ /**
+ * Implements
http://java.sun.com/docs/books/jls/third_edition/html/conversions.html#26917
+ */
+ 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
