[PATCH] Support enum params that override Object#toString()

[Anuraag Agrawal]

Hi all,

As discussed in
http://mail.openjdk.java.net/pipermail/jmh-dev/2017-December/002678.html
currently JMH does not handle enum params where the enum overrides toString
when using the reflection or ASM generators. This is because the spec
explicitly allows enums to override toString() to something that doesn't
match the descriptor's name, but restoring the enum using valueOf can only
happen with the descriptor's name.

https://docs.oracle.com/javase/7/docs/api/java/lang/Enum.html#toString()

This patch changes use of toString() to name(), which is always guaranteed
to be the descriptor's name and round-trippable.

https://docs.oracle.com/javase/7/docs/api/java/lang/Enum.html#name()

The patch has been attached and inlined below, including a regression test
which fails without the logic change.

Note, I just signed and emailed the Oracle CLA, so it may take some time
for that to get processed.

Thanks. Patch follows

# HG changeset patch
# User Anuraag Agrawal <anuraaga at gmail.com>
# Date 1512560148 -32400
#      Wed Dec 06 20:35:48 2017 +0900
# Node ID 4eefe5751280399fdbde2df655d2dcf43f2de557
# Parent  1ddf31f810a3100b9433c3fedf24615e85b1d1a7
Use Enum.name() instead of Enum.toString() for determining the string value
of enum params. Only Enum.name() is guaranteed to round-trip through
Enum.valueOf during execution, the specification explicitly allows
overriding toString() to something that will not round-trip.

diff --git
a/jmh-core-it/src/test/java/org/openjdk/jmh/it/params/EnumParamSequenceTest.java
b/jmh-core-it/src/test/java/org/openjdk/jmh/it/params/EnumParamToStringOverridingTest.java
copy from
jmh-core-it/src/test/java/org/openjdk/jmh/it/params/EnumParamSequenceTest.java
copy to
jmh-core-it/src/test/java/org/openjdk/jmh/it/params/EnumParamToStringOverridingTest.java
---
a/jmh-core-it/src/test/java/org/openjdk/jmh/it/params/EnumParamSequenceTest.java
+++
b/jmh-core-it/src/test/java/org/openjdk/jmh/it/params/EnumParamToStringOverridingTest.java
@@ -45,56 +45,32 @@
 @Warmup(iterations = 1, time = 100, timeUnit = TimeUnit.MICROSECONDS)
 @Fork(1)
 @State(Scope.Thread)
-public class EnumParamSequenceTest {
+public class EnumParamToStringOverridingTest {

     @Param({"VALUE_A", "VALUE_B", "VALUE_C"})
     public SampleEnumA a;

-    @Param({"VALUE_A", "VALUE_B", "VALUE_C"})
-    public SampleEnumB b;
-
     @Benchmark
     public void test() {
         Fixtures.work();
     }

     @Test
-    public void full() throws RunnerException {
+    public void normal() throws RunnerException {
         Options opts = new OptionsBuilder()
                 .include(Fixtures.getTestMask(this.getClass()))
                 .shouldFailOnError(true)
                 .build();

-        Assert.assertEquals(3 * 3, new Runner(opts).run().size());
-    }
-
-    @Test
-    public void constrainedA() throws RunnerException {
-        Options opts = new OptionsBuilder()
-                .include(Fixtures.getTestMask(this.getClass()))
-                .shouldFailOnError(true)
-                .param("a", SampleEnumA.VALUE_A.name())
-                .build();
-
-        Assert.assertEquals(1 * 3, new Runner(opts).run().size());
-    }
-
-    @Test
-    public void constrainedB() throws RunnerException {
-        Options opts = new OptionsBuilder()
-                .include(Fixtures.getTestMask(this.getClass()))
-                .shouldFailOnError(true)
-                .param("b", SampleEnumB.VALUE_A.name())
-                .build();
-
-        Assert.assertEquals(1*3, new Runner(opts).run().size());
+        Assert.assertEquals(3, new Runner(opts).run().size());
     }

     public enum SampleEnumA {
-        VALUE_A, VALUE_B, VALUE_C
-    }
+        VALUE_A, VALUE_B, VALUE_C;

-    public enum SampleEnumB {
-        VALUE_A, VALUE_B, VALUE_C
+        @Override
+        public String toString() {
+            return name().toLowerCase();
+        }
     }
 }
diff --git
a/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMClassInfo.java
b/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMClassInfo.java
---
a/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMClassInfo.java
+++
b/jmh-generator-asm/src/main/java/org/openjdk/jmh/generators/asm/ASMClassInfo.java
@@ -221,7 +221,7 @@
             try {
                 Collection<String> res = new ArrayList<>();
                 for (Object cnst : Class.forName(origQualifiedName, false,
Thread.currentThread().getContextClassLoader()).getEnumConstants()) {
-                    res.add(cnst.toString());
+                    res.add(((Enum<?>) cnst).name());
                 }
                 return res;
             } catch (ClassNotFoundException e) {
diff --git
a/jmh-generator-reflection/src/main/java/org/openjdk/jmh/generators/reflection/RFClassInfo.java
b/jmh-generator-reflection/src/main/java/org/openjdk/jmh/generators/reflection/RFClassInfo.java
---
a/jmh-generator-reflection/src/main/java/org/openjdk/jmh/generators/reflection/RFClassInfo.java
+++
b/jmh-generator-reflection/src/main/java/org/openjdk/jmh/generators/reflection/RFClassInfo.java
@@ -162,7 +162,7 @@
     public Collection<String> getEnumConstants() {
         Collection<String> res = new ArrayList<>();
         for (Object cnst : klass.getEnumConstants()) {
-            res.add(cnst.toString());
+            res.add(((Enum<?>) cnst).name());
         }
         return res;
     }

- Anuraag
-------------- next part --------------
A non-text attachment was scrubbed...
Name: enum-name.patch
Type: text/x-patch
Size: 4491 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/jmh-dev/attachments/20171226/081311bb/enum-name.patch>