Reviewers: Lex, Message: Thanks. I left it as recordMethodCall for now because I'd already ran all the tests, and couldn't think of a better one. FWIW, it is recording the fact that some method call is occurring, and we don't analyze the target method to find out specifically what it does. Hopefully all this code will die soon anyway.
Description: Fixes a bug in ExpressionAnalyzer were clinit-triggering field refs were not being considered. This could cause bugs in the inliner where clinits would be dropped. Please review this at http://gwt-code-reviews.appspot.com/124802 Affected files: M dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java M dev/core/test/com/google/gwt/dev/jjs/impl/ExpressionAnalyzerTest.java Index: dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java index d8c9904bb95f170b3c11d0754be8e5ff3432d8c8..18005f059d024ee489ec0d137f29a5a0d5fd0bef 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java @@ -130,6 +130,10 @@ public class ExpressionAnalyzer extends JVisitor { accessesFieldNonFinal = true; } + if (x.hasClinit()) { + recordMethodCall(); + } + JExpression instance = x.getInstance(); if (instance == null) { return; @@ -159,15 +163,7 @@ public class ExpressionAnalyzer extends JVisitor { @Override public void endVisit(JMethodCall x, Context ctx) { - /* - * We can't assume anything about method calls right now, except that it - * can't assign to one of our locals or one of our parameters. It's possible - * that it could read from a field, assign to a field or throw an exception - * that we can't see. - */ - assignmentToField = true; - accessesField = true; - canThrowException = true; + recordMethodCall(); } @Override @@ -297,4 +293,19 @@ public class ExpressionAnalyzer extends JVisitor { assignmentToLocal = true; } } + + /** + * We can't assume anything about method calls right now, except that it can't + * access any of our locals or parameters. + * + * TODO: what about accessing arrays? Should be treated like field refs I + * guess. + */ + private void recordMethodCall() { + assignmentToField = true; + accessesField = true; + accessesFieldNonFinal = true; + canThrowException = true; + createsObject = true; + } } Index: dev/core/test/com/google/gwt/dev/jjs/impl/ExpressionAnalyzerTest.java diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/ExpressionAnalyzerTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/ExpressionAnalyzerTest.java index 47bc7b36ed1c6f6839aa341aeb9fd0d505f09418..ee0252ed9c513a10466450d14ee3a396dd10032a 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/ExpressionAnalyzerTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/ExpressionAnalyzerTest.java @@ -116,6 +116,22 @@ public class ExpressionAnalyzerTest extends OptimizerTestBase { analyzeExpression("int", "0").check(); } + public void testFieldAccessClinit() throws Exception { + sourceOracle.addOrReplace(new MockJavaResource("test.Foo") { + @Override + protected CharSequence getContent() { + StringBuffer code = new StringBuffer(); + code.append("package test;\n"); + code.append("public class Foo {\n"); + code.append(" static final boolean value = trueMethod();"); + code.append(" static boolean trueMethod() { return true; }"); + code.append("}\n"); + return code; + } + }); + analyzeExpression("boolean", "Foo.value").accessesFieldNonFinal().canThrowException().createsObject().hasAssignmentToField().check(); + } + public void testFieldAccessInstance() throws Exception { sourceOracle.addOrReplace(new MockJavaResource("test.Foo") { @Override -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
