Fix for issue #937; static initializers must statically initialize super class. Also includes a fix for a possible ICE in GenerateJavaAST.java when JDT hands us a ThisRef while processing field initializers in an interface. Review by: mmendez, alex.tkachman git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@956 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JFieldRef.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JFieldRef.java index bdd7689..c78e8c7 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/ast/JFieldRef.java +++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JFieldRef.java
@@ -62,9 +62,8 @@ // A cross-class reference to a static, non constant field forces clinit if (field.isStatic() && (!field.isFinal() || field.constInitializer == null)) { - JReferenceType fieldEncloser = field.getEnclosingType(); - if (enclosingType != fieldEncloser - && program.typeOracle.hasClinit(fieldEncloser)) { + if (program.typeOracle.checkClinit(enclosingType, + field.getEnclosingType())) { // Therefore, we have side effects return true; }
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java index 855349c..8311ceb 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java +++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
@@ -98,7 +98,7 @@ JClassType cType = (JClassType) type; if (qType instanceof JClassType) { - return getOrCreate(subClassMap, cType).contains(qType); + return isSubClass(cType, (JClassType) qType); } else if (qType instanceof JInterfaceType) { return getOrCreate(couldImplementMap, cType).contains(qType); } @@ -148,15 +148,15 @@ JClassType cType = (JClassType) type; if (qType instanceof JClassType) { - return getOrCreate(superClassMap, cType).contains(qType); + return isSuperClass(cType, (JClassType) qType); } else if (qType instanceof JInterfaceType) { - return getOrCreate(implementsMap, cType).contains(qType); + return implementsInterface(cType, (JInterfaceType) qType); } } else if (type instanceof JInterfaceType) { JInterfaceType iType = (JInterfaceType) type; if (qType instanceof JInterfaceType) { - return getOrCreate(superInterfaceMap, iType).contains(qType); + return extendsInterface(iType, (JInterfaceType) qType); } } else if (type instanceof JNullType) { @@ -166,6 +166,33 @@ return false; } + /** + * Returns <code>true</code> if a static field access of <code>toType</code> + * from within <code>fromType</code> should generate a clinit call. This + * will be true in cases where <code>toType</code> has a live clinit method + * which we cannot statically know has already run. We can statically know the + * clinit method has already run when: + * <ol> + * <li><code>fromType == toType</code></li> + * <li><code>toType</code> is a superclass of <code>fromType</code> + * (because <code>toType</code>'s clinit would have already run + * <code>fromType</code>'s clinit; see JLS 12.4)</li> + * </ol> + */ + public boolean checkClinit(JReferenceType fromType, JReferenceType toType) { + if (fromType == toType) { + return false; + } + if (!hasClinit(toType)) { + return false; + } + if (fromType instanceof JClassType && toType instanceof JClassType + && isSuperClass((JClassType) fromType, (JClassType) toType)) { + return false; + } + return true; + } + public void computeAfterAST() { recomputeClinits(); } @@ -209,6 +236,13 @@ } } + /** + * Returns true if qType is a superinterface of type, directly or indirectly. + */ + public boolean extendsInterface(JInterfaceType type, JInterfaceType qType) { + return getOrCreate(superInterfaceMap, type).contains(qType); + } + public JMethod[] getAllVirtualOverrides(JMethod method) { Set/* <JMethod> */results = new HashSet/* <JMethod> */(); Map/* <JClassType, Set<JMethod>> */overrideMap = getOrCreateMap( @@ -228,9 +262,27 @@ } public boolean hasClinit(JReferenceType type) { + if (hasDirectClinit(type)) { + return true; + } + if (type != null && type.extnds != null) { + return hasClinit(type.extnds); + } + return false; + } + + public boolean hasDirectClinit(JReferenceType type) { return hasClinitSet.contains(type); } + /** + * Returns true if qType is an implemented interface of type, directly or + * indirectly. + */ + public boolean implementsInterface(JClassType type, JInterfaceType qType) { + return getOrCreate(implementsMap, type).contains(qType); + } + public boolean isInstantiatedType(JReferenceType type) { if (type instanceof JNullType) { return true; @@ -245,6 +297,20 @@ return instantiatedTypes.contains(type); } + /** + * Returns true if qType is a subclass of type, directly or indirectly. + */ + public boolean isSubClass(JClassType type, JClassType qType) { + return getOrCreate(subClassMap, type).contains(qType); + } + + /** + * Returns true if qType is a superclass of type, directly or indirectly. + */ + public boolean isSuperClass(JClassType type, JClassType qType) { + return getOrCreate(superClassMap, type).contains(qType); + } + public void recomputeClinits() { hasClinitSet.clear(); for (int i = 0; i < program.getDeclaredTypes().size(); ++i) {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java index a5759b9..f0bc1f5 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
@@ -886,14 +886,14 @@ JMethod method = (JMethod) typeMap.get(x.binding); assert (type == method.getType()); - JExpression qualifier = dispProcessExpression(x.receiver); + JExpression qualifier; if (x.receiver instanceof ThisReference) { if (method.isStatic()) { // don't bother qualifying it, it's a no-op - // TODO(???): this may be handled by later optimizations now qualifier = null; } else if (x.receiver instanceof QualifiedThisReference) { - // do nothing, the qualifier is correct + // use the supplied qualifier + qualifier = dispProcessExpression(x.receiver); } else { /* * In cases where JDT had to synthesize a this ref for us, it could @@ -902,6 +902,8 @@ */ qualifier = createThisRef(info, method.getEnclosingType()); } + } else { + qualifier = dispProcessExpression(x.receiver); } JMethodCall call = new JMethodCall(program, info, qualifier, method);
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java index 36ad3b3..66d473c 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
@@ -526,8 +526,13 @@ List/* <JsFunction> */jsFuncs = popList(x.methods.size()); // methods List/* <JsStatement> */jsFields = popList(x.fields.size()); // fields - if (typeOracle.hasClinit(x)) { - handleClinit((JsFunction) jsFuncs.get(0)); + if (typeOracle.hasDirectClinit(x)) { + // see if there's a super class we need to chain to + JClassType superType = x.extnds; + while (superType != null && !typeOracle.hasDirectClinit(superType)) { + superType = superType.extnds; + } + handleClinit((JsFunction) jsFuncs.get(0), superType); } else { jsFuncs.set(0, null); } @@ -751,9 +756,9 @@ JsStatements globalStmts = jsProgram.getGlobalBlock().getStatements(); - if (typeOracle.hasClinit(x)) { + if (typeOracle.hasDirectClinit(x)) { JsFunction clinitFunc = (JsFunction) jsFuncs.get(0); - handleClinit(clinitFunc); + handleClinit(clinitFunc, null); globalStmts.add(clinitFunc.makeStmt()); } @@ -1467,11 +1472,19 @@ } } - private void handleClinit(JsFunction clinitFunc) { + private void handleClinit(JsFunction clinitFunc, JReferenceType chainTo) { + JsStatements statements = clinitFunc.getBody().getStatements(); // self-assign to the null method immediately (to prevent reentrancy) JsExpression asg = createAssignment(clinitFunc.getName().makeRef(), nullMethodName.makeRef()); - clinitFunc.getBody().getStatements().add(0, asg.makeStmt()); + statements.add(0, asg.makeStmt()); + if (chainTo != null) { + JMethod chainToMeth = (JMethod) chainTo.methods.get(0); + JsInvocation jsInvocation = new JsInvocation(); + JsNameRef qualifier = getName(chainToMeth).makeRef(); + jsInvocation.setQualifier(qualifier); + statements.add(1, jsInvocation.makeStmt()); + } } private JsInvocation maybeCreateClinitCall(JField x) { @@ -1480,12 +1493,14 @@ } JReferenceType enclosingType = x.getEnclosingType(); - if (!typeOracle.hasClinit(enclosingType)) { + if (!typeOracle.checkClinit(currentMethod.getEnclosingType(), enclosingType)) { return null; } - // don't need to clinit on my own static fields - if (enclosingType == currentMethod.getEnclosingType()) { - return null; + + // The actual target class might not have a direct clinit; find the super + // class that does. + while (!typeOracle.hasDirectClinit(enclosingType)) { + enclosingType = enclosingType.extnds; } JMethod clinitMethod = (JMethod) enclosingType.methods.get(0); JsInvocation jsInvocation = new JsInvocation(); @@ -1497,21 +1512,23 @@ if (!x.isStatic()) { return null; } - JReferenceType enclosingType = x.getEnclosingType(); if (!typeOracle.hasClinit(enclosingType)) { return null; } - + if (program.isStaticImpl(x)) { + return null; + } // avoid recursion sickness if (x == enclosingType.methods.get(0)) { return null; } - if (program.isStaticImpl(x)) { - return null; + // The actual target class might not have a direct clinit; find the super + // class that does. + while (!typeOracle.hasDirectClinit(enclosingType)) { + enclosingType = enclosingType.extnds; } - JMethod clinitMethod = (JMethod) enclosingType.methods.get(0); JsInvocation jsInvocation = new JsInvocation(); jsInvocation.setQualifier(getName(clinitMethod).makeRef());
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java b/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java index 51c68ff..71b4a7d 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java
@@ -204,20 +204,14 @@ private boolean checkClinitViolation(JMethodCall x, JExpression resultExpression) { JReferenceType targetEnclosingType = x.getTarget().getEnclosingType(); - if (!program.typeOracle.hasClinit(targetEnclosingType)) { - // No clinit needed; target doesn't have one. + if (!program.typeOracle.checkClinit(currentMethod.getEnclosingType(), targetEnclosingType)) { + // Access from this class to the target class won't trigger a clinit return false; } if (program.isStaticImpl(x.getTarget())) { // No clinit needed; target is really an instance method. return false; } - if (currentMethod.getEnclosingType() == targetEnclosingType) { - // No clinit needed; intra-class call. - // TODO: we could maybe broaden the test condition to include types that - // we can statically determine must have been initialized. - return false; - } /* * Potential clinit violation! We can only allow this if the result is
diff --git a/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java b/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java index 3d6baab..1dde413 100644 --- a/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java +++ b/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
@@ -90,6 +90,16 @@ } } + private static class SideEffectCauser6Super { + static { + CompilerTest.sideEffectChecker++; + } + } + + private static class SideEffectCauser6 extends SideEffectCauser6Super { + public static String causeClinitSideEffectOnRead = "bar"; + } + private static final class UninstantiableType { public Object field; @@ -185,8 +195,10 @@ assertEquals(3, sideEffectChecker); String foo = SideEffectCauser4.causeClinitSideEffectOnRead; assertEquals(4, sideEffectChecker); - String bar = jsniReadSideEffectCauser5(); + jsniReadSideEffectCauser5(); assertEquals(5, sideEffectChecker); + foo = SideEffectCauser6.causeClinitSideEffectOnRead; + assertEquals(6, sideEffectChecker); String checkRescued = NonSideEffectCauser.NOT_A_COMPILE_TIME_CONSTANT; assertEquals(null, checkRescued); }