This addresses issue #622. The problem is that CastOptimizer leaves the
tree in a particular state, namely:
"Cast.throwClassCastExceptionUnlessNull(value).value". The expectation is
that FixDanglignRefsVisitor would replace the ".value" field reference
with ".nullField". The problem is, however, that only TightenTypesVisitor
can force FDRV to run; CastOptimizer cannot.
My fix is to eliminate CastOptimizer altogether and move its work (mostly)
into TypeTightener. Impossible casts are replaced in the tree with a
(<null>) cast, which becomes a throwClassCastExceptionUnlessNull() in
CastNormalizer.
Review by: mmendez
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@403 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
index 51988d1..5d887e7 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
@@ -34,7 +34,6 @@
import com.google.gwt.dev.jjs.impl.ArrayNormalizer;
import com.google.gwt.dev.jjs.impl.BuildTypeMap;
import com.google.gwt.dev.jjs.impl.CastNormalizer;
-import com.google.gwt.dev.jjs.impl.CastOptimizer;
import com.google.gwt.dev.jjs.impl.CatchBlockNormalizer;
import com.google.gwt.dev.jjs.impl.CompoundAssignmentNormalizer;
import com.google.gwt.dev.jjs.impl.DeadCodeElimination;
@@ -336,14 +335,12 @@
// - params based on assignment and call sites
// - method bodies based on return statements
// - polymorphic methods based on return types of all implementors
+ // - optimize casts and instanceof
didChange = TypeTightener.exec(jprogram) || didChange;
// tighten method call bindings
didChange = MethodCallTightener.exec(jprogram) || didChange;
- // remove unnecessary casts / optimize instanceof
- didChange = CastOptimizer.exec(jprogram) || didChange;
-
// dead code removal??
didChange = DeadCodeElimination.exec(jprogram) || didChange;
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JCastOperation.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JCastOperation.java
index 2adf61d..417a3e1 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JCastOperation.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JCastOperation.java
@@ -43,10 +43,11 @@
}
public boolean hasSideEffects() {
- // technically this isn't true, but since the same cast on the same
- // expression always evaluates the same way, it effectively has no side
- // effects
- return getExpr().hasSideEffects();
+ // Any live cast operations might throw a ClassCastException
+ //
+ // TODO: revisit this when we support the concept of whether something
+ // can/must complete normally!
+ return true;
}
public void traverse(JVisitor visitor, Context ctx) {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java
index 7368151..0a9f81d 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java
@@ -155,8 +155,9 @@
// @Override
public void endVisit(JCastOperation x, Context ctx) {
- assert (x.getCastType() != program.getTypeNull());
- recordCast(x.getCastType(), x.getExpr());
+ if (x.getCastType() != program.getTypeNull()) {
+ recordCast(x.getCastType(), x.getExpr());
+ }
}
// @Override
@@ -351,7 +352,23 @@
public void endVisit(JCastOperation x, Context ctx) {
JExpression replaceExpr;
JType toType = x.getCastType();
- if (toType instanceof JReferenceType) {
+ if (toType instanceof JNullType) {
+ /*
+ * Magic: a null type cast means the user tried a cast that couldn't
+ * possibly work. Typically this means either the statically resolvable
+ * arg type is incompatible with the target type, or the target type was
+ * globally uninstantiable. We handle this cast by throwing a
+ * ClassCastException, unless the argument is null.
+ */
+ JMethod method = program.getSpecialMethod("Cast.throwClassCastExceptionUnlessNull");
+ /*
+ * Override the type of the magic method with the null type.
+ */
+ JMethodCall call = new JMethodCall(program, x.getSourceInfo(), null,
+ method, program.getTypeNull());
+ call.getArgs().add(x.getExpr());
+ replaceExpr = call;
+ } else if (toType instanceof JReferenceType) {
JExpression curExpr = x.getExpr();
JReferenceType refType = (JReferenceType) toType;
JType argType = x.getExpr().getType();
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/CastOptimizer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/CastOptimizer.java
deleted file mode 100644
index 517e037..0000000
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/CastOptimizer.java
+++ /dev/null
@@ -1,138 +0,0 @@
-/*
- * Copyright 2006 Google Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License"); you may not
- * use this file except in compliance with the License. You may obtain a copy of
- * the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
- * License for the specific language governing permissions and limitations under
- * the License.
- */
-package com.google.gwt.dev.jjs.impl;
-
-import com.google.gwt.dev.jjs.ast.Context;
-import com.google.gwt.dev.jjs.ast.JBinaryOperation;
-import com.google.gwt.dev.jjs.ast.JBinaryOperator;
-import com.google.gwt.dev.jjs.ast.JCastOperation;
-import com.google.gwt.dev.jjs.ast.JInstanceOf;
-import com.google.gwt.dev.jjs.ast.JMethod;
-import com.google.gwt.dev.jjs.ast.JMethodCall;
-import com.google.gwt.dev.jjs.ast.JModVisitor;
-import com.google.gwt.dev.jjs.ast.JNullLiteral;
-import com.google.gwt.dev.jjs.ast.JProgram;
-import com.google.gwt.dev.jjs.ast.JReferenceType;
-import com.google.gwt.dev.jjs.ast.JType;
-import com.google.gwt.dev.jjs.ast.JTypeOracle;
-
-/**
- * Optimizer that will remove all trivially computable casts and instanceof
- * operations.
- */
-public class CastOptimizer {
-
- /**
- * Replaces all trivially computable casts and instanceof operations.
- */
- private class ReplaceTrivialCastsVisitor extends JModVisitor {
-
- // @Override
- public void endVisit(JCastOperation x, Context ctx) {
- JType argType = x.getExpr().getType();
- if (!(x.getCastType() instanceof JReferenceType)
- || !(argType instanceof JReferenceType)) {
- return;
- }
-
- JReferenceType toType = (JReferenceType) x.getCastType();
- JReferenceType fromType = (JReferenceType) argType;
-
- boolean triviallyTrue = false;
- boolean triviallyFalse = false;
-
- JTypeOracle typeOracle = program.typeOracle;
- if (typeOracle.canTriviallyCast(fromType, toType)) {
- triviallyTrue = true;
- } else if (!typeOracle.isInstantiatedType(toType)) {
- triviallyFalse = true;
- } else if (!typeOracle.canTheoreticallyCast(fromType, toType)) {
- triviallyFalse = true;
- }
-
- if (triviallyTrue) {
- // remove the cast operation
- ctx.replaceMe(x.getExpr());
- } else if (triviallyFalse) {
- // throw a ClassCastException unless the argument is null
- JMethod method = program.getSpecialMethod("Cast.throwClassCastExceptionUnlessNull");
- /*
- * Override the type of the called method with the null type. Null flow
- * will proceedeth forth from this cast operation. Assuredly, if the
- * call completes normally it will return null.
- */
- JMethodCall call = new JMethodCall(program, x.getSourceInfo(), null,
- method, program.getTypeNull());
- call.getArgs().add(x.getExpr());
- ctx.replaceMe(call);
- }
- }
-
- // @Override
- public void endVisit(JInstanceOf x, Context ctx) {
- JType argType = x.getExpr().getType();
- if (!(argType instanceof JReferenceType)) {
- return;
- }
-
- JReferenceType toType = x.getTestType();
- JReferenceType fromType = (JReferenceType) argType;
-
- boolean triviallyTrue = false;
- boolean triviallyFalse = false;
-
- JTypeOracle typeOracle = program.typeOracle;
- if (fromType == program.getTypeNull()) {
- // null is never instanceOf anything
- triviallyFalse = true;
- } else if (typeOracle.canTriviallyCast(fromType, toType)) {
- triviallyTrue = true;
- } else if (!typeOracle.isInstantiatedType(toType)) {
- triviallyFalse = true;
- } else if (!typeOracle.canTheoreticallyCast(fromType, toType)) {
- triviallyFalse = true;
- }
-
- if (triviallyTrue) {
- // replace with a simple null test
- JNullLiteral nullLit = program.getLiteralNull();
- JBinaryOperation neq = new JBinaryOperation(program, x.getSourceInfo(),
- program.getTypePrimitiveBoolean(), JBinaryOperator.NEQ,
- x.getExpr(), nullLit);
- ctx.replaceMe(neq);
- } else if (triviallyFalse) {
- // replace with a false literal
- ctx.replaceMe(program.getLiteralBoolean(false));
- }
- }
- }
-
- public static boolean exec(JProgram program) {
- return new CastOptimizer(program).execImpl();
- }
-
- private final JProgram program;
-
- private CastOptimizer(JProgram program) {
- this.program = program;
- }
-
- private boolean execImpl() {
- ReplaceTrivialCastsVisitor replacer = new ReplaceTrivialCastsVisitor();
- replacer.accept(program);
- return replacer.didChange();
- }
-}
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 9ecd657..4e13025 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
@@ -19,10 +19,13 @@
import com.google.gwt.dev.jjs.ast.JArrayRef;
import com.google.gwt.dev.jjs.ast.JArrayType;
import com.google.gwt.dev.jjs.ast.JBinaryOperation;
+import com.google.gwt.dev.jjs.ast.JBinaryOperator;
+import com.google.gwt.dev.jjs.ast.JCastOperation;
import com.google.gwt.dev.jjs.ast.JClassType;
import com.google.gwt.dev.jjs.ast.JExpression;
import com.google.gwt.dev.jjs.ast.JField;
import com.google.gwt.dev.jjs.ast.JFieldRef;
+import com.google.gwt.dev.jjs.ast.JInstanceOf;
import com.google.gwt.dev.jjs.ast.JInterfaceType;
import com.google.gwt.dev.jjs.ast.JLocal;
import com.google.gwt.dev.jjs.ast.JLocalDeclarationStatement;
@@ -31,6 +34,7 @@
import com.google.gwt.dev.jjs.ast.JMethodCall;
import com.google.gwt.dev.jjs.ast.JModVisitor;
import com.google.gwt.dev.jjs.ast.JNewArray;
+import com.google.gwt.dev.jjs.ast.JNullLiteral;
import com.google.gwt.dev.jjs.ast.JNullType;
import com.google.gwt.dev.jjs.ast.JParameter;
import com.google.gwt.dev.jjs.ast.JParameterRef;
@@ -39,6 +43,7 @@
import com.google.gwt.dev.jjs.ast.JReturnStatement;
import com.google.gwt.dev.jjs.ast.JTryStatement;
import com.google.gwt.dev.jjs.ast.JType;
+import com.google.gwt.dev.jjs.ast.JTypeOracle;
import com.google.gwt.dev.jjs.ast.JVariable;
import com.google.gwt.dev.jjs.ast.JVariableRef;
import com.google.gwt.dev.jjs.ast.JVisitor;
@@ -58,7 +63,7 @@
* The purpose of this pass is to record "type flow" information and then use
* the information to infer places where "tighter" (that is, more specific)
* types can be inferred for locals, fields, parameters, and method return
- * types.
+ * types. We also optimize dynamic casts and instanceof operations.
*
* Type flow occurs automatically in most JExpressions. But locals, fields,
* parameters, and method return types serve as "way points" where type
@@ -162,6 +167,7 @@
}
}
}
+
/**
* Record "type flow" information. Variables receive type flow via assignment.
* As a special case, Parameters also receive type flow based on the types of
@@ -370,16 +376,51 @@
* to change the declared type of a field, local, parameter, or method to a
* more specific type.
*
- * We must iterate mutiple times because each way point we tighten creates
- * more opportunities to do additional tightening for the things that depend
- * on it.
+ * Also optimize dynamic casts and instanceof operations where possible.
*/
- public class TightenTypesVisitor extends JVisitor {
+ public class TightenTypesVisitor extends JModVisitor {
- private boolean didChange = false;
+ /**
+ * <code>true</code> if this visitor has changed the AST apart from calls
+ * to Context.
+ */
+ private boolean myDidChange = false;
public boolean didChange() {
- return didChange;
+ return myDidChange || super.didChange();
+ }
+
+ public void endVisit(JCastOperation x, Context ctx) {
+ JType argType = x.getExpr().getType();
+ if (!(x.getCastType() instanceof JReferenceType)
+ || !(argType instanceof JReferenceType)) {
+ return;
+ }
+
+ JReferenceType toType = (JReferenceType) x.getCastType();
+ JReferenceType fromType = (JReferenceType) argType;
+
+ boolean triviallyTrue = false;
+ boolean triviallyFalse = false;
+
+ JTypeOracle typeOracle = program.typeOracle;
+ if (typeOracle.canTriviallyCast(fromType, toType)) {
+ triviallyTrue = true;
+ } else if (!typeOracle.isInstantiatedType(toType)) {
+ triviallyFalse = true;
+ } else if (!typeOracle.canTheoreticallyCast(fromType, toType)) {
+ triviallyFalse = true;
+ }
+
+ if (triviallyTrue) {
+ // remove the cast operation
+ ctx.replaceMe(x.getExpr());
+ } else if (triviallyFalse) {
+ // replace with a magic NULL cast
+ JCastOperation newOp = new JCastOperation(program, x.getSourceInfo(),
+ program.getTypeNull(), x.getExpr());
+ ctx.replaceMe(newOp);
+ }
}
// @Override
@@ -388,6 +429,45 @@
}
// @Override
+ public void endVisit(JInstanceOf x, Context ctx) {
+ JType argType = x.getExpr().getType();
+ if (!(argType instanceof JReferenceType)) {
+ // TODO: is this even possible? Replace with assert maybe.
+ return;
+ }
+
+ JReferenceType toType = x.getTestType();
+ JReferenceType fromType = (JReferenceType) argType;
+
+ boolean triviallyTrue = false;
+ boolean triviallyFalse = false;
+
+ JTypeOracle typeOracle = program.typeOracle;
+ if (fromType == program.getTypeNull()) {
+ // null is never instanceOf anything
+ triviallyFalse = true;
+ } else if (typeOracle.canTriviallyCast(fromType, toType)) {
+ triviallyTrue = true;
+ } else if (!typeOracle.isInstantiatedType(toType)) {
+ triviallyFalse = true;
+ } else if (!typeOracle.canTheoreticallyCast(fromType, toType)) {
+ triviallyFalse = true;
+ }
+
+ if (triviallyTrue) {
+ // replace with a simple null test
+ JNullLiteral nullLit = program.getLiteralNull();
+ JBinaryOperation neq = new JBinaryOperation(program, x.getSourceInfo(),
+ program.getTypePrimitiveBoolean(), JBinaryOperator.NEQ,
+ x.getExpr(), nullLit);
+ ctx.replaceMe(neq);
+ } else if (triviallyFalse) {
+ // replace with a false literal
+ ctx.replaceMe(program.getLiteralBoolean(false));
+ }
+ }
+
+ // @Override
public void endVisit(JLocal x, Context ctx) {
tighten(x);
}
@@ -410,7 +490,7 @@
// tighten based on non-instantiability
if (!program.typeOracle.isInstantiatedType(refType)) {
x.setType(typeNull);
- didChange = true;
+ myDidChange = true;
return;
}
@@ -443,7 +523,7 @@
resultType = program.strongerType(refType, resultType);
if (refType != resultType) {
x.setType(resultType);
- didChange = true;
+ myDidChange = true;
}
}
@@ -456,7 +536,7 @@
if (!program.typeOracle.isInstantiatedType((JReferenceType) leafType)) {
arrayType = program.getTypeArray(typeNull, arrayType.getDims());
x.setType(arrayType);
- didChange = true;
+ myDidChange = true;
}
}
}
@@ -501,7 +581,7 @@
// tighten based on non-instantiability
if (!program.typeOracle.isInstantiatedType(refType)) {
x.setType(typeNull);
- didChange = true;
+ myDidChange = true;
return;
}
@@ -551,7 +631,7 @@
resultType = program.strongerType(refType, resultType);
if (refType != resultType) {
x.setType(resultType);
- didChange = true;
+ myDidChange = true;
}
}
}
@@ -586,6 +666,12 @@
private boolean execImpl() {
RecordVisitor recorder = new RecordVisitor();
recorder.accept(program);
+
+ /*
+ * We must iterate mutiple times because each way point we tighten creates
+ * more opportunities to do additional tightening for the things that depend
+ * on it.
+ */
boolean madeChanges = false;
while (true) {
TightenTypesVisitor tightener = new TightenTypesVisitor();
