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

[Anuraag Agrawal]

Hello - my CLA has been processed so I guess the patch is ready for a look.
Can anyone help with this?

Thanks,
- Anuraag

On Tue, Dec 26, 2017 at 7:15 PM, Anuraag Agrawal <anuraaga at gmail.com> wrote:

> 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
>