Fixes issue #3247. The upshot is, Pruner could leave the AST in an invalid state. In practice, this almost never mattered because TypeTightener would happily come along and fix things up. But sometimes, this would occur on the final Pruner pass, and since TypeTightener doesn't run after that, badness ensued.
Patch by: spoon, scottb
git-svn-id: https://google-web-toolkit.googlecode.com/svn/releases/1.6@4460 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java b/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
index 0c9859a..01b04ba 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
@@ -68,6 +68,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.Stack;
/**
* Remove globally unreferenced classes, interfaces, methods, parameters, and
@@ -90,19 +91,28 @@
public class Pruner {
/**
- * Remove assignments to pruned fields, locals and params. Also nullify the
- * return type of methods declared to return a globally uninstantiable type.
+ * Remove assignments to pruned fields, locals and params. Nullify the return
+ * type of methods declared to return a globally uninstantiable type. Replace
+ * references to pruned variables and methods by references to the null field
+ * and null method, and drop assignments to pruned variables.
*/
private class CleanupRefsVisitor extends JModVisitor {
+ private Stack<JExpression> lValues = new Stack<JExpression>();
+ {
+ // Initialize a sentinel value to avoid having to check for empty stack.
+ lValues.push(null);
+ }
+
@Override
public void endVisit(JBinaryOperation x, Context ctx) {
// The LHS of assignments may have been pruned.
if (x.getOp() == JBinaryOperator.ASG) {
+ lValues.pop();
JExpression lhs = x.getLhs();
if (lhs instanceof JVariableRef) {
JVariableRef variableRef = (JVariableRef) lhs;
- if (!referencedNonTypes.contains(variableRef.getTarget())) {
+ if (isVariablePruned(variableRef.getTarget())) {
// TODO: better null tracking; we might be missing some NPEs here.
JExpression replacement = makeReplacementForAssignment(
x.getSourceInfo(), variableRef, x.getRhs());
@@ -114,8 +124,9 @@
@Override
public void endVisit(JDeclarationStatement x, Context ctx) {
+ lValues.pop();
// The variable may have been pruned.
- if (!referencedNonTypes.contains(x.getVariableRef().getTarget())) {
+ if (isVariablePruned(x.getVariableRef().getTarget())) {
JExpression replacement = makeReplacementForAssignment(
x.getSourceInfo(), x.getVariableRef(), x.getInitializer());
ctx.replaceMe(replacement.makeStatement());
@@ -123,6 +134,25 @@
}
@Override
+ public void endVisit(JFieldRef x, Context ctx) {
+ // Handle l-values at a higher level.
+ if (lValues.peek() == x) {
+ return;
+ }
+
+ if (isPruned(x.getField())) {
+ /*
+ * We assert that field must be non-static if it's an rvalue, otherwise
+ * it would have been rescued.
+ */
+ assert !x.getField().isStatic();
+ // The field is gone; replace x by a null field reference.
+ JFieldRef fieldRef = transformToNullFieldRef(x, program);
+ ctx.replaceMe(fieldRef);
+ }
+ }
+
+ @Override
public void endVisit(JMethod x, Context ctx) {
JType type = x.getType();
if (type instanceof JReferenceType) {
@@ -136,6 +166,16 @@
public void endVisit(JMethodCall x, Context ctx) {
JMethod method = x.getTarget();
+ // Is the method pruned entirely?
+ if (isPruned(method)) {
+ /*
+ * We assert that method must be non-static, otherwise it would have
+ * been rescued.
+ */
+ ctx.replaceMe(transformToNullMethodCall(x, program));
+ return;
+ }
+
// Did we prune the parameters of the method we're calling?
if (methodToOriginalParamsMap.containsKey(method)) {
// This must be a static method
@@ -187,7 +227,7 @@
@Override
public void endVisit(JsniFieldRef x, Context ctx) {
- if (isUninstantiable(x.getField())) {
+ if (isPruned(x.getField())) {
String ident = x.getIdent();
JField nullField = program.getNullField();
program.jsniMap.put(ident, nullField);
@@ -201,7 +241,7 @@
@Override
public void endVisit(JsniMethodRef x, Context ctx) {
// Redirect JSNI refs to uninstantiable types to the null method.
- if (isUninstantiable(x.getTarget())) {
+ if (isPruned(x.getTarget())) {
String ident = x.getIdent();
JMethod nullMethod = program.getNullMethod();
program.jsniMap.put(ident, nullMethod);
@@ -211,13 +251,36 @@
}
}
- private <T extends HasEnclosingType & CanBeStatic> boolean isUninstantiable(
- T node) {
+ @Override
+ public boolean visit(JBinaryOperation x, Context ctx) {
+ if (x.getOp() == JBinaryOperator.ASG) {
+ lValues.push(x.getLhs());
+ }
+ return true;
+ }
+
+ @Override
+ public boolean visit(JDeclarationStatement x, Context ctx) {
+ lValues.push(x.getVariableRef());
+ return true;
+ }
+
+ private <T extends HasEnclosingType & CanBeStatic> boolean isPruned(T node) {
+ if (!referencedNonTypes.contains(node)) {
+ return true;
+ }
JReferenceType enclosingType = node.getEnclosingType();
return !node.isStatic() && enclosingType != null
&& !program.typeOracle.isInstantiatedType(enclosingType);
}
+ private boolean isVariablePruned(JVariable variable) {
+ if (variable instanceof JField) {
+ return isPruned((JField) variable);
+ }
+ return !referencedNonTypes.contains(variable);
+ }
+
private JExpression makeReplacementForAssignment(SourceInfo info,
JVariableRef variableRef, JExpression rhs) {
// Replace with a multi, which may wind up empty.
@@ -232,7 +295,7 @@
}
}
- // If there is an initializer, evaluate it second.
+ // If there is an rhs, evaluate it second.
if (rhs != null) {
multi.exprs.add(rhs);
}
@@ -948,6 +1011,59 @@
return new Pruner(program, noSpecialTypes).execImpl();
}
+ /**
+ * Transform a reference to a pruned instance field into a reference to the
+ * null field, which will be used to replace <code>x</code>.
+ */
+ public static JFieldRef transformToNullFieldRef(JFieldRef x, JProgram program) {
+ JExpression instance = x.getInstance();
+ assert instance != null;
+ if (!instance.hasSideEffects()) {
+ instance = program.getLiteralNull();
+ }
+
+ JFieldRef fieldRef = new JFieldRef(program, x.getSourceInfo(), instance,
+ program.getNullField(), x.getEnclosingType(), primitiveTypeOrNullType(
+ program, x.getType()));
+ return fieldRef;
+ }
+
+ /**
+ * Transform a call to a pruned instance method (or static impl) into a call
+ * to the null method, which will be used to replace <code>x</code>.
+ */
+ public static JMethodCall transformToNullMethodCall(JMethodCall x,
+ JProgram program) {
+ JExpression instance = x.getInstance();
+ List<JExpression> args = x.getArgs();
+ if (program.isStaticImpl(x.getTarget())) {
+ instance = args.get(0);
+ args = args.subList(1, args.size());
+ } else {
+ assert !x.getTarget().isStatic();
+ }
+ assert (instance != null);
+ if (!instance.hasSideEffects()) {
+ instance = program.getLiteralNull();
+ }
+
+ JMethodCall newCall = new JMethodCall(program, x.getSourceInfo(), instance,
+ program.getNullMethod(), primitiveTypeOrNullType(program, x.getType()));
+ // Retain the original arguments, they will be evaluated for side effects.
+ newCall.getArgs().addAll(args);
+ return newCall;
+ }
+
+ /**
+ * Return the smallest type that is is a subtype of the argument.
+ */
+ static JType primitiveTypeOrNullType(JProgram program, JType type) {
+ if (type instanceof JPrimitiveType) {
+ return type;
+ }
+ return program.getTypeNull();
+ }
+
private final Map<JMethod, ArrayList<JParameter>> methodToOriginalParamsMap = new HashMap<JMethod, ArrayList<JParameter>>();
private final JProgram program;
private final Set<JNode> referencedNonTypes = new HashSet<JNode>();
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java b/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
index 65ed0ab..836ed93 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
@@ -38,7 +38,6 @@
import com.google.gwt.dev.jjs.ast.JNullType;
import com.google.gwt.dev.jjs.ast.JParameter;
import com.google.gwt.dev.jjs.ast.JParameterRef;
-import com.google.gwt.dev.jjs.ast.JPrimitiveType;
import com.google.gwt.dev.jjs.ast.JProgram;
import com.google.gwt.dev.jjs.ast.JReferenceType;
import com.google.gwt.dev.jjs.ast.JReturnStatement;
@@ -110,14 +109,13 @@
x.getField(), x.getEnclosingType());
ctx.replaceMe(fieldRef);
}
- } else if (!isStatic && instance.getType() == typeNull) {
+ } else if (!isStatic && instance.getType() == typeNull
+ && x.getField() != program.getNullField()) {
+ // Change any dereference of null to use the null field
if (!instance.hasSideEffects()) {
instance = program.getLiteralNull();
}
- JFieldRef fieldRef = new JFieldRef(program, x.getSourceInfo(),
- instance, program.getNullField(), null,
- primitiveTypeOrNullType(x.getType()));
- ctx.replaceMe(fieldRef);
+ ctx.replaceMe(Pruner.transformToNullFieldRef(x, program));
}
}
@@ -137,38 +135,14 @@
ctx.replaceMe(newCall);
}
} else if (!isStatic && instance.getType() == typeNull) {
- // bind null instance calls to the null method
- if (!instance.hasSideEffects()) {
- instance = program.getLiteralNull();
- }
- JMethodCall newCall = new JMethodCall(program, x.getSourceInfo(),
- instance, program.getNullMethod(),
- primitiveTypeOrNullType(x.getType()));
- ctx.replaceMe(newCall);
+ ctx.replaceMe(Pruner.transformToNullMethodCall(x, program));
} else if (isStaticImpl && method.params.size() > 0
&& method.params.get(0).isThis() && x.getArgs().size() > 0
&& x.getArgs().get(0).getType() == typeNull) {
// bind null instance calls to the null method for static impls
- instance = x.getArgs().get(0);
- if (!instance.hasSideEffects()) {
- instance = program.getLiteralNull();
- }
- JMethodCall newCall = new JMethodCall(program, x.getSourceInfo(),
- instance, program.getNullMethod(),
- primitiveTypeOrNullType(x.getType()));
- ctx.replaceMe(newCall);
+ ctx.replaceMe(Pruner.transformToNullMethodCall(x, program));
}
}
-
- /**
- * Return the smallest type that is is a subtype of the argument.
- */
- private JType primitiveTypeOrNullType(JType type) {
- if (type instanceof JPrimitiveType) {
- return type;
- }
- return program.getTypeNull();
- }
}
/**
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 e6e3657..a383f0b 100644
--- a/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
+++ b/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
@@ -1008,7 +1008,11 @@
try {
u.intField = 0;
- fail("Expected NullPointerException (5)");
+ /*
+ * Currently, null.nullField gets pruned rather than being allowed to
+ * execute its side effect of tripping an NPE.
+ */
+ // fail("Expected NullPointerException (5)");
} catch (Exception expected) {
}
