GROOVY-12240: Initialize argument-less enum constants with a direct constructor call

src/main/java/org/codehaus/groovy/classgen/EnumConstantInit.java

@@ -0,0 +1,91 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.codehaus.groovy.classgen;
+
+import org.codehaus.groovy.ast.ClassHelper;
+import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.GroovyCodeVisitor;
+import org.codehaus.groovy.ast.Parameter;
+import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.classgen.asm.BytecodeHelper;
+import org.objectweb.asm.MethodVisitor;
+
+import static org.codehaus.groovy.ast.ClassHelper.int_TYPE;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.param;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.params;
+import static org.objectweb.asm.Opcodes.DUP;
+import static org.objectweb.asm.Opcodes.INVOKESPECIAL;
+import static org.objectweb.asm.Opcodes.NEW;
+
+/**
+ * Initializes an enum constant that supplies no arguments of its own by calling the
+ * {@code (String,int)} constructor of the enum directly.
+ * <p>
+ * The {@code $INIT} helper generated by {@link EnumVisitor} spreads an {@code Object[]}
+ * over the constructors of the enum, which means the meta class selects the constructor
+ * at run time by reflecting over {@code getDeclaredConstructors()}.  Where that reflection
+ * is unavailable, e.g. in a GraalVM native image for which the enum was not registered,
+ * the static initializer of the enum fails.  When the arguments are known at compile time
+ * to be exactly the name and the ordinal, the constructor can be selected there instead.
+ * <p>
+ * Only the bytecode generator sees the direct call: every other visitor is given the
+ * {@code $INIT} call, which is also emitted if the expected constructor turns out not to
+ * be present once all transforms have run.
+ */
+final class EnumConstantInit extends BytecodeExpression {
+
+    private static final Parameter[] NAME_AND_ORDINAL = params(param(ClassHelper.STRING_TYPE, "name"), param(int_TYPE, "ordinal"));
+
+    private final ClassNode enumClass;
+    private final String name;
+    private final int ordinal;
+    private final Expression initCall;
+
+    EnumConstantInit(final ClassNode enumClass, final String name, final int ordinal, final Expression initCall) {
+        super(enumClass.getPlainNodeReference());
+        this.enumClass = enumClass;
+        this.name = name;
+        this.ordinal = ordinal;
+        this.initCall = initCall;
+    }
+
+    @Override
+    public String getText() {
+        return initCall.getText();
+    }
+
+    @Override
+    public void visit(final GroovyCodeVisitor visitor) {
+        if (visitor instanceof AsmClassGenerator && enumClass.getDeclaredConstructor(NAME_AND_ORDINAL) != null) {
+            super.visit(visitor); // i.e. visitBytecodeExpression(this)
+        } else {
+            initCall.visit(visitor);
+        }
+    }
+
+    @Override
+    public void visit(final MethodVisitor mv) {
+        String owner = BytecodeHelper.getClassInternalName(enumClass);
+        mv.visitTypeInsn(NEW, owner);
+        mv.visitInsn(DUP);
+        mv.visitLdcInsn(name);
+        BytecodeHelper.pushConstant(mv, ordinal);
+        mv.visitMethodInsn(INVOKESPECIAL, owner, "<init>", "(Ljava/lang/String;I)V", false);
+    }
+}

src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java

@@ -264,6 +264,7 @@ private void addInit(final ClassNode enumClass, final FieldNode minValue, final
 
         // static init
         List<FieldNode> fields = enumClass.getFields();
+        boolean directInit = canInitDirectly(enumClass, fields);
         List<Expression> arrayInit = new ArrayList<>();
         List<Statement> block = new ArrayList<>();
         int index = -1;
@@ -320,7 +321,9 @@ private void addInit(final ClassNode enumClass, final FieldNode minValue, final
                 }
             }
             arrayInit.add(fieldX(field));
-            block.add(assignS(fieldX(field), callX(enumType, "$INIT", args)));
+            Expression init = callX(enumType, "$INIT", args);
+            if (directInit) init = new EnumConstantInit(enumClass, field.getName(), index, init);
+            block.add(assignS(fieldX(field), init));
         }
 
         if (!isAIC) {
@@ -338,6 +341,31 @@ private void addInit(final ClassNode enumClass, final FieldNode minValue, final
         enumClass.addStaticInitializerStatements(block, true);
     }
 
+    /**
+     * Determines whether the constants of the given enum can be initialized with a direct
+     * constructor call rather than with a call to the synthetic {@code $INIT} helper.
+     * <p>
+     * This is only the case when every constant is a plain identifier and so supplies no
+     * arguments of its own; the constructor arguments are then known to be exactly the
+     * compiler-supplied name and ordinal.  Constants declared with arguments, with named
+     * arguments or with a class body keep the {@code $INIT} path, as do enums whose own
+     * constructor cannot accept just the name and the ordinal.
+     *
+     * @param enumClass the enum being completed
+     * @param fields    the fields of {@code enumClass}, before any initial value is cleared
+     * @return {@code true} if a direct constructor call may be attempted
+     */
+    private static boolean canInitDirectly(final ClassNode enumClass, final List<FieldNode> fields) {
+        // an abstract enum or one that is extended has constants with a body, i.e. subclasses
+        if (isAnonymousInnerClass(enumClass) || enumClass.isAbstract() || !isNotExtended(enumClass)) return false;
+        // GROOVY-10811: a declared constructor must be callable with no user-supplied argument
+        if (!enumClass.getDeclaredConstructors().isEmpty() && !hasNoArgConstructor(enumClass)) return false;
+        for (FieldNode field : fields) {
+            if (field.isEnum() && field.getInitialExpression() != null) return false;
+        }
+        return true;
+    }
+
     private void addError(final AnnotatedNode an, final String msg) {
         getSourceUnit().getErrorCollector().addErrorAndContinue(
                 new SyntaxErrorMessage(

src/test/groovy/gls/enums/EnumTest.groovy

@@ -959,6 +959,58 @@ final class EnumTest extends CompilableTestSupport {
             }
         '''
     }
+
+    // constants that supply no arguments of their own are created with a direct
+    // constructor call instead of through the synthetic $INIT helper
+    @Test
+    void testConstantsWithoutArguments() {
+        assert Weekday.values()*.name() == ['MON', 'TUE', 'WED']
+        assert Weekday.values()*.ordinal() == [0, 1, 2]
+        assert Weekday.valueOf('TUE') == Weekday.TUE
+        assert Weekday.TUE.declaringClass == Weekday
+        assert Weekday.MIN_VALUE == Weekday.MON
+        assert Weekday.MAX_VALUE == Weekday.WED
+        assert Weekday.MON.next() == Weekday.TUE
+        assert Weekday.MON.previous() == Weekday.WED
+        assert Weekday.TUE in (Weekday.MON..Weekday.WED)
+        assert EnumSet.allOf(Weekday).size() == 3
+        assert Weekday.MON.compareTo(Weekday.WED) < 0
+    }
+
+    @Test
+    void testConstantsWithoutArgumentsAreSerializable() {
+        def buffer = new ByteArrayOutputStream()
+        new ObjectOutputStream(buffer).writeObject(Weekday.TUE)
+        def restored = new ObjectInputStream(new ByteArrayInputStream(buffer.toByteArray())).readObject()
+        assert restored.is(Weekday.TUE)
+    }
+
+    @Test
+    void testConstantsWithoutArgumentsRunTheDeclaredConstructor() {
+        assert Tagged.values()*.tag == ['tagged', 'tagged']
+    }
+
+    @Test
+    void testConstantsWithoutArgumentsWhenConstructorHasDefaults() {
+        assert Defaulted.values()*.label == ['none', 'none']
+    }
+}
+
+enum Weekday {
+    MON, TUE, WED
+}
+
+enum Tagged {
+    ALPHA, BETA
+    private final String tag
+    Tagged() { tag = 'tagged' }
+    String getTag() { tag }
+}
+
+enum Defaulted {
+    ONE, TWO
+    final String label
+    Defaulted(String label = 'none') { this.label = label }
 }
 
 enum UsCoin {

src/test/groovy/org/codehaus/groovy/classgen/asm/EnumConstantInitBytecodeTest.groovy

@@ -0,0 +1,175 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.codehaus.groovy.classgen.asm
+
+import org.junit.jupiter.api.Test
+
+/**
+ * Checks how the static initializer of an enum creates its constants.
+ * <p>
+ * A constant that supplies no arguments of its own is created with a direct call to the
+ * {@code (String,int)} constructor of the enum.  Every other constant keeps going through
+ * the synthetic {@code $INIT} helper, which spreads an {@code Object[]} over the
+ * constructors of the enum and so has the meta class select one reflectively.
+ */
+final class EnumConstantInitBytecodeTest extends AbstractBytecodeTestCase {
+
+    private List<String> staticInitializerOf(final String source) {
+        compile(method: '<clinit>', classNamePattern: 'E', source)
+        bodyOf('static <clinit>()V')
+    }
+
+    private List<String> bodyOf(final String header) {
+        def all = sequence.instructions
+        int start = all.findIndexOf { it == header }
+        assert start >= 0: "method not found: $header\n$sequence"
+        int end = all.findIndexOf(start) { it.startsWith('MAXSTACK') }
+        all[(start + 1)..<end]
+    }
+
+    private static void assertDirectConstructorCall(final List<String> code) {
+        assert code.join('\n').contains([
+            'NEW E',
+            'DUP',
+            'LDC "ONE"',
+            'ICONST_0',
+            'INVOKESPECIAL E.<init> (Ljava/lang/String;I)V',
+            'PUTSTATIC E.ONE : LE;'
+        ].join('\n'))
+        assert !code.any { it.contains('$INIT') }
+        assert !code.any { it.contains('selectConstructorAndTransformArguments') }
+    }
+
+    private static void assertInitHelperCall(final List<String> code) {
+        assert code.any { it.contains('$INIT') }
+        assert !code.any { it.startsWith('NEW E') }
+    }
+
+    @Test
+    void testConstantsWithoutArgumentsCallConstructorDirectly() {
+        def code = staticInitializerOf '''
+            enum E { ONE, TWO }
+        '''
+        assertDirectConstructorCall(code)
+    }
+
+    @Test
+    void testInitHelperIsStillGenerated() {
+        staticInitializerOf '''
+            enum E { ONE, TWO }
+        '''
+        def header = sequence.instructions.find { it.contains(' $INIT([Ljava/lang/Object;)LE;') }
+        assert header != null
+        assert bodyOf(header).any { it.contains('selectConstructorAndTransformArguments') }
+    }
+
+    @Test
+    void testExplicitNoArgConstructorCallsConstructorDirectly() {
+        def code = staticInitializerOf '''
+            enum E {
+                ONE, TWO
+                private final String tag
+                E() { tag = 'x' }
+            }
+        '''
+        assertDirectConstructorCall(code)
+    }
+
+    @Test
+    void testConstructorWithDefaultsCallsConstructorDirectly() {
+        def code = staticInitializerOf '''
+            enum E {
+                ONE, TWO
+                final String tag
+                E(String tag = 'x') { this.tag = tag }
+            }
+        '''
+        assertDirectConstructorCall(code)
+    }
+
+    @Test
+    void testCompileStaticConstantsCallConstructorDirectly() {
+        def code = staticInitializerOf '''
+            @groovy.transform.CompileStatic
+            enum E { ONE, TWO }
+        '''
+        assertDirectConstructorCall(code)
+    }
+
+    @Test
+    void testConstantsWithArgumentsUseInitHelper() {
+        def code = staticInitializerOf '''
+            enum E {
+                ONE(1), TWO(2)
+                final int value
+                E(int value) { this.value = value }
+            }
+        '''
+        assertInitHelperCall(code)
+    }
+
+    @Test
+    void testConstantsWithNamedArgumentsUseInitHelper() {
+        def code = staticInitializerOf '''
+            enum E {
+                ONE(value: 1), TWO(value: 2)
+                int value
+            }
+        '''
+        assertInitHelperCall(code)
+    }
+
+    @Test
+    void testConstantsWithABodyUseInitHelper() {
+        def code = staticInitializerOf '''
+            enum E {
+                ONE { int twice() { 2 } },
+                TWO { int twice() { 4 } }
+                abstract int twice()
+            }
+        '''
+        assertInitHelperCall(code)
+    }
+
+    @Test
+    void testMixOfConstantsWithAndWithoutArgumentsUsesInitHelper() {
+        def code = staticInitializerOf '''
+            enum E {
+                ONE, TWO(2)
+                final int value
+                E(int value = 0) { this.value = value }
+            }
+        '''
+        assertInitHelperCall(code)
+    }
+
+    // the transform leaves the enum without a constructor that takes just the name and
+    // the ordinal, so the direct call is not available and $INIT is emitted as before
+    @Test
+    void testMissingNameAndOrdinalConstructorUsesInitHelper() {
+        def code = staticInitializerOf '''
+            @groovy.transform.TupleConstructor(defaults = false)
+            enum E {
+                ONE
+                String value
+            }
+        '''
+        assertInitHelperCall(code)
+    }
+}