Fixes issue #1132. In r956 I put in a fix to make static initializers chain to their superclass static initializers. Unfortunately, the change I put in resulted in the wrong evaluation order. I'm backing out much of that change, and instead fixing the same problem a different way.
This change adds an explicit chaining call from a subclass clinit to a superclass clinit very early in AST generation. It's actually a much more straightforward fix. This may produce slightly more code in the short term, because the old implementation would essentially elide empty subclass ctors. When we we get better inlining in place any differences should go away.
Found by: ispeters, jcai99
Review by: mmendez
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@1197 8db76d5a-ed1c-0410-87a9-c151d255dfc7
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 8311ceb..a7eff3e 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
@@ -262,16 +262,6 @@
}
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);
}
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 b8a0765..98586bf 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
@@ -265,6 +265,18 @@
currentSeparatorPositions = x.compilationResult.lineSeparatorPositions;
currentFileName = String.valueOf(x.compilationResult.fileName);
+ /*
+ * Make clinits chain to super class (JDT doesn't write code to do
+ * this). Call super class $clinit; $clinit is always in position 0.
+ */
+ if (currentClass.extnds != null) {
+ JMethod myClinit = (JMethod) currentClass.methods.get(0);
+ JMethod superClinit = (JMethod) currentClass.extnds.methods.get(0);
+ JMethodCall superClinitCall = new JMethodCall(program,
+ myClinit.getSourceInfo(), null, superClinit);
+ myClinit.body.statements.add(0, superClinitCall.makeStatement());
+ }
+
if (x.fields != null) {
// Process fields
for (int i = 0, n = x.fields.length; i < n; ++i) {
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 84d9b4c..6a17d97 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,13 +526,8 @@
List/* <JsFunction> */jsFuncs = popList(x.methods.size()); // methods
List/* <JsStatement> */jsFields = popList(x.fields.size()); // fields
- 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);
+ if (typeOracle.hasClinit(x)) {
+ handleClinit((JsFunction) jsFuncs.get(0));
} else {
jsFuncs.set(0, null);
}
@@ -752,9 +747,9 @@
JsStatements globalStmts = jsProgram.getGlobalBlock().getStatements();
- if (typeOracle.hasDirectClinit(x)) {
+ if (typeOracle.hasClinit(x)) {
JsFunction clinitFunc = (JsFunction) jsFuncs.get(0);
- handleClinit(clinitFunc, null);
+ handleClinit(clinitFunc);
globalStmts.add(clinitFunc.makeStmt());
}
@@ -1479,19 +1474,12 @@
}
}
- private void handleClinit(JsFunction clinitFunc, JReferenceType chainTo) {
+ private void handleClinit(JsFunction clinitFunc) {
JsStatements statements = clinitFunc.getBody().getStatements();
// self-assign to the null method immediately (to prevent reentrancy)
JsExpression asg = createAssignment(clinitFunc.getName().makeRef(),
nullMethodName.makeRef());
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) {
@@ -1505,11 +1493,6 @@
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());
@@ -1532,11 +1515,6 @@
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/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java b/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
index dac5929..b894cb5 100644
--- a/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
+++ b/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
@@ -100,6 +100,30 @@
}
}
+ /**
+ * Ensures that a superclass's clinit is run before supercall arguments are
+ * evaluated.
+ */
+ private static class SideEffectCauser7 extends SideEffectCauser7Super {
+ public SideEffectCauser7() {
+ super(SideEffectCauser7Super.SHOULD_BE_TRUE);
+ }
+ }
+
+ private static class SideEffectCauser7Super {
+ public static boolean SHOULD_BE_TRUE = false;
+
+ static {
+ SHOULD_BE_TRUE = true;
+ }
+
+ public SideEffectCauser7Super(boolean should_be_true) {
+ if (should_be_true) {
+ CompilerTest.sideEffectChecker++;
+ }
+ }
+ }
+
private static final class UninstantiableType {
public Object field;
@@ -203,6 +227,8 @@
assertEquals(5, sideEffectChecker);
foo = SideEffectCauser6.causeClinitSideEffectOnRead;
assertEquals(6, sideEffectChecker);
+ new SideEffectCauser7();
+ assertEquals(7, sideEffectChecker);
String checkRescued = NonSideEffectCauser.NOT_A_COMPILE_TIME_CONSTANT;
assertEquals(null, checkRescued);
}
