codeconsole opened a new pull request, #2772: URL: https://github.com/apache/groovy/pull/2772
https://issues.apache.org/jira/browse/GROOVY-12240 ### Motivation `EnumVisitor` creates every enum constant through a synthetic helper: ```groovy def $INIT(Object[] para) { return this(*para) } ``` `this(*para)` is a spread constructor call, so `InvocationWriter.makeDirectConstructorCall` refuses it — it bails on `SpreadExpression`, and again on `!controller.isConstructor()` — and the body compiles to `ScriptBytecodeAdapter.despreadList` plus `selectConstructorAndTransformArguments`. The meta class then picks the constructor at run time by reflecting over `getDeclaredConstructors()`. The static initializer reaches `$INIT` itself through a dynamic call site. For `enum Colors { RED, GREEN, BLUE }` that is the whole constant-creation path, even though the only arguments are the compiler-supplied name and ordinal, both known at compile time. Where reflection over the enum isn't available the class simply cannot initialize. In a GraalVM native image built without reachability metadata for the enum, `getDeclaredConstructors()` returns nothing and class initialization throws ``` groovy.lang.GroovyRuntimeException: Could not find matching constructor for: com.example.MyEnum(String, Integer) ``` (note the boxed `Integer` — the ordinal has been through `Object[]`). This kills the application in a static initializer before any user code runs. `@CompileStatic` does not help: Groovy already compiles the *call site* statically (`StaticTypeCheckingVisitor`, GROOVY-10845); it is `$INIT`'s own body that is necessarily dynamic. ### Change When **every** constant of an enum is a plain identifier, the arguments are provably `[name, ordinal]`, and the static initializer now calls the enum's `(String,int)` constructor directly. ``` static {}; static {}; 0: ldc // class Colors 0: new // class Colors 2: ldc // String RED 3: dup 4: iconst_0 4: ldc // String RED 5: invokestatic Integer.valueOf 6: iconst_0 8: invokedynamic invoke:(Class;String; 7: invokespecial "<init>":(Ljava/lang/String;I)V Integer;)Object; 10: putstatic Field RED:LColors; 13: invokedynamic cast:(Object;)LColors; 18: putstatic Field RED:LColors; ``` The same shape javac emits for a Java enum: 21 bytes and two indy call sites per constant become 13 bytes and none. ### When the new path applies Only when all of the following hold: - every constant is a plain identifier — no arguments, no named arguments, no class body; - the enum is not abstract and has no `EnumConstantClassNode` inner classes; - the enum declares no constructor, or declares one callable with no user-supplied argument; - and, checked at bytecode generation once every transform has run, a `(String,int)` constructor actually exists. Anything else keeps the existing `$INIT` path. ### How the fallback works `EnumConstantInit` is a `BytecodeExpression` that holds the original `$INIT` call. It hands that call to every visitor except `AsmClassGenerator`, and hands it to `AsmClassGenerator` too when the expected constructor isn't present. So type checking, scope resolution and AST transforms see exactly the tree they see today, and a shape that cannot use the direct call degrades to today's bytecode rather than to a different failure. A concrete case: `@TupleConstructor(defaults = false) enum E { ONE; String value }` compiles today and fails at class initialization with `Could not find matching constructor`. It has no `(String,int)` constructor, so it keeps `$INIT` and keeps failing in exactly that way. There is a test for it. `$INIT` is unchanged and still generated for every enum. ### Scope, stated honestly This is a **compile-time** change. It only helps code compiled by a Groovy that carries the fix; bytecode already compiled by an earlier Groovy keeps its `$INIT` path whichever Groovy runs it. I confirmed this by building a GraalVM native image of an application against a patched Groovy: the framework's own enums, compiled by an earlier Groovy, still failed with `Could not find matching constructor` until reachability metadata was restored. Enums whose constants take arguments (`RED(255, 0, 0)`) are not addressed and still require reachability metadata in a native image. Fixing those would mean relaxing `InvocationWriter.makeDirectConstructorCall` to work outside a constructor, which is a much wider change and deliberately left alone. One semantic narrowing worth reviewer attention: a plain enum's `<clinit>` no longer touches the meta class, so anything relying on intercepting an enum constructor via `ExpandoMetaClass` before class initialization would no longer see it. I believe this is unreachable in practice — the enum is initialized once, before any such hook could be installed, and Java enums offer no equivalent — but it is a real change. ### Tests - `EnumConstantInitBytecodeTest` (new) — asserts the emitted `<clinit>` instruction sequence for the direct case, that `$INIT` is still generated with its usual body, and that constants with arguments / named arguments / a body / a mix, and the missing-constructor case, all keep `$INIT`. - `gls.enums.EnumTest` — behaviour coverage for the new path: values, ordinals, `valueOf`, `next`/`previous`, `MIN_VALUE`/`MAX_VALUE`, ranges, `EnumSet`, `compareTo`, serialization identity, and enums with an explicit no-arg or all-defaults constructor. Also verified by hand across packaged, nested and doubly-nested enums, `@CompileStatic`, `@TypeChecked`, and a 400-constant enum (correct `iconst`/`bipush`/`sipush` selection at 5/6/127/128/399). `./gradlew :test` passes in full: 16,548 tests, 0 failures. Related to GROOVY-12234. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
