EnumOrdinalizer cleanup, and fix ICE on empty enums. - Change to MethodInliner removes spurious casts and the need for special-case code in EnumOrdinalizer. - Fix a case in EnumOrdinalizer were we'd try to modify an immutable list of statements. - General clean-up of EnumOrdinalizer. - TypeRemapper correctly records mutations. - Removed a few of the JavaAstConstructor changes that were put in for EnumOrdinalizer as they aren't really needed. What's left is a closer model to the real java.lang.Enum as it uses native methods. http://gwt-code-reviews.appspot.com/1426804/ http://gwt-code-reviews.appspot.com/1423809/ Patch by: me, jbrosenberg Review by: jbrosenberg, me git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10101 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java index 7cdbdc5..0ad3931 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java
@@ -19,15 +19,18 @@ import com.google.gwt.dev.jjs.SourceInfo; import com.google.gwt.dev.jjs.ast.Context; import com.google.gwt.dev.jjs.ast.JArrayType; +import com.google.gwt.dev.jjs.ast.JBlock; import com.google.gwt.dev.jjs.ast.JCastOperation; import com.google.gwt.dev.jjs.ast.JClassLiteral; import com.google.gwt.dev.jjs.ast.JClassType; import com.google.gwt.dev.jjs.ast.JDeclarationStatement; +import com.google.gwt.dev.jjs.ast.JDeclaredType; import com.google.gwt.dev.jjs.ast.JEnumField; import com.google.gwt.dev.jjs.ast.JEnumType; 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.JMethod; import com.google.gwt.dev.jjs.ast.JMethodBody; import com.google.gwt.dev.jjs.ast.JMethodCall; @@ -51,11 +54,9 @@ import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.Stack; import java.util.TreeSet; /** @@ -333,7 +334,6 @@ */ private class CannotBeOrdinalAnalyzer extends ImplicitUpcastAnalyzer { - private final Stack<JCastOperation> castOpsToIgnore = new Stack<JCastOperation>(); private final Map<String, SourceInfo> jsniClassLiteralsInfo = new HashMap<String, SourceInfo>(); public CannotBeOrdinalAnalyzer(JProgram program) { @@ -356,12 +356,6 @@ @Override public void endVisit(JCastOperation x, Context ctx) { - // see if we've previously marked this castOp to be exempted - if (!castOpsToIgnore.empty() && castOpsToIgnore.peek() == x) { - castOpsToIgnore.pop(); - return; - } - // check for explicit cast (check both directions) blackListIfEnumCast(x.getExpr().getType(), x.getCastType(), x.getSourceInfo()); blackListIfEnumCast(x.getCastType(), x.getExpr().getType(), x.getSourceInfo()); @@ -450,6 +444,12 @@ } @Override + public void endVisit(JInstanceOf x, Context ctx) { + // If any instanceof tests haven't been optimized out, black list. + blackListIfEnum(x.getTestType(), x.getSourceInfo()); + } + + @Override public void endVisit(JMethodCall x, Context ctx) { // exempt calls to certain methods on the Enum super class if (x.getTarget() == enumCreateValueOfMapMethod || x.getTarget() == enumSuperConstructor @@ -541,37 +541,6 @@ } @Override - public boolean visit(JFieldRef x, Context ctx) { - /* - * If we have a field ref of Enum.ordinal, then we want to allow a cast - * operation from enum subclass to Enum on the instance. Other optimizers - * have a tendency to convert things like: - * - * 'switch(enumObj)' to 'switch((Enum)enumObj).ordinal' - * - * We don't want to blacklist enumObj in that case, so we push this castOp - * on a stack and check it in the subsequent call to endVisit for - * JCastOperation. We can't simply return false and prevent the visit of - * the JCastOperation altogether, since we do need to visit the - * JCastOperation's sub-expression. Since the sub-expression could - * potentially also contain similar cast operations, we use a stack to - * keep track of 'castOpsToIgnore'. - */ - if (x.getField() == enumOrdinalField) { - if (x.getInstance() != null && x.getInstance() instanceof JCastOperation) { - JCastOperation castOp = (JCastOperation) x.getInstance(); - if (getPossiblyUnderlyingType(castOp.getCastType()) == enumType) { - JEnumType fromType = getEnumType(castOp.getExpr().getType()); - if (fromType != null) { - castOpsToIgnore.push(castOp); - } - } - } - } - return true; - } - - @Override public boolean visit(JMethod x, Context ctx) { /* * Don't want to visit the generated getClass() method on an enum, since @@ -600,32 +569,6 @@ if (x.getTarget() == enumCreateValueOfMapMethod) { return false; } - - /* - * If we have a method call of Enum.ordinal(), then we want to allow a - * cast operation from enum subclass to Enum on the instance. Other - * optimizers have a tendency to convert things like: - * - * 'switch(enumObj.ordinal())' to 'switch((Enum)enumObj).ordinal' - * - * We don't want to blacklist enumObj in that case, so we push this castOp - * on a stack and and check it in the subsequent call to endVisit for - * JCastOperation (above). We can't simply return false and prevent the - * visit of the JCastOperation altogether, since we do need to visit the - * castOperation's sub-expression. - */ - if (x.getTarget() == enumOrdinalMethod) { - if (x.getInstance() != null && x.getInstance() instanceof JCastOperation) { - JCastOperation castOp = (JCastOperation) x.getInstance(); - if (getPossiblyUnderlyingType(castOp.getCastType()) == enumType) { - JEnumType fromType = getEnumType(castOp.getExpr().getType()); - if (fromType != null) { - castOpsToIgnore.push(castOp); - } - } - } - } - // ok to visit return true; } @@ -720,11 +663,11 @@ return true; } + /** + * Replace an enum field constant, with it's integer valued ordinal. + */ @Override public boolean visit(JField x, Context ctx) { - /* - * Replace an enum field constant, with it's integer valued ordinal. - */ if (x instanceof JEnumField && canBeOrdinal(x.getEnclosingType())) { int ordinal = ((JEnumField) x).ordinal(); x.setInitializer(new JDeclarationStatement(x.getSourceInfo(), new JFieldRef(x @@ -733,11 +676,11 @@ return true; } + /** + * Replace an enum field ref with it's integer valued ordinal. + */ @Override public boolean visit(JFieldRef x, Context ctx) { - /* - * Replace an enum field ref with it's integer valued ordinal. - */ JField field = x.getField(); if (field instanceof JEnumField && canBeOrdinal(field.getEnclosingType())) { int ordinal = ((JEnumField) field).ordinal(); @@ -746,7 +689,7 @@ return true; } - /* + /** * Remap enum types with JPrimitiveType.INT. Also handle case for arrays, * replace enum leaftype with JPrimitive.INT. This is an override * implementation called from TypeRemapper. @@ -785,44 +728,82 @@ return null; } - /* + /** * Remove initialization of enum constants, and the $VALUES array, in the * clinit for an ordinalizable enum. */ private void updateClinit(JMethod method) { - List<JStatement> stmts = ((JMethodBody) method.getBody()).getStatements(); - Iterator<JStatement> it = stmts.iterator(); - // look for statements of the form EnumValueField = ... - while (it.hasNext()) { - JStatement stmt = it.next(); + assert JProgram.isClinit(method); + JDeclaredType enclosingType = method.getEnclosingType(); + JBlock block = ((JMethodBody) method.getBody()).getBlock(); + int removeIndex = 0; + // Make a copy to avoid concurrent modification. + for (JStatement stmt : new ArrayList<JStatement>(block.getStatements())) { if (stmt instanceof JDeclarationStatement) { JVariableRef ref = ((JDeclarationStatement) stmt).getVariableRef(); if (ref instanceof JFieldRef) { JFieldRef enumRef = (JFieldRef) ref; - if (enumRef.getField().getEnclosingType() == method.getEnclosingType()) { - // see if LHS is a field ref that corresponds to the class of this - // method - if (enumRef.getField() instanceof JEnumField) { - it.remove(); - } else if (enumRef.getField().getName().equals("$VALUES")) { - it.remove(); + // See if LHS is a field ref to the class being initialized. + JField field = enumRef.getField(); + if (field.isStatic() && field.getEnclosingType() == enclosingType) { + if (field instanceof JEnumField || field.getName().equals("$VALUES")) { + block.removeStmt(removeIndex--); + field.setInitializer(null); } } } } + ++removeIndex; } } } /** - * A visitor which will replace references to the ordinal field and ordinal - * method refs with the appropropriate ordinal integer value. + * Any references to the {@link Enum#ordinal()} method and + * {@link Enum#ordinal} field for ordinalized types are replaced with the + * qualifying instance. * * Note, this visitor must run after the ReplaceEnumTypesWithInteger visitor, * since it depends on detecting the locations where the enumOrdinalField or * enumOrdinalMethod have had their types changed to integer. */ private class ReplaceOrdinalFieldAndMethodRefsWithOrdinal extends JModVisitor { + /** + * Replace any references to Enum.ordinal field with the qualifying + * instance, if that instance has been ordinalized (e.g. replace + * <code>4.ordinal</code> with <code>4</code>). + */ + @Override + public void endVisit(JFieldRef x, Context ctx) { + super.endVisit(x, ctx); + if (x.getField() == enumOrdinalField) { + if (x.getInstance() != null) { + JType type = x.getInstance().getType(); + if (type == JPrimitiveType.INT) { + ctx.replaceMe(x.getInstance()); + } + } + } + } + + /** + * Replace any references to Enum.ordinal() with the qualifying instance, if + * that instance has been ordinalized (e.g. replace <code>4.ordinal()</code> + * with <code>4</code>). + */ + @Override + public void endVisit(JMethodCall x, Context ctx) { + super.endVisit(x, ctx); + if (x.getTarget() == enumOrdinalMethod) { + if (x.getInstance() != null) { + JType type = x.getInstance().getType(); + if (type == JPrimitiveType.INT) { + ctx.replaceMe(x.getInstance()); + } + } + } + } + @Override public boolean visit(JClassType x, Context ctx) { // don't waste time visiting the large ClassLiteralHolder class @@ -831,83 +812,9 @@ } return true; } - - @Override - public boolean visit(JFieldRef x, Context ctx) { - /* - * Check field refs that refer to Enum.ordinal, but which have already - * been ordinalized by the previous pass. This is implemented with visit - * instead of endVisit since, in the case of an underlying cast operation, - * we don't want to traverse the instance expression before we replace it. - */ - if (x.getField() == enumOrdinalField) { - if (x.getInstance() != null) { - JType type = x.getInstance().getType(); - if (type == JPrimitiveType.INT) { - /* - * See if this fieldRef was converted to JPrimitiveType.INT, but - * still points to the Enum.ordinal field. If so, replace the field - * ref with the ordinalized int itself. - */ - ctx.replaceMe(x.getInstance()); - } else if (x.getInstance() instanceof JCastOperation) { - /* - * See if this reference to Enum.ordinal is via a cast from an enum - * sub-class, that we've already ordinalized to JPrimitiveType.INT. - * If so, replace the whole cast operation. (see JFieldRef visit - * method in CannotBeOrdinalAnalyzer above). - */ - JCastOperation castOp = (JCastOperation) x.getInstance(); - if (getPossiblyUnderlyingType(castOp.getType()) == enumType) { - if (castOp.getExpr().getType() == JPrimitiveType.INT) { - ctx.replaceMe(castOp.getExpr()); - } - } - } - } - } - return true; - } - - @Override - public boolean visit(JMethodCall x, Context ctx) { - /* - * See if this methodCall was converted to JPrimitiveType.INT, but still - * points to the Enum.ordinal() method. If so, replace the method call - * with the ordinal expression itself. Implement with visit (and not - * endVisit), so we don't traverse the method call itself unnecessarily. - */ - if (x.getTarget() == enumOrdinalMethod) { - if (x.getInstance() != null) { - JType type = x.getInstance().getType(); - if (type == JPrimitiveType.INT) { - /* - * See if this instance was converted to JPrimitiveType.INT, but - * still points to the Enum.ordinal() method. If so, replace the - * method call with the ordinalized int itself. - */ - ctx.replaceMe(x.getInstance()); - } else if (x.getInstance() instanceof JCastOperation) { - /* - * See if this reference to Enum.ordinal() is via a cast from an - * enum sub-class, that we've already ordinalized to - * JPrimitiveType.INT. If so, replace the whole cast operation. (see - * JMethodCall visit method in CannotBeOrdinalAnalyzer above). - */ - JCastOperation castOp = (JCastOperation) x.getInstance(); - if (getPossiblyUnderlyingType(castOp.getType()) == enumType) { - if (castOp.getExpr().getType() == JPrimitiveType.INT) { - ctx.replaceMe(castOp.getExpr()); - } - } - } - } - } - return true; - } } - public static final String NAME = EnumOrdinalizer.class.getSimpleName(); + private static final String NAME = EnumOrdinalizer.class.getSimpleName(); private static Tracker tracker = null; @@ -951,14 +858,9 @@ private final JMethod enumOrdinalMethod; private final JMethod enumSuperConstructor; private final Map<String, JEnumType> enumsVisited = new HashMap<String, JEnumType>(); - private final JType enumType; - private final JType javaScriptObjectType; - private final JType nullType; - private final Set<JEnumType> ordinalizationBlackList = new HashSet<JEnumType>(); - private final JProgram program; public EnumOrdinalizer(JProgram program) { @@ -966,7 +868,6 @@ this.classLiteralHolderType = program.getIndexedType("ClassLiteralHolder"); this.nullType = program.getTypeNull(); this.javaScriptObjectType = program.getJavaScriptObject(); - this.enumType = program.getIndexedType("Enum"); this.enumOrdinalField = program.getIndexedField("Enum.ordinal"); this.classCreateForEnumMethod = program.getIndexedMethod("Class.createForEnum"); this.enumCreateValueOfMapMethod = program.getIndexedMethod("Enum.createValueOfMap"); @@ -1019,7 +920,6 @@ if (tracker != null) { tracker.maybeDumpAST(program, 2); } - return stats; }
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 0195ae8..92eb2bb 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
@@ -30,6 +30,7 @@ import com.google.gwt.dev.jjs.ast.JParameter; import com.google.gwt.dev.jjs.ast.JParameterRef; import com.google.gwt.dev.jjs.ast.JProgram; +import com.google.gwt.dev.jjs.ast.JReferenceType; import com.google.gwt.dev.jjs.ast.JReturnStatement; import com.google.gwt.dev.jjs.ast.JStatement; import com.google.gwt.dev.jjs.ast.JThisRef; @@ -505,18 +506,8 @@ return stats; } - /** - * Insert an implicit cast if the types differ; it might get optimized out - * later, but in some cases it will force correct math evaluation. - */ - private static JExpression maybeCast(JExpression exp, JType targetType) { - if (exp.getType() != targetType) { - exp = new JCastOperation(exp.getSourceInfo(), targetType, exp); - } - return exp; - } - private JMethod currentMethod; + private final JProgram program; private MethodInliner(JProgram program) { @@ -541,4 +532,32 @@ } return stats; } + + /** + * Insert an implicit cast if the types differ; it might get optimized out + * later, but in some cases it will force correct math evaluation. + */ + private JExpression maybeCast(JExpression exp, JType targetType) { + if (targetType instanceof JReferenceType) { + assert exp.getType() instanceof JReferenceType; + targetType = merge((JReferenceType) exp.getType(), (JReferenceType) targetType); + } + if (!program.typeOracle.canTriviallyCast(exp.getType(), targetType)) { + exp = new JCastOperation(exp.getSourceInfo(), targetType, exp); + } + return exp; + } + + private JReferenceType merge(JReferenceType source, JReferenceType target) { + JReferenceType result; + if (program.typeOracle.canTriviallyCast(source.getUnderlyingType(), target.getUnderlyingType())) { + result = source; + } else { + result = target; + } + if ((!target.canBeNull())) { + result = result.getNonNull(); + } + return result; + } }
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/TypeRemapper.java b/dev/core/src/com/google/gwt/dev/jjs/impl/TypeRemapper.java index ef5bf10..c8f383b 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/TypeRemapper.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/TypeRemapper.java
@@ -29,13 +29,11 @@ import com.google.gwt.dev.jjs.ast.JVariable; /** - * This class is a visitor which can find all sites where a JType can be updated - * from one type to another, and calls an overridable remap method for each - * instance. An extending class can override the remap and return a new type - * where it deems it necessary, such as to replace the type of all references of - * a class. + * A visitor that changes all JType references in the AST. Subclasses override + * {@link #remap(JType)} to replace all occurrences of one or more types with + * different types. */ -public class TypeRemapper extends JModVisitor { +public abstract class TypeRemapper extends JModVisitor { @Override public void endVisit(JBinaryOperation x, Context ctx) { @@ -44,8 +42,11 @@ @Override public void endVisit(JCastOperation x, Context ctx) { - // JCastOperation doesn't have a settable castType method, so need to - // create a new one and do a replacement. + /* + * JCastOperation doesn't have a settable castType method, so need to create + * a new one and do a replacement. Use remap() instead of modRemap() since + * the ctx.replaceMe() will record a change. + */ JType remapCastType = remap(x.getCastType()); if (remapCastType != x.getCastType()) { JCastOperation newX = new JCastOperation(x.getSourceInfo(), remapCastType, x.getExpr()); @@ -55,41 +56,47 @@ @Override public void endVisit(JConditional x, Context ctx) { - x.setType(remap(x.getType())); + x.setType(modRemap(x.getType())); } @Override public void endVisit(JConstructor x, Context ctx) { - x.setType(remap(x.getType())); + x.setType(modRemap(x.getType())); } @Override public void endVisit(JGwtCreate x, Context ctx) { - x.setType(remap(x.getType())); + x.setType(modRemap(x.getType())); } @Override public void endVisit(JMethod x, Context ctx) { - x.setType(remap(x.getType())); + x.setType(modRemap(x.getType())); } @Override public void endVisit(JNewArray x, Context ctx) { - x.setType((JArrayType) remap(x.getArrayType())); + x.setType((JArrayType) modRemap(x.getArrayType())); } @Override public void endVisit(JVariable x, Context ctx) { - x.setType(remap(x.getType())); + x.setType(modRemap(x.getType())); } /** - * An overriding method will be called for each detected JType element. + * Override to return a possibly-different type. * - * @param type + * @param type the original type + * @return a replacement type, which may be the original type */ - protected JType remap(JType type) { - // override to possibly return an different type - return type; + protected abstract JType remap(JType type); + + private JType modRemap(JType type) { + JType result = remap(type); + if (result != type) { + madeChanges(); + } + return result; } }
diff --git a/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java b/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java index 97d8883..cf265c8 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java +++ b/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java
@@ -84,37 +84,18 @@ "java.lang.Class") { @Override public CharSequence getContent() { - // This has extra code in the createForEnum method, to keep it from being inlined StringBuffer code = new StringBuffer(); code.append("package java.lang;\n"); code.append("import com.google.gwt.core.client.JavaScriptObject;\n"); code.append("public final class Class<T> {\n"); - code.append(" static <T> Class<T> createForArray(String packageName,\n"); - code.append(" String className, String seedName, Class<?> componentType) {\n"); - code.append(" return new Class<T>(); }\n"); - code.append(" static <T> Class<T> createForClass(String packageName,\n"); - code.append(" String className, String seedName, Class<? super T> superclass) {\n"); - code.append(" return new Class<T>(); }\n"); - code.append(" static <T> Class<T> createForEnum(String packageName,\n"); - code.append(" String className, String seedName, Class<? super T> superclass,\n"); - code.append(" JavaScriptObject enumConstantsFunc, JavaScriptObject enumValueOfFunc) {\n"); - code.append(" Class<T> newClass = new Class<T>();\n"); - code.append(" newClass.className = className;\n"); - code.append(" newClass.packageName = packageName;\n"); - code.append(" newClass.seedName = seedName;\n"); - code.append(" return newClass;}\n"); - code.append(" static <T> Class<T> createForInterface(String packageName, String className) {\n"); - code.append(" return new Class<T>(); }\n"); - code.append(" static <T> Class<T> createForPrimitive(String packageName,\n"); - code.append(" String className, String jni) {\n"); - code.append(" return new Class<T>(); }\n"); + code.append(" static <T> Class<T> createForArray(String packageName, String className, String seedName, Class<?> componentType) { return new Class<T>(); }\n"); + code.append(" static <T> Class<T> createForClass(String packageName, String className, String seedName, Class<? super T> superclass) { return new Class<T>(); }\n"); + code.append(" static <T> Class<T> createForEnum(String packageName, String className, String seedName, Class<? super T> superclass, JavaScriptObject enumConstantsFunc, JavaScriptObject enumValueOfFunc) { return new Class<T>(); }\n"); + code.append(" static <T> Class<T> createForInterface(String packageName, String className) { return new Class<T>(); }\n"); + code.append(" static <T> Class<T> createForPrimitive(String packageName, String className, String jni) { return new Class<T>(); }\n"); code.append(" static boolean isClassMetadataEnabled() { return true; }\n"); code.append(" public boolean desiredAssertionStatus() { return true; }\n"); - code.append(" public String getName() { return className; }\n"); - code.append(" public T[] getEnumConstants() { return null; }\n"); - code.append(" private String packageName = null;"); - code.append(" private String className = null;"); - code.append(" private String seedName = null;"); + code.append(" public String getName() { return null; }\n"); code.append("}\n"); return code; } @@ -139,16 +120,12 @@ code.append("import java.io.Serializable;\n"); code.append("import com.google.gwt.core.client.JavaScriptObject;\n"); code.append("public abstract class Enum<E extends Enum<E>> implements Serializable {\n"); - code.append(" public static <T extends Enum<T>> T valueOf(Class<T> enumType, String name) { \n"); - code.append(" for (T enumConstant : enumType.getEnumConstants()) {\n"); - code.append(" if (enumConstant.name() != null) {\n"); - code.append(" return enumConstant;}}\n"); - code.append(" return null;}\n"); + code.append(" public static native <T extends Enum<T>> T valueOf(Class<T> enumType, String name) /*-{ }-*/;\n"); + code.append(" protected static native <T extends Enum<T>> JavaScriptObject createValueOfMap(T[] enumConstants) /*-{ }-*/;\n"); + code.append(" protected static native <T extends Enum<T>> T valueOf(JavaScriptObject map, String name) /*-{ }-*/;\n"); code.append(" protected Enum(String name, int ordinal) { \n"); code.append(" this.name = name;\n"); code.append(" this.ordinal = ordinal;}\n"); - code.append(" protected static <T extends Enum<T>> JavaScriptObject createValueOfMap(T[] enumConstants) { return null;}\n"); - code.append(" protected static <T extends Enum<T>> T valueOf(JavaScriptObject map, String name) { return null; }\n"); code.append(" private final String name;\n"); code.append(" private final int ordinal;\n"); code.append(" public final String name() { return name; }\n");
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java index b97648d..6cb1ab9 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java
@@ -166,7 +166,7 @@ EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker(); assertTrue(tracker.isOrdinalized("test.EntryPoint$Fruit")); } - + public void testOrdinalizeFieldRefOrdinalMethodCall() throws UnableToCompleteException { EnumOrdinalizer.resetTracker(); @@ -189,16 +189,31 @@ EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker(); assertTrue(tracker.isOrdinalized("test.EntryPoint$Fruit")); } - + + public void testOrdinalizeUnusedEmptyEnum() throws UnableToCompleteException { + EnumOrdinalizer.resetTracker(); + + setupEmptyEnum(); + + optimize("void", "EmptyEnum myEnum;"); + + EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker(); + assertTrue(tracker.isOrdinalized("test.EntryPoint$EmptyEnum")); + } + + public void testOrdinalizeUnusedEnum() throws UnableToCompleteException { + EnumOrdinalizer.resetTracker(); + + setupFruitEnum(); + + optimize("void", "Fruit myEnum;"); + + EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker(); + assertTrue(tracker.isOrdinalized("test.EntryPoint$Fruit")); + } + public void testOrdinalizeMethodCallExpressionOrdinalFieldRef() throws UnableToCompleteException { - /* - * the switch expression gets transformed to - * 'switch ((Enum) getResolvedFruit(fruit)).ordinal)' - * by the time the EnumOrdinalizer sees it. So this test is testing the - * logic to replace an ordinal field ref expression with the expression - * itself. - */ EnumOrdinalizer.resetTracker(); setupFruitEnum(); @@ -367,7 +382,14 @@ assertTrue(tracker.isVisited("test.EntryPoint$Fruit")); assertFalse(tracker.isOrdinalized("test.EntryPoint$Fruit")); } - + + public void testNotOrdinalizableInstanceOf() { + /* + * TODO(jbrosenberg): determine if this really needs to be true, and write a + * test to prove it. + */ + } + public void testNotOrdinalizableStaticFieldRef() throws UnableToCompleteException { EnumOrdinalizer.resetTracker(); @@ -795,12 +817,16 @@ assertTrue(tracker.isVisited("test.EntryPoint$Vegetable")); assertFalse(tracker.isOrdinalized("test.EntryPoint$Vegetable")); } - + + private void setupEmptyEnum() { + addSnippetClassDecl("public enum EmptyEnum {}"); + } + private void setupFruitEnum() { addSnippetClassDecl("public enum Fruit {APPLE, ORANGE}"); setupExtraDummyEnum(); } - + private void setupFruitEnumWithInstanceField() { addSnippetClassDecl("public enum Fruit {APPLE(\"a\"), ORANGE(\"b\");", " public final String instanceField;", @@ -921,4 +947,4 @@ return didChange; } -} +} \ No newline at end of file