JChangeTrackingVisitor was missing some field modifications.
Fields were not considered modified if only their initializer
was modified.
Now we conservatively consider a static field modified if the class
initializer ($clinit) for that that class was considered modified; and
similarly for instance fields and instance initializer.
Change-Id: Ie7c2897d01a77cdd789b50bfd20e500b8a361b32
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JClassType.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JClassType.java
index 7d0e38d..4b94ea6 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JClassType.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JClassType.java
@@ -17,6 +17,7 @@
import com.google.gwt.dev.jjs.SourceInfo;
import com.google.gwt.dev.jjs.SourceOrigin;
+import com.google.gwt.dev.jjs.impl.GwtAstBuilder;
import java.io.Serializable;
@@ -62,6 +63,19 @@
}
@Override
+ public final JMethod getInitMethod() {
+ assert getMethods().size() > 1;
+ JMethod init = this.getMethods().get(1);
+
+ if (!init.getName().equals(GwtAstBuilder.INIT_NAME)) {
+ // the init method was removed.
+ return null;
+ }
+
+ return init;
+ }
+
+ @Override
public final JClassType getSuperClass() {
return superClass;
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java
index c0df81f..01c201a 100755
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java
@@ -319,18 +319,10 @@
/**
* Returns the instance initializer ($init) method.
- * Can only be called after making sure the class has an instance initializer method.
*
* @return The instance initializer method.
*/
- public final JMethod getInitMethod() {
- assert getMethods().size() > 1;
- JMethod init = this.getMethods().get(1);
-
- assert init != null;
- assert init.getName().equals(GwtAstBuilder.INIT_NAME);
- return init;
- }
+ public abstract JMethod getInitMethod();
@Override
public String getJavahSignatureName() {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JInterfaceType.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JInterfaceType.java
index 68992e2..0ad9d82 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JInterfaceType.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JInterfaceType.java
@@ -55,6 +55,11 @@
}
@Override
+ public final JMethod getInitMethod() {
+ return null;
+ }
+
+ @Override
public JClassType getSuperClass() {
return null;
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java
index 2135760..061b708 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java
@@ -276,6 +276,19 @@
return isClinit;
}
+ public static boolean isInit(JMethod method) {
+ JDeclaredType enclosingType = method.getEnclosingType();
+
+ if (method.isStatic()) {
+ // Hack, check the name.
+ return method.getName().equals(GwtAstBuilder.STATIC_INIT_NAME);
+ }
+
+ boolean isInit = enclosingType != null && method == enclosingType.getInitMethod();
+ assert !isInit || method.getName().equals(GwtAstBuilder.INIT_NAME);
+ return isInit;
+ }
+
public static void serializeTypes(List<JDeclaredType> types, ObjectOutputStream stream)
throws IOException {
stream.writeObject(types);
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java b/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java
index e56beee..b602019 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java
@@ -2031,9 +2031,11 @@
* context).
*/
public static OptimizerStats exec(JProgram program, OptimizerContext optimizerCtx) {
- Iterable<JMethod> modifiedMethods =
+ Set<JMethod> affectedMethods =
optimizerCtx.getModifiedMethodsSince(optimizerCtx.getLastStepFor(NAME));
- OptimizerStats stats = new DeadCodeElimination(program).execImpl(modifiedMethods, optimizerCtx);
+ affectedMethods.addAll(optimizerCtx.getMethodsByReferencedFields(
+ optimizerCtx.getModifiedFieldsSince(optimizerCtx.getLastStepFor(NAME))));
+ OptimizerStats stats = new DeadCodeElimination(program).execImpl(affectedMethods, optimizerCtx);
optimizerCtx.setLastStepFor(NAME, optimizerCtx.getOptimizationStep());
optimizerCtx.incOptimizationStep();
JavaAstVerifier.assertProgramIsConsistent(program);
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/JChangeTrackingVisitor.java b/dev/core/src/com/google/gwt/dev/jjs/impl/JChangeTrackingVisitor.java
index 2294e5e..27fbff1 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/JChangeTrackingVisitor.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/JChangeTrackingVisitor.java
@@ -18,6 +18,7 @@
import com.google.gwt.dev.jjs.ast.JField;
import com.google.gwt.dev.jjs.ast.JMethod;
import com.google.gwt.dev.jjs.ast.JModVisitor;
+import com.google.gwt.dev.jjs.ast.JProgram;
import com.google.gwt.dev.jjs.ast.JVariable;
import java.util.Collection;
@@ -81,10 +82,24 @@
@Override
public final void endVisit(JMethod x, Context ctx) {
exit(x, ctx);
- if (methodModified) {
- optimizerCtx.markModified(x);
- }
currentMethod = null;
+
+ if (!methodModified) {
+ return;
+ }
+
+ optimizerCtx.markModified(x);
+ if (JProgram.isClinit(x) || JProgram.isInit(x)) {
+ // Mark all class static fields as modified when a class clinit is modified to reflect
+ // that when inline declaration statements are modified the corresponding field must
+ // be considered modified.
+ for (JField potentiallyModifiedField: x.getEnclosingType().getFields()) {
+ if (potentiallyModifiedField.isStatic() && JProgram.isClinit(x)
+ || !potentiallyModifiedField.isStatic() && !JProgram.isClinit(x)) {
+ optimizerCtx.markModified(potentiallyModifiedField);
+ }
+ }
+ }
}
@Override
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/JChangeTrackingVisitorTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/JChangeTrackingVisitorTest.java
index 4ef8ecf..ac25619 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/JChangeTrackingVisitorTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/JChangeTrackingVisitorTest.java
@@ -26,6 +26,13 @@
import com.google.gwt.dev.jjs.ast.JPrimitiveType;
import com.google.gwt.dev.jjs.ast.JProgram;
import com.google.gwt.dev.jjs.ast.JVariable;
+import com.google.gwt.thirdparty.guava.common.base.Function;
+import com.google.gwt.thirdparty.guava.common.collect.FluentIterable;
+import com.google.gwt.thirdparty.guava.common.collect.ImmutableSet;
+import com.google.gwt.thirdparty.guava.common.collect.Sets;
+
+import java.util.Arrays;
+import java.util.Set;
/**
* Test for {@link JChangeTrackingVisitor}.
@@ -467,6 +474,8 @@
public void testModificationTracking() throws Exception {
addSnippetClassDecl("static class A {",
" public double field;",
+ " public static int staticField = 1 < 2 ? 1 : 0;",
+ " public int instanceField = 1 + 1;",
" public A(double f) { field = f; }",
" public void fun1 () { for(int i = 3; i < 4; i++) i = 8; }",
" public int fun2 (int a) { return a > 1 ? 1 : 0; }",
@@ -488,89 +497,103 @@
new ReplaceConditionalExprWithItsThenExprVisitor(optimizerCtx);
repalceConditionalExprVisitor.accept(program.getFromTypeMap("test.EntryPoint$A"));
optimizerCtx.incOptimizationStep();
- assertEquals(0, optimizerCtx.getModifiedFieldsSince(first).size());
- assertEquals(3, optimizerCtx.getModifiedMethodsSince(first).size());
- assertTrue(optimizerCtx.getModifiedMethodsSince(first).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun2")));
- assertTrue(optimizerCtx.getModifiedMethodsSince(first).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun3")));
- assertTrue(optimizerCtx.getModifiedMethodsSince(first).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun5")));
+ Set<JMethod> modifiedMethodsInIteration1 =
+ ImmutableSet.copyOf(getMethods(program, "test.EntryPoint$A",
+ "fun2", "fun3", "fun5", GwtAstBuilder.CLINIT_NAME));
+ Set<JField> modifiedFieldsInIteration1 =
+ ImmutableSet.copyOf(getFields(program, "test.EntryPoint$A",
+ "staticField"));
+
+ assertEquals(modifiedFieldsInIteration1, optimizerCtx.getModifiedFieldsSince(first));
+ assertEquals(modifiedMethodsInIteration1, optimizerCtx.getModifiedMethodsSince(first));
int second = optimizerCtx.getOptimizationStep();
ReplaceAddOperationWithItsFirstOperandVisitor replaceAddOperationVisitor =
new ReplaceAddOperationWithItsFirstOperandVisitor(optimizerCtx);
replaceAddOperationVisitor.accept(program.getFromTypeMap("test.EntryPoint$A"));
optimizerCtx.incOptimizationStep();
- assertEquals(0, optimizerCtx.getModifiedFieldsSince(second).size());
- assertEquals(2, optimizerCtx.getModifiedMethodsSince(second).size());
- assertTrue(optimizerCtx.getModifiedMethodsSince(second).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun4")));
- assertTrue(optimizerCtx.getModifiedMethodsSince(second).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun5")));
+ Set<JMethod> modifiedMethodsInIteration2 =
+ ImmutableSet.copyOf(getMethods(program, "test.EntryPoint$A",
+ "fun4", "fun5", GwtAstBuilder.INIT_NAME));
+ Set<JField> modifiedFieldsInIteration2 =
+ ImmutableSet.copyOf(getFields(program, "test.EntryPoint$A",
+ "instanceField", "field"));
- assertEquals(0, optimizerCtx.getModifiedFieldsSince(first).size());
+ assertEquals(modifiedFieldsInIteration2, optimizerCtx.getModifiedFieldsSince(second));
+ assertEquals(modifiedMethodsInIteration2, optimizerCtx.getModifiedMethodsSince(second));
- assertEquals(4, optimizerCtx.getModifiedMethodsSince(first).size());
- assertTrue(optimizerCtx.getModifiedMethodsSince(first).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun2")));
- assertTrue(optimizerCtx.getModifiedMethodsSince(first).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun3")));
- assertTrue(optimizerCtx.getModifiedMethodsSince(first).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun4")));
- assertTrue(optimizerCtx.getModifiedMethodsSince(first).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun5")));
+ assertEquals(Sets.union(modifiedFieldsInIteration1, modifiedFieldsInIteration2),
+ optimizerCtx.getModifiedFieldsSince(first));
+ assertEquals(Sets.union(modifiedMethodsInIteration1, modifiedMethodsInIteration2),
+ optimizerCtx.getModifiedMethodsSince(first));
int third = optimizerCtx.getOptimizationStep();
SetFieldOfIntToLongVisitor setFieldOfIntToLongVisitor =
new SetFieldOfIntToLongVisitor(optimizerCtx);
setFieldOfIntToLongVisitor.accept(program.getFromTypeMap("test.EntryPoint$B"));
optimizerCtx.incOptimizationStep();
- assertEquals(1, optimizerCtx.getModifiedFieldsSince(third).size());
- assertTrue(optimizerCtx.getModifiedFieldsSince(third).contains(
- JJSTestBase.findField(program.getFromTypeMap("test.EntryPoint$B"), "field1")));
- assertEquals(0, optimizerCtx.getModifiedMethodsSince(third).size());
+ Set<JMethod> modifiedMethodsInIteration3 = ImmutableSet.of();
+ Set<JField> modifiedFieldsInIteration3 =
+ ImmutableSet.copyOf(getFields(program, "test.EntryPoint$B",
+ "field1"));
- assertEquals(1, optimizerCtx.getModifiedFieldsSince(second).size());
- assertTrue(optimizerCtx.getModifiedFieldsSince(second).contains(
- JJSTestBase.findField(program.getFromTypeMap("test.EntryPoint$B"), "field1")));
+ assertEquals(modifiedFieldsInIteration3, optimizerCtx.getModifiedFieldsSince(third));
+ assertEquals(modifiedMethodsInIteration3, optimizerCtx.getModifiedMethodsSince(third));
- assertEquals(2, optimizerCtx.getModifiedMethodsSince(second).size());
- assertTrue(optimizerCtx.getModifiedMethodsSince(second).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun4")));
- assertTrue(optimizerCtx.getModifiedMethodsSince(second).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun5")));
+ assertEquals(Sets.union(modifiedFieldsInIteration2, modifiedFieldsInIteration3),
+ optimizerCtx.getModifiedFieldsSince(second));
+ assertEquals(Sets.union(modifiedMethodsInIteration2, modifiedMethodsInIteration3),
+ optimizerCtx.getModifiedMethodsSince(second));
- assertEquals(1, optimizerCtx.getModifiedFieldsSince(first).size());
- assertTrue(optimizerCtx.getModifiedFieldsSince(first).contains(
- JJSTestBase.findField(program.getFromTypeMap("test.EntryPoint$B"), "field1")));
+ assertEquals(Sets.union(Sets.union(
+ modifiedFieldsInIteration1, modifiedFieldsInIteration2), modifiedFieldsInIteration3),
+ optimizerCtx.getModifiedFieldsSince(first));
+ assertEquals(Sets.union(Sets.union(
+ modifiedMethodsInIteration1, modifiedMethodsInIteration2), modifiedMethodsInIteration3),
+ optimizerCtx.getModifiedMethodsSince(first));
- assertEquals(4, optimizerCtx.getModifiedMethodsSince(first).size());
- assertTrue(optimizerCtx.getModifiedMethodsSince(first).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun2")));
- assertTrue(optimizerCtx.getModifiedMethodsSince(first).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun3")));
- assertTrue(optimizerCtx.getModifiedMethodsSince(first).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun4")));
- assertTrue(optimizerCtx.getModifiedMethodsSince(first).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun5")));
+ Set<JMethod> methodsDeletedInIteration4 = getMethods(program, "test.EntryPoint$A", "fun5");
RemoveMethodsWithThreeParamsVisitor removeMethodsWithThreeParamsVisitor =
new RemoveMethodsWithThreeParamsVisitor(optimizerCtx);
removeMethodsWithThreeParamsVisitor.accept(program.getFromTypeMap("test.EntryPoint$A"));
- assertEquals(3, optimizerCtx.getModifiedMethodsSince(first).size());
- assertTrue(optimizerCtx.getModifiedMethodsSince(first).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun2")));
- assertTrue(optimizerCtx.getModifiedMethodsSince(first).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun3")));
- assertTrue(optimizerCtx.getModifiedMethodsSince(first).contains(
- JJSTestBase.findMethod(program.getFromTypeMap("test.EntryPoint$A"), "fun4")));
+ assertEquals(Sets.difference(Sets.union(Sets.union(
+ modifiedMethodsInIteration1, modifiedMethodsInIteration2),
+ modifiedMethodsInIteration3),methodsDeletedInIteration4),
+ optimizerCtx.getModifiedMethodsSince(first));
+
+ Set<JField> fieldsDeletedInIteration4 = getFields(program, "test.EntryPoint$B", "field1");
RemoveFieldsOfLongType removeFieldsOfLongType = new RemoveFieldsOfLongType(optimizerCtx);
removeFieldsOfLongType.accept(program.getFromTypeMap("test.EntryPoint$B"));
- assertEquals(0, optimizerCtx.getModifiedFieldsSince(first).size());
+
+ assertEquals(Sets.difference(Sets.union(Sets.union(
+ modifiedFieldsInIteration1, modifiedFieldsInIteration2),
+ modifiedFieldsInIteration3),fieldsDeletedInIteration4),
+ optimizerCtx.getModifiedFieldsSince(first));
+ }
+
+ private Set<JMethod> getMethods(
+ final JProgram program, final String clazz, String... methodNames) {
+ return FluentIterable.from(Arrays.asList(methodNames)).transform(
+ new Function<String, JMethod>() {
+ @Override
+ public JMethod apply(String methodName) {
+ return JJSTestBase.findMethod(program.getFromTypeMap(clazz), methodName);
+ }
+ }).toSet();
+ }
+
+ private Set<JField> getFields(final JProgram program, final String clazz, String... fieldNames) {
+ return FluentIterable.from(Arrays.asList(fieldNames)).transform(
+ new Function<String, JField>() {
+ @Override
+ public JField apply(String fieldName) {
+ return JJSTestBase.findField(program.getFromTypeMap(clazz), fieldName);
+ }
+ }).toSet();
}
}
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/JJSTestBase.java b/dev/core/test/com/google/gwt/dev/jjs/impl/JJSTestBase.java
index 7e53200..69bbc67 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/JJSTestBase.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/JJSTestBase.java
@@ -374,9 +374,9 @@
*/
protected SourceLevel sourceLevel = SourceLevel.DEFAULT_SOURCE_LEVEL;
- protected static void assertContainsAll(Iterable<String> expectedMethodSnippets,
- Set<String> actualMethodSnippets) {
- List<String> missing = FluentIterable.from(expectedMethodSnippets)
+ protected static <T> void assertContainsAll(Iterable<T> expectedMethodSnippets,
+ Set<T> actualMethodSnippets) {
+ List<T> missing = FluentIterable.from(expectedMethodSnippets)
.filter(Predicates.not(Predicates.in(actualMethodSnippets)))
.toList();
assertTrue(missing + " not contained in " + actualMethodSnippets, missing.size() == 0);
