Reviewers: cromwellian,

Description:
Don't allow enum ordinalization if a custom enum method references
values() directly
Addresses issue 7067
(http://code.google.com/p/google-web-toolkit/issues/detail?id=7067)


Please review this at http://gwt-code-reviews.appspot.com/1730804/

Affected files:
  M dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java
  M dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java


Index: dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java
===================================================================
--- dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java (revision 11028) +++ dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java (working copy)
@@ -434,16 +434,18 @@
         blackListIfEnumExpression(x.getInstance());
       } else if (x.getField().isStatic()) {
         /*
- * 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.
+ * Black list if the $VALUES static field is referenced, unless it's + * within the auto-generated clinit or values method, within the enum
+         * class itself.
          *
          * 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()) { + && ((this.currentMethod.getEnclosingType() != x.getField().getEnclosingType()) ||
+                (!this.currentMethod.getName().equals("values") &&
+                 !this.currentMethod.getName().equals("$clinit")))) {
blackListIfEnum(x.getField().getEnclosingType(), x.getSourceInfo());
         }
       }
Index: dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java
===================================================================
--- dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java (revision 11028) +++ dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java (working copy)
@@ -259,6 +259,33 @@
     assertTrue(tracker.isOrdinalized("test.EntryPoint$Fruit"));
   }

+  public void testNotOrdinalizableStaticMethodThatRefsValuesArray()
+      throws UnableToCompleteException  {
+    EnumOrdinalizer.resetTracker();
+
+    // this will cause a static method that references an element
+    // of the values() array
+    setupFruitEnumWithStaticMethodThatRefsValuesArray();
+    optimize("void", "Fruit y = Fruit.forInteger(0);");
+
+    EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
+    assertTrue(tracker.isVisited("test.EntryPoint$Fruit"));
+    assertFalse(tracker.isOrdinalized("test.EntryPoint$Fruit"));
+  }
+
+  public void testNotOrdinalizableStaticMethodThatRefsValuesLength()
+      throws UnableToCompleteException  {
+    EnumOrdinalizer.resetTracker();
+
+    // this will cause a static method that references values().length
+    setupFruitEnumWithStaticMethodThatRefsValuesLength();
+    optimize("void", "Fruit y = Fruit.forInteger(0);");
+
+    EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker();
+    assertTrue(tracker.isVisited("test.EntryPoint$Fruit"));
+    assertFalse(tracker.isOrdinalized("test.EntryPoint$Fruit"));
+  }
+
   public void testNotOrdinalizableInstanceStaticFieldRef()
       throws UnableToCompleteException  {
     EnumOrdinalizer.resetTracker();
@@ -977,6 +1004,29 @@
                         "  public static final int staticMethod() {",
                         "    int x = 0;",
                         "    return x;",
+                        "  }",
+                        "}");
+  }
+
+  private void setupFruitEnumWithStaticMethodThatRefsValuesArray() {
+    // add a little extra logic here, to prevent inlining
+    addSnippetClassDecl("public enum Fruit {APPLE, ORANGE;",
+                        "  public static Fruit forInteger(int value) {",
+                        "    if (value < 0 || value >= 2) {",
+                        "      return ORANGE;",
+                        "    }",
+                        "    return Fruit.values()[value];",
+                        "  }",
+                        "}");
+  }
+
+  private void setupFruitEnumWithStaticMethodThatRefsValuesLength() {
+    addSnippetClassDecl("public enum Fruit {APPLE, ORANGE;",
+                        "  public static Fruit forInteger(int value) {",
+ " if (value < 0 || value >= Fruit.values().length) {",
+                        "      return ORANGE;",
+                        "    }",
+                        "    return APPLE;",
                         "  }",
                         "}");
   }


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to