codeconsole opened a new pull request, #3024:
URL: https://github.com/apache/groovy/pull/3024
GROOVY-12240 made enum constants without arguments call the `(String,int)`
constructor directly. Every constant with arguments still goes through the
synthetic `$INIT(Object[])` helper. Its body is a spread `this(*para)`, which
compiles to `ScriptBytecodeAdapter.despreadList` plus
`selectConstructorAndTransformArguments`, so `MetaClassImpl` selects the
constructor by reflecting over the declared constructors. `@CompileStatic`
doesn't change that: the static initializer then calls `$INIT` with
`invokestatic`, but `$INIT`'s own body stays dynamic.
`EnumVisitor.canInitDirectly` disabled the direct path for any constant with
arguments.
As a result, an ordinary enum fails to initialize in a GraalVM native image
built without reflection metadata for it:
```groovy
enum AccessModifier {
PRIVATE('private'), PROTECTED('protected'), PUBLIC('public')
final String keyword
AccessModifier(String keyword) { this.keyword = keyword }
}
```
```
groovy.lang.GroovyRuntimeException: Could not find matching constructor for:
AccessModifier(String, Integer, String)
```
This change emits a direct constructor call when the compiler can prove what
the runtime would do. That requires all three of:
- every argument is a literal: a constant, `null` or a class literal;
- exactly one non-synthetic constructor has the matching arity, and no
constructor could take the arguments as varargs;
- each parameter is the literal's own type, the primitive of a wrapper
literal, or a type other than `Boolean` that the literal extends or implements
(`null` goes only to a non-primitive parameter).
Under those conditions `MetaClassImpl.chooseMethod` has a single valid
candidate. `ParameterTypes.isValidMethod` rejects every other arity unless the
last parameter is an array, and `CachedClass` does not list synthetic
constructors. The cast `$INIT` applies before the call returns the argument
unchanged: `xxxUnbox` for a primitive, the indy cast otherwise. The indy cast
to `Boolean` goes through `asBoolean`, which is why `Boolean` is excluded.
Everything else keeps `$INIT`:
- named arguments, class bodies and abstract enums, as before;
- same-arity overloads and varargs;
- a GString for a String parameter;
- a literal the runtime would convert, such as an int literal for a long
parameter or a BigDecimal literal for a double;
- `null` for a primitive, and any argument that isn't a literal.
The choice is per enum, as in GROOVY-12240, so the documented opt-out (an
explicit empty argument list such as `ONE()`) still applies to the whole enum.
An enum's static initializer either needs constructor reflection or it doesn't.
Implementation:
- The decision is made at bytecode generation in `EnumConstantInit`, because
only then are the constructors (enum completion, default-argument expansion,
AST transforms) and the transformed arguments final.
- A `Constants` object shared by an enum's constants tracks the latest
wrapper of each constant as transforms replace it. It decides once, and each
constant re-checks itself.
- The arguments are read from the `$INIT` call as the transforms left it,
and the bytecode generator loads them exactly as it would for that call.
- `$INIT` is still generated, and every visitor other than the class
generator sees the same tree.
- Node-metadata reads that the wrapper doesn't answer itself fall through to
the wrapped `$INIT` call. That keeps what the type checker records for it
(GROOVY-10845) readable from the right-hand side of the constant's assignment;
`MiscSTCTest.testEnumConstructorChecks` reads it there. If reviewers would
rather adapt that test than give the wrapper this behavior, it is easy to
change.
As with GROOVY-12240, a meta class registered before the enum initializes no
longer sees these constants being created. Where every constant has arguments,
passing one argument as a non-literal expression keeps the enum on `$INIT`.
`EnumTest` pins both the narrowing and the opt-out.
Tests:
- `EnumConstantInitBytecodeTest` gains direct-call assertions for String,
int, every literal kind, `@CompileStatic`, mixed constants and arity-selected
constructors. It also asserts that the following stay on `$INIT`: same-arity
overloads, `null` with overloads, varargs (alone and beside an exact match), a
GString, converting literals, non-literal arguments, a mix of qualifying and
non-qualifying constants, arguments with a body, the `ONE()` opt-out, and the
`@CompileStatic` fallback.
- `EnumTest` gains behavior tests: values, ordinals, field values and
runtime classes, `valueOf`, `switch`, static fields read from the constructor
(including initialization order), default arguments, arity and
`@CompileStatic`. It also tests meta-class interception and the opt-out.
Seven of the new tests fail on master. The enum-related suites and the
`groovy.transform.stc.*`, `org.codehaus.groovy.classgen.*`, `gls.*` and
`bugs.*` packages pass (7,591 tests), as do the Checkstyle and CodeNarc gates.
A differential probe compiled 26 enums with this branch's compiler and with
master's and found identical runtime values. Of the 26, 18 now take the direct
call; 8 stay on `$INIT` by design: BigDecimal→double, int→long, a `Boolean`
parameter, a GString, varargs, a list literal, `'c' as char` and `ONE()`.
The same gap exists on 6.0.x, so a backport to `GROOVY_6_0_X` would let
Grails 9, which builds with Groovy 6, initialize enums like the one above in a
native image without metadata.
There is no JIRA issue for this yet.
--
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]
