From 4d53a4e3dfce02e147815e7ffc14b4234d8f8d97 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Fri, 7 Aug 2026 12:59:28 -0700 Subject: [PATCH 1/2] GROOVY-12240: Initialize argument-less enum constants with a direct constructor call EnumVisitor routes every enum constant through the synthetic $INIT(Object[]) helper, whose body is a spread constructor call. That compiles to ScriptBytecodeAdapter.despreadList plus selectConstructorAndTransformArguments, so the meta class picks the constructor at run time by reflecting over getDeclaredConstructors(); the static initializer in turn reaches $INIT itself through a dynamic call site. For a constant that supplies no arguments of its own the arguments are only the compiler-supplied name and ordinal, both of which are known at compile time. Emit a direct call to the (String,int) constructor of the enum for those, so the static initializer needs neither the meta class nor reflection. That matters where reflection over the enum is not available: in a GraalVM native image built without reachability metadata for the enum, getDeclaredConstructors() is empty and class initialization fails with groovy.lang.GroovyRuntimeException: Could not find matching constructor for: com.example.MyEnum(String, Integer) @CompileStatic does not help, because it is $INIT's own body that is dynamic. The direct call is used only when every constant of the enum is a plain identifier. Constants with arguments, with named arguments or with a class body keep the $INIT path, as do abstract enums, the classes generated for constant bodies and enums without a constructor that takes just the name and the ordinal. $INIT is still generated in every case, and only the bytecode generator is shown the direct call, so type checking and every other visitor see the same tree as before. --- .../groovy/classgen/EnumConstantInit.java | 91 +++++++++ .../codehaus/groovy/classgen/EnumVisitor.java | 30 ++- src/test/groovy/gls/enums/EnumTest.groovy | 52 ++++++ .../asm/EnumConstantInitBytecodeTest.groovy | 175 ++++++++++++++++++ 4 files changed, 347 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/codehaus/groovy/classgen/EnumConstantInit.java create mode 100644 src/test/groovy/org/codehaus/groovy/classgen/asm/EnumConstantInitBytecodeTest.groovy diff --git a/src/main/java/org/codehaus/groovy/classgen/EnumConstantInit.java b/src/main/java/org/codehaus/groovy/classgen/EnumConstantInit.java new file mode 100644 index 00000000000..ff8c94b417e --- /dev/null +++ b/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. + *

+ * 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. + *

+ * 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, "", "(Ljava/lang/String;I)V", false); + } +} diff --git a/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java b/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java index 7eb22de349d..22bf9334cd3 100644 --- a/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java +++ b/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 fields = enumClass.getFields(); + boolean directInit = canInitDirectly(enumClass, fields); List arrayInit = new ArrayList<>(); List 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. + *

+ * 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 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( diff --git a/src/test/groovy/gls/enums/EnumTest.groovy b/src/test/groovy/gls/enums/EnumTest.groovy index dc5b92085d3..c28aea1f9b9 100644 --- a/src/test/groovy/gls/enums/EnumTest.groovy +++ b/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 { diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/EnumConstantInitBytecodeTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/EnumConstantInitBytecodeTest.groovy new file mode 100644 index 00000000000..e900a1e6e3b --- /dev/null +++ b/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. + *

+ * 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 staticInitializerOf(final String source) { + compile(method: '', classNamePattern: 'E', source) + bodyOf('static ()V') + } + + private List 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).. code) { + assert code.join('\n').contains([ + 'NEW E', + 'DUP', + 'LDC "ONE"', + 'ICONST_0', + 'INVOKESPECIAL E. (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 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) + } +} From 72fb430accdf570292da27ae031b9c48a662e958 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Sat, 8 Aug 2026 21:03:02 -0700 Subject: [PATCH 2/2] GROOVY-12240: Let expression transforms reach the wrapped $INIT call EnumConstantInit hands the $INIT call it wraps to every visitor other than the bytecode generator, but transformExpression was inherited from BytecodeExpression and returns this, so a ClassCodeExpressionTransformer stops at the wrapper and never descends into the call it is holding. That is not only cosmetic. StaticCompilationTransformer is such a transformer, and it is what rewrites a StaticMethodCallExpression carrying a direct method call target into the MethodCallExpression that StaticInvocationWriter emits as an invokestatic. Skipping it, an enum that qualifies for the direct call when the AST is built but turns out at bytecode generation not to have the (String,int) constructor fell back to a $INIT call that no longer compiled statically. For @CompileStatic @TupleConstructor(defaults = false) enum E { ONE; String[] value } the static initializer went from INVOKESTATIC E.$INIT ([Ljava/lang/Object;)LE; to INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.invokeStaticMethodN INVOKEDYNAMIC cast(Ljava/lang/Object;)LE; which is a meta class dispatch in a statically compiled class, and reflection in exactly the place this change set out to remove it from. Transform the wrapped call and keep the result, the way MapStyleConstructorCall already does. Every enum that does not take the direct call now compiles to the same bytecode as before this change again. --- .../groovy/classgen/EnumConstantInit.java | 14 ++++++++++++-- .../asm/EnumConstantInitBytecodeTest.groovy | 17 +++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/EnumConstantInit.java b/src/main/java/org/codehaus/groovy/classgen/EnumConstantInit.java index ff8c94b417e..8bd90df696e 100644 --- a/src/main/java/org/codehaus/groovy/classgen/EnumConstantInit.java +++ b/src/main/java/org/codehaus/groovy/classgen/EnumConstantInit.java @@ -23,6 +23,7 @@ import org.codehaus.groovy.ast.GroovyCodeVisitor; import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.expr.Expression; +import org.codehaus.groovy.ast.expr.ExpressionTransformer; import org.codehaus.groovy.classgen.asm.BytecodeHelper; import org.objectweb.asm.MethodVisitor; @@ -45,8 +46,9 @@ * to be exactly the name and the ordinal, the constructor can be selected there instead. *

* 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. + * {@code $INIT} call and every {@link ExpressionTransformer} rewrites it in place, so the + * call that is emitted if the expected constructor turns out not to be present once all + * transforms have run is the one that would have been emitted without this expression. */ final class EnumConstantInit extends BytecodeExpression { @@ -79,6 +81,14 @@ public void visit(final GroovyCodeVisitor visitor) { } } + @Override + public Expression transformExpression(final ExpressionTransformer transformer) { + Expression result = new EnumConstantInit(enumClass, name, ordinal, transformer.transform(initCall)); + result.setSourcePosition(this); + result.copyNodeMetaData(this); + return result; + } + @Override public void visit(final MethodVisitor mv) { String owner = BytecodeHelper.getClassInternalName(enumClass); diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/EnumConstantInitBytecodeTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/EnumConstantInitBytecodeTest.groovy index e900a1e6e3b..323ff7822de 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/EnumConstantInitBytecodeTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/EnumConstantInitBytecodeTest.groovy @@ -172,4 +172,21 @@ final class EnumConstantInitBytecodeTest extends AbstractBytecodeTestCase { ''' assertInitHelperCall(code) } + + // the $INIT call that is emitted instead of the direct call must be the one the static + // compilation transformer produced, not the untransformed call it was given + @Test + void testMissingNameAndOrdinalConstructorKeepsStaticInitHelperCall() { + def code = staticInitializerOf ''' + @groovy.transform.CompileStatic + @groovy.transform.TupleConstructor(defaults = false) + enum E { + ONE + String[] value + } + ''' + assertInitHelperCall(code) + assert code.any { it.contains('INVOKESTATIC E.$INIT ([Ljava/lang/Object;)LE;') } + assert !code.any { it.contains('ScriptBytecodeAdapter') } + } }