Reviewers: scottb, Message: Review requested. The explanation is longer than the bugfix.
Description: This fixes a problem where a statically-initialized field could be referred to before it has been initialized by its clinit. I found this pattern would result in invalid code generation; class NiceApi { static Foo IMPL = new FooImpl(); static NiceApi get() { return IMPL; } interface InnerIntf {} abstract void instance(InnerIntf inner); } class NiceApiImpl extends NiceApi { static Object SOME_FIELD = new Object(); void instance(InnerIntf inner) { instanceImpl(innerIntf); } static void instanceImpl(InnerIntf inner) { SOME_FIELD.doSomething(inner); } } could get reduced to doSomething(SOME_FIELD, (FooImplClinit(), moreCode)); Please review this at http://gwt-code-reviews.appspot.com/78818 Affected files: M dev/core/src/com/google/gwt/dev/jjs/impl/MakeCallsStatic.java Index: dev/core/src/com/google/gwt/dev/jjs/impl/MakeCallsStatic.java diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/MakeCallsStatic.java b/dev/core/src/com/google/gwt/dev/jjs/impl/MakeCallsStatic.java index 7a7f6b8481c606693a634d9ae1219c56444d2437..1fe417ad801aef52a677c146aece7f5744c91e68 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/MakeCallsStatic.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/MakeCallsStatic.java @@ -19,6 +19,7 @@ import com.google.gwt.dev.jjs.SourceInfo; import com.google.gwt.dev.jjs.ast.Context; import com.google.gwt.dev.jjs.ast.JAbstractMethodBody; import com.google.gwt.dev.jjs.ast.JClassType; +import com.google.gwt.dev.jjs.ast.JExpression; import com.google.gwt.dev.jjs.ast.JMethod; import com.google.gwt.dev.jjs.ast.JMethodBody; import com.google.gwt.dev.jjs.ast.JMethodCall; @@ -31,6 +32,7 @@ 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; import com.google.gwt.dev.jjs.ast.js.JsniMethodBody; import com.google.gwt.dev.js.ast.JsContext; import com.google.gwt.dev.js.ast.JsExpression; @@ -320,15 +322,31 @@ public class MakeCallsStatic { return new MakeCallsStatic(program).execImpl(); } - static JMethodCall makeStaticCall(JMethodCall x, JMethod newMethod) { + static JExpression makeStaticCall(JMethodCall x, JMethod newMethod) { // Update the call site JMethodCall newCall = new JMethodCall(x.getSourceInfo().makeChild( MakeCallsStatic.class, "Devirtualized function call"), null, newMethod); - // The qualifier becomes the first arg - newCall.addArg(x.getInstance()); - newCall.addArgs(x.getArgs()); - return newCall; + /* + * If the qualifier is a JMultiExpression, invoke on the last value. This + * ensures that clinits maintain the same execution order relative to + * parameters in deeply-inlined scenarios. + */ + // (a, b).foo() --> (a, foo(b)) + if (x.getInstance() instanceof JMultiExpression) { + JMultiExpression multi = (JMultiExpression) x.getInstance(); + int lastIndex = multi.exprs.size() - 1; + newCall.addArg(multi.exprs.get(lastIndex)); + newCall.addArgs(x.getArgs()); + multi.exprs.set(lastIndex, newCall); + return multi; + } else { + // The qualifier becomes the first arg + // a.foo(b) --> foo(a,b) + newCall.addArg(x.getInstance()); + newCall.addArgs(x.getArgs()); + return newCall; + } } protected Set<JMethod> toBeMadeStatic = new HashSet<JMethod>(); --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---