Allows enum ordinalization to proceed for enums with static methods/fields
-- This offers modest code-size gains, and will open way for further optimizations
-- Also, fixes a bug in checking for instanceof arguments for EnumOrdinalizer
-- Fixed some formatting in EnumOrdinalizer
-- Added new tests to EnumOrdinalizerTest
Review at http://gwt-code-reviews.appspot.com/1428808
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10135 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JEnumType.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JEnumType.java
index d44f0cc..77e01cb 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JEnumType.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JEnumType.java
@@ -29,6 +29,7 @@
*/
private List<JEnumField> enumList = Lists.create();
+ private boolean isOrdinalized = false;
public JEnumType(SourceInfo info, String name, boolean isAbstract) {
super(info, name, isAbstract, false);
@@ -63,4 +64,12 @@
public JEnumType isEnumOrSubclass() {
return this;
}
+
+ public boolean isOrdinalized() {
+ return isOrdinalized;
+ }
+
+ public void setOrdinalized() {
+ isOrdinalized = true;
+ }
}
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 0ad3931..c0e5a90 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
@@ -71,9 +71,13 @@
* This optimizer modifies enum classes to change their field constants to ints,
* and to remove initialization of those constants in the clinit method. An
* ordinalized enum class will not be removed from the AST by this optimizer,
- * but as long as all references to it are replaced (which is one of the
- * requirements for ordinalization), then the enum class itself will be pruned
- * by subsequent optimizer passes.
+ * but as long as all references to it are replaced, then the enum class itself
+ * will be pruned by subsequent optimizer passes. Some enum classes may not be
+ * completely removed however. Ordinalization can proceed in cases where there
+ * are added static fields or methods in the enum class. In such cases a reduced
+ * version of the original enum class can remain in the AST, containing only
+ * static fields and methods which aren't part of the enum infrastructure (in
+ * which case it will no longer behave as an enum class at all).
*
* Regardless of whether an ordinalized enum class ends up being completely
* pruned away, the AST is expected to be in a coherent and usable state after
@@ -295,13 +299,29 @@
* does this by keeping track of a "black-list" for ordinals which violate the
* conditions for ordinalization, below.
*
- * An enum cannot be ordinalized, if it: is implicitly upcast. is implicitly
- * cast to from a nullType. is implicitly cast to or from a javaScriptObject
- * type. is explicitly cast to another type (or vice-versa). it's class
- * literal is used explicitly. it has an artificial rescue recorded for it.
- * has any field referenced, except for: one of it's enum constants
- * Enum.ordinal has any method called, except for: ordinal() Enum.ordinal()
- * Enum() super constructor Enum.createValueOfMap()
+ * An enum cannot be ordinalized, if it:
+ * <ul>
+ * <li>is implicitly upcast.</li>
+ * <li>is implicitly cast to from a nullType.</li>
+ * <li>is implicitly cast to or from a javaScriptObject type.</li>
+ * <li>is explicitly cast to another type (or vice-versa).</li>
+ * <li>is tested in an instanceof expression.</li>
+ * <li>it's class literal is used explicitly.</li>
+ * <li>it has an artificial rescue recorded for it.</li>
+ * <li>has any field referenced, except for:</li>
+ * <ul>
+ * <li>static fields, other than the synthetic $VALUES field.</li>
+ * <li>Enum.ordinal.</li>
+ * </ul>
+ * <li>has any method called, except for:</li>
+ * <ul>
+ * <li>ordinal().</li>
+ * <li>Enum.ordinal().</li>
+ * <li>Enum() super constructor.</li>
+ * <li>Enum.createValueOfMap().</li>
+ * <li>static methods, other than values() or valueOf().</li>
+ * </ul>
+ * </ul>
*
* This visitor extends the ImplicitUpcastAnalyzer, which encapsulates all the
* conditions where implicit upcasting can occur in an AST. The rest of the
@@ -394,6 +414,11 @@
JEnumType maybeEnum = x.isEnumOrSubclass();
if (maybeEnum != null) {
enumsVisited.put(program.getClassLiteralName(maybeEnum), maybeEnum);
+
+ // don't need to re-ordinalize a previously ordinalized enum
+ if (maybeEnum.isOrdinalized()) {
+ addToBlackList(maybeEnum, x.getSourceInfo());
+ }
}
}
@@ -408,44 +433,28 @@
// check any instance field reference other than ordinal
blackListIfEnumExpression(x.getInstance());
} else if (x.getField().isStatic()) {
- // check static field references
-
/*
- * Need to exempt static fieldRefs to the special $VALUES array that
- * gets generated for all enum classes, if the reference occurs within
- * the enum class itself (such as happens in the clinit() or values()
- * method for all enums).
+ * Black list if the $VALUES static field is referenced outside of an
+ * enum class. This can happen if there's a call to an enum's values()
+ * method, which then gets inlined.
+ *
+ * TODO (jbrosenberg): Investigate further whether referencing the
+ * $VALUES array (as well as the values() method) should not block
+ * ordinalization. Instead, convert $VALUES to an array of int.
*/
if (x.getField().getName().equals("$VALUES")
- && this.currentMethod.getEnclosingType() == x.getField().getEnclosingType()) {
- if (getEnumType(x.getField().getEnclosingType()) != null) {
- return;
- }
+ && this.currentMethod.getEnclosingType() != x.getField().getEnclosingType()) {
+ blackListIfEnum(x.getField().getEnclosingType(), x.getSourceInfo());
}
-
- /*
- * Need to exempt static fieldRefs for enum constants themselves. Detect
- * these as final fields, that have the same enum type as their
- * enclosing type.
- */
- if (x.getField().isFinal()
- && (x.getField().getEnclosingType() == getEnumType(x.getField().getType()))) {
- return;
- }
-
- /*
- * Check any other refs to static fields of an enum class. This includes
- * references to $VALUES that might occur outside of the enum class
- * itself. This can occur when a call to the values() method gets
- * inlined, etc. Also check here for any user defined static fields.
- */
- blackListIfEnum(x.getField().getEnclosingType(), x.getSourceInfo());
}
}
@Override
public void endVisit(JInstanceOf x, Context ctx) {
// If any instanceof tests haven't been optimized out, black list.
+ blackListIfEnum(x.getExpr().getType(), x.getSourceInfo());
+ // TODO (jbrosenberg): Investigate further whether ordinalization can be
+ // allowed in this case.
blackListIfEnum(x.getTestType(), x.getSourceInfo());
}
@@ -461,11 +470,10 @@
if (x.getInstance() != null) {
blackListIfEnumExpression(x.getInstance());
} else if (x.getTarget().isStatic()) {
- /*
- * need to exempt static methodCalls for an enum class if it occurs
- * within the enum class itself (such as in $clinit() or values())
- */
- if (this.currentMethod.getEnclosingType() != x.getTarget().getEnclosingType()) {
+ // black-list static method calls on an enum class only for valueOf()
+ // and values()
+ String methodName = x.getTarget().getName();
+ if (methodName.equals("valueOf") || methodName.equals("values")) {
blackListIfEnum(x.getTarget().getEnclosingType(), x.getSourceInfo());
}
}
@@ -888,17 +896,14 @@
ordinalAnalyzer.accept(program);
ordinalAnalyzer.afterVisitor();
- if (tracker != null) {
- for (JEnumType type : enumsVisited.values()) {
- tracker.addVisited(type.getName());
- if (!ordinalizationBlackList.contains(type)) {
- tracker.addOrdinalized(type.getName());
- }
- }
- }
-
// Bail if we don't need to do any ordinalization
if (enumsVisited.size() == ordinalizationBlackList.size()) {
+ // Update tracker stats
+ if (tracker != null) {
+ for (JEnumType type : enumsVisited.values()) {
+ tracker.addVisited(type.getName());
+ }
+ }
return stats;
}
@@ -920,6 +925,19 @@
if (tracker != null) {
tracker.maybeDumpAST(program, 2);
}
+
+ // Update enums ordinalized, and tracker stats
+ for (JEnumType type : enumsVisited.values()) {
+ if (tracker != null) {
+ tracker.addVisited(type.getName());
+ }
+ if (!ordinalizationBlackList.contains(type)) {
+ if (tracker != null) {
+ tracker.addOrdinalized(type.getName());
+ }
+ type.setOrdinalized();
+ }
+ }
return stats;
}
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 cf265c8..ad305cc 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java
@@ -72,10 +72,11 @@
code.append("package com.google.gwt.lang;\n");
code.append("public final class Cast {\n");
code.append(" public static Object dynamicCast(Object src, int dstId) { return src;}\n");
- code.append(" public static boolean isNull(Object a) { return false;}\n");
- code.append(" public static boolean isNotNull(Object a) { return false;}\n");
- code.append(" public static boolean jsEquals(Object a, Object b) { return false;}\n");
- code.append(" public static boolean jsNotEquals(Object a, Object b) { return false;}\n");
+ code.append(" public static boolean instanceOf(Object src, int dstId) { return false;}\n");
+ code.append(" public static native boolean isNull(Object a) /*-{ }-*/;\n");
+ code.append(" public static native boolean isNotNull(Object a) /*-{ }-*/;\n");
+ code.append(" public static native boolean jsEquals(Object a, Object b) /*-{ }-*/;\n");
+ code.append(" public static native boolean jsNotEquals(Object a, Object b) /*-{ }-*/;\n");
code.append("}\n");
return code;
}
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 6cb1ab9..34f0a84 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
@@ -43,12 +43,11 @@
super.setUp();
EnumOrdinalizer.enableTracker();
- // don't need to runDeadCodeElimination, it's after we're done anyway
- runDeadCodeElimination = false;
-
// defaults, can be overridden by individual test cases
runTypeTightener = false;
runMethodCallTightener = false;
+ runMethodInliner = true;
+ runMakeCallsStatic = true;
}
public void testOrdinalizeBasicAssignment()
@@ -240,6 +239,58 @@
assertTrue(tracker.isOrdinalized("test.EntryPoint$Fruit"));
}
+ public void testOrdinalizableStaticFieldRef()
+ throws UnableToCompleteException {
+ EnumOrdinalizer.resetTracker();
+
+ // this will cause a static field ref in the enum clinit
+ setupFruitEnumWithStaticField();
+ optimize("void", "String y = Fruit.staticField;");
+
+ EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
+ assertTrue(tracker.isOrdinalized("test.EntryPoint$Fruit"));
+ }
+
+ public void testOrdinalizableStaticMethod()
+ throws UnableToCompleteException {
+ EnumOrdinalizer.resetTracker();
+
+ // this will cause a static method enum class
+ setupFruitEnumWithStaticMethod();
+ optimize("void", "int y = Fruit.staticMethod();");
+
+ EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
+ assertTrue(tracker.isOrdinalized("test.EntryPoint$Fruit"));
+ }
+
+ public void testNotOrdinalizableInstanceStaticFieldRef()
+ throws UnableToCompleteException {
+ EnumOrdinalizer.resetTracker();
+
+ // this will cause a static field ref in the enum clinit
+ setupFruitEnumWithStaticField();
+ optimize("void", "Fruit fruit = Fruit.APPLE;",
+ "String y = fruit.staticField;");
+
+ EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
+ assertTrue(tracker.isVisited("test.EntryPoint$Fruit"));
+ assertFalse(tracker.isOrdinalized("test.EntryPoint$Fruit"));
+ }
+
+ public void testNotOrdinalizableInstanceStaticMethod()
+ throws UnableToCompleteException {
+ EnumOrdinalizer.resetTracker();
+
+ // this will cause a static method enum class
+ setupFruitEnumWithStaticMethod();
+ optimize("void", "Fruit fruit = Fruit.APPLE;",
+ "int y = fruit.staticMethod();");
+
+ EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
+ assertTrue(tracker.isVisited("test.EntryPoint$Fruit"));
+ assertFalse(tracker.isOrdinalized("test.EntryPoint$Fruit"));
+ }
+
public void testNotOrdinalizableClassLiteralReference()
throws UnableToCompleteException {
EnumOrdinalizer.resetTracker();
@@ -383,20 +434,40 @@
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 testNotOrdinalizableInstanceOfEnumExpression()
+ throws UnableToCompleteException {
+ setupFruitEnum();
+ optimize("void", "Fruit fruit = Fruit.APPLE;",
+ "if (fruit instanceof Enum) {",
+ " fruit = Fruit.ORANGE;",
+ "}");
+
+ EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
+ assertTrue(tracker.isVisited("test.EntryPoint$Fruit"));
+ assertFalse(tracker.isOrdinalized("test.EntryPoint$Fruit"));
}
- public void testNotOrdinalizableStaticFieldRef()
+ public void testNotOrdinalizableInstanceOfEnumTestType()
+ throws UnableToCompleteException {
+ setupFruitEnum();
+ optimize("void", "Object fruitObj = new Object();",
+ "if (fruitObj instanceof Fruit) {",
+ " fruitObj = null;",
+ "}");
+
+ EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
+ assertTrue(tracker.isVisited("test.EntryPoint$Fruit"));
+ assertFalse(tracker.isOrdinalized("test.EntryPoint$Fruit"));
+ }
+
+ public void testNotOrdinalizableStaticFieldRefToVALUES()
throws UnableToCompleteException {
EnumOrdinalizer.resetTracker();
- // this will cause a static field ref in the enum clinit
- setupFruitEnumWithStaticField();
- optimize("void");
+ // this ends up inlining the values() method call, and thus $VALUES is referenced external
+ // to the Fruit enum class.
+ setupFruitEnum();
+ optimize("void", "Fruit[] fruits = Fruit.values();");
EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
assertTrue(tracker.isVisited("test.EntryPoint$Fruit"));
@@ -407,7 +478,9 @@
throws UnableToCompleteException {
EnumOrdinalizer.resetTracker();
- // this ends up calling Fruit.clinit() first (which is a static method call)
+ // make sure values() method call doesn't doesn't get inlined
+ runMethodInliner = false;
+
setupFruitEnum();
optimize("void", "Fruit[] fruits = Fruit.values();");
@@ -824,7 +897,6 @@
private void setupFruitEnum() {
addSnippetClassDecl("public enum Fruit {APPLE, ORANGE}");
- setupExtraDummyEnum();
}
private void setupFruitEnumWithInstanceField() {
@@ -834,31 +906,27 @@
" instanceField = str;",
" }",
"}");
- setupExtraDummyEnum();
}
private void setupFruitEnumWithStaticField() {
addSnippetClassDecl("public enum Fruit {APPLE, ORANGE;",
" public static final String staticField = \"STATIC\";",
"}");
- setupExtraDummyEnum();
+ }
+
+ private void setupFruitEnumWithStaticMethod() {
+ addSnippetClassDecl("public enum Fruit {APPLE, ORANGE;",
+ " public static final int staticMethod() {",
+ " int x = 0;",
+ " return x;",
+ " }",
+ "}");
}
private void setupVegetableEnum() {
addSnippetClassDecl("public enum Vegetable {CARROT, SPINACH}");
}
- private void setupExtraDummyEnum() {
- /*
- * Assure there are at least more 2 enums in the program, so inlining or
- * tightening doesn't push a single enum sub-class into the methods of the
- * Enum super-class itself (which prevents ordinalization in most cases).
- * TODO(jbrosenberg): Make ordinalization work if there's only 1 enum in
- * a program.
- */
- addSnippetClassDecl("public enum DummyEnum {DUMMY}");
- }
-
private void setupFruitSwitchMethod() {
addSnippetClassDecl("public static String fruitSwitch(Fruit fruit) {",
" switch(fruit) {",
@@ -878,19 +946,11 @@
private final boolean runCastNormalizer = true;
private final boolean runEqualityNormalizer = true;
- /*
- * EnumOrdinalizer depends MakeCallsStatic and MethodInliner running before
- * it runs, since they cleanup the internal structure of an enum class to
- * inline instance methods like $init.
- * TODO(jbrosenberg): Update EnumOrdinalizer to be able to succeed
- * irrespective of the ordering and interaction with other optimizers.
- */
- private final boolean runMakeCallsStatic = true;
- private final boolean runMethodInliner = true;
-
- // these can be enabled where needed
- private boolean runMethodCallTightener = false;
- private boolean runTypeTightener = false;
+ // These are enabled as needed for a given test
+ private boolean runMakeCallsStatic;
+ private boolean runMethodInliner;
+ private boolean runMethodCallTightener;
+ private boolean runTypeTightener;
@Override
protected boolean optimizeMethod(JProgram program, JMethod method) {
