Reviewers: scottb, Description: Issue 3710 is caused by the Java inliner failing to take into account an implicit cast to long in Integer::longValue(). The inliner already adds explicit casts to the parameters of inlined methods. This patch has it do so for return values as well.
If we run into many such problems, we could eliminate the whole class of them by running the cast normalizer before optimization, thus making all the casts explicit and impossible to forget. Please review this at http://gwt-code-reviews.appspot.com/39805 Affected files: dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java user/test/com/google/gwt/dev/jjs/test/NativeLongTest.java Index: user/test/com/google/gwt/dev/jjs/test/NativeLongTest.java =================================================================== --- user/test/com/google/gwt/dev/jjs/test/NativeLongTest.java (revision 5562) +++ user/test/com/google/gwt/dev/jjs/test/NativeLongTest.java (working copy) @@ -23,6 +23,21 @@ * LongLibTest. */ public class NativeLongTest extends GWTTestCase { + /** + * A class that wraps an int. See {...@link NativeLongTest#testImplicitCastToLong()}. + */ + private static class IntegerWrapper { + private final int i; + + public IntegerWrapper(int i) { + this.i = i; + } + + public long longValue() { + return i; // implicit cast to long + } + } + private static class RequestIdFactory { static RequestIdFactory instance = new RequestIdFactory(); @@ -122,6 +137,10 @@ l += 5; assertEquals(15, l); assertTrue(15 == l); + + // Issue 3710 + IntegerWrapper wrap = new IntegerWrapper(20); + assertEquals(400L, wrap.longValue() * wrap.longValue()); } public void testInlinedIntInitializer() { Index: dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java =================================================================== --- dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java (revision 5562) +++ dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java (working copy) @@ -32,6 +32,7 @@ import com.google.gwt.dev.jjs.ast.JReturnStatement; import com.google.gwt.dev.jjs.ast.JStatement; import com.google.gwt.dev.jjs.ast.JThisRef; +import com.google.gwt.dev.jjs.ast.JType; import com.google.gwt.dev.jjs.ast.JVisitor; import com.google.gwt.dev.jjs.ast.js.JMultiExpression; @@ -221,6 +222,7 @@ if (expr != null) { if (!ignoringReturnValue || expr.hasSideEffects()) { JExpression clone = cloner.cloneExpression(expr); + clone = maybeCast(clone, body.getMethod().getType()); multi.exprs.add(clone); } } @@ -451,13 +453,7 @@ JExpression arg = methodCall.getArgs().get(paramIndex); JExpression clone = cloner.cloneExpression(arg); - /* - * Insert an implicit cast if the types differ; it might get optimized out - * later, but in some cases it will force correct math evaluation. - */ - if (clone.getType() != x.getType()) { - clone = new JCastOperation(clone.getSourceInfo(), x.getType(), clone); - } + clone = maybeCast(clone, x.getType()); ctx.replaceMe(clone); } } @@ -493,6 +489,17 @@ return new MethodInliner(program).execImpl(); } + /** + * Insert an implicit cast if the types differ; it might get optimized out + * later, but in some cases it will force correct math evaluation. + */ + private static JExpression maybeCast(JExpression exp, JType targetType) { + if (exp.getType() != targetType) { + exp = new JCastOperation(exp.getSourceInfo(), targetType, exp); + } + return exp; + } + private JMethod currentMethod; private final JProgram program; --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
