Revision: 6991 Author: [email protected] Date: Wed Nov 18 09:31:51 2009 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);
Patch by: cromwellian Review by: spoon http://code.google.com/p/google-web-toolkit/source/detail?r=6991 Added: /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/test/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizerTest.java Wed Nov 18 09:31:51 2009 @@ -0,0 +1,96 @@ +/* + * 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.JProgram; + +/** + * Tests {...@link PostOptimizationCompoundAssignmentNormalizer}. + */ +public class PostOptimizationCompoundAssignmentNormalizerTest + extends OptimizerTestBase { + + private final class Result { + + private final String optimized; + + private final String returnType; + + private final String userCode; + + public Result(String returnType, String userCode, String optimized) { + this.returnType = returnType; + this.userCode = userCode; + this.optimized = optimized; + } + + public void into(String expected) throws UnableToCompleteException { + JProgram program = compileSnippet(returnType, expected); + expected = getMainMethodSource(program); + assertEquals(userCode, expected, optimized); + } + + /** + * Compare without compiling expected, needed when optimizations produce + * incorrect java code (e.g. "a" || "b" is incorrect in java). + */ + public void intoString(String expected) { + String actual = optimized; + assertTrue(actual.startsWith("{")); + assertTrue(actual.endsWith("}")); + actual = actual.substring(1, actual.length() - 2).trim(); + // Join lines in actual code and remove indentations + actual = actual.replaceAll(" +", " ").replaceAll("\n", ""); + assertEquals(userCode, expected, actual); + } + } + + 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 integral op= integral + optimize("void", "int x=2; int d=3; x += d;").into( + "int x=2; int d=3; x += d;"); + // don't touch, different integral types + optimize("void", "int x=2; short d=3; x += d;").into( + "int x=2; short d=3; x += d;"); + // integral with long, don't touch + optimize("void", "int x=2; long d=3L; x += d;").into( + "int x=2; long d=3L; x += (int)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;"); + } + + private Result optimize(final String returnType, final String codeSnippet) + throws UnableToCompleteException { + JProgram program = compileSnippet(returnType, codeSnippet); + PostOptimizationCompoundAssignmentNormalizer.exec(program); + LongCastNormalizer.exec(program); + return new Result(returnType, codeSnippet, getMainMethodSource(program)); + } +} ======================================= --- /trunk/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java Mon Nov 16 11:29:17 2009 +++ /trunk/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java Wed Nov 18 09:31:51 2009 @@ -245,10 +245,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 Thu Apr 9 08:41:34 2009 +++ /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java Wed Nov 18 09:31:51 2009 @@ -96,8 +96,11 @@ JExpression newLhs = replacer.accept(x.getLhs()); exitTempUsageScope(); - 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), @@ -396,6 +399,17 @@ protected abstract String getTempPrefix(); + /** + * 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 Wed Oct 28 09:10:53 2009 +++ /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java Wed Nov 18 09:31:51 2009 @@ -17,9 +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 @@ -27,6 +31,7 @@ */ public class PostOptimizationCompoundAssignmentNormalizer extends CompoundAssignmentNormalizer { + public static void exec(JProgram program) { new PostOptimizationCompoundAssignmentNormalizer(program).breakUpAssignments(); } @@ -40,6 +45,21 @@ return "$t"; } + @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() == program.getTypePrimitiveLong()) { @@ -50,6 +70,12 @@ && x.getType() != program.getTypePrimitiveDouble()) { 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; } @@ -68,4 +94,24 @@ } return false; } -} + + private boolean isIntegral(JType type) { + // if it can't be widened into a long, it must be float + return 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
