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);
}
