[
https://issues.apache.org/jira/browse/GROOVY-12491?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18131894#comment-18131894
]
ASF GitHub Bot commented on GROOVY-12491:
-----------------------------------------
codeconsole commented on PR #3025:
URL: https://github.com/apache/groovy/pull/3025#issuecomment-6400424812
Pushed 073ef08395. Replies in the order of the review:
1. Fixed in 1c7809b1f5: a miss throws a `GroovyBugError` naming the helper
and the trait, as `Traits.findHelpers` already does for a missing helper class.
`testPrecompiledTraitHelperWithoutInitializerIsRejected` strips `$init$` from a
precompiled helper with ASM and expects the error; it fails without the guard.
`getDeclaredMethod` compares parameter types by class name, so generic,
`@SelfType` and precompiled traits match
(`testInitializerOfGenericTraitIsCalledDirectly`,
`testInitializerOfSelfTypeTraitIsCalledDirectly`,
`testInitializerOfPrecompiledTraitIsCalledDirectly`).
2. Done in 9f2c73507e for both `$init$` and `$static$init$`; the
`$static$init$` call did not set it either, only the forwarders did. The field
initializer calls from 177b7ccdee set it too. The bytecode is unchanged (38
class files compared). `safe` is already false by default.
3. Done in 716bbd9371: the note is on `Traits.INIT_METHOD`, and the JIRA
description says the same. Runtime trait application has no separate path:
`ProxyGeneratorAdapter` compiles a `$TraitAdapter` class that `TraitComposer`
composes like any other class, so on 6.0.0 `withTraits` and `as` also went
through the meta class, and now they do not.
`testTraitInitializerIsNotDispatchedThroughMetaClass` covers the compiled
classes, a `@TypeChecked` class and both runtime forms.
4. Done in this PR, 177b7ccdee: the field initializers (`$init$<field>`,
`$static$init$<field>`) are now calls with the helper method as target. Dynamic
and `@TypeChecked` classes called them with invokedynamic (`@CompileStatic`
ones were already direct); every mode now emits invokestatic, also against
traits compiled by Groovy 4.0.24 and 5.0.3, and with indy off.
`testFieldInitializersOfTraitAreCalledDirectly` and the extended precompiled
test fail without it.
5. Added in 073ef08395:
`testInitializersFollowTheOrderOfTheImplementsClause` (`implements C, B` gives
B, A, C), `testSubclassCallsTheInitializersOfTheTraitsItDeclares` and
`testInitializationOrderWithSwappedTraitsAndSuperclass`. The orders match
6.0.0, including a subclass that declares its superclass's trait again through
a sub-trait and so initializes its own copy of that trait's fields, as
`_traits.adoc` describes.
6. Added in 073ef08395: `testInitializerIsCalledDirectlyFromAnonymousClass`
checks the constructors of anonymous classes created in a method and in a
closure.
Test gaps:
- `$init$` that cannot be resolved:
`testPrecompiledTraitHelperWithoutInitializerIsRejected` (1c7809b1f5).
- Declaration order and superclass/subclass traits: item 5.
- `new Greeting() {}`: item 6.
- `@CompileStatic` trait with field initializers:
`testStaticallyCompiledTraitIsInitializedDirectly` (073ef08395), with dynamic,
`@TypeChecked` and `@CompileStatic` implementers; it also checks that the
trait's own `$init$` makes no dynamic call.
Nits:
- The note on `Traits.INIT_METHOD` names GROOVY-12491. The sibling constants
in `Traits` carry no `@since`, so none was added.
- Done in 073ef08395: the assertions also reject an invokedynamic call site
whose bootstrap arguments name an initializer, and cover the field initializers.
- Kept; the new precompiled test uses the same pattern.
`TraitInitBytecodeTest` now has 47 tests; with the pre-PR `TraitComposer`,
37 fail. 14,443 tests pass on JDK 21, and `bugs.Groovy12142`,
`bugs.Groovy12363` and `TraitInitBytecodeTest` on JDK 17.
> Classes implementing a trait call the helper's $init$ through the meta class
> ----------------------------------------------------------------------------
>
> Key: GROOVY-12491
> URL: https://issues.apache.org/jira/browse/GROOVY-12491
> Project: Groovy
> Issue Type: Bug
> Components: Compiler, Static compilation
> Affects Versions: 6.0.0
> Reporter: Scott Murphy Heiberg
> Priority: Major
>
> When a class implements a trait, {{TraitComposer.applyTrait}} adds
> {{Helper.$init$(this)}} to the class's object initializer without a method
> target (the {{$static$init$}} call beside it has one). Every constructor
> therefore dispatches the call through the meta class of the helper, even in
> {{@CompileStatic}} classes:
> {code:groovy}
> trait Greeting { String greeting = 'hello' }
> @CompileStatic class StaticGreeter implements Greeting {}
> class DynamicGreeter implements Greeting {}
> {code}
> * {{StaticGreeter.<init>}}: {{ldc Greeting$Trait$Helper; ldc "$init$"; ...
> invokestatic ScriptBytecodeAdapter.invokeMethodN}}
> * {{DynamicGreeter.<init>}}: {{invokedynamic
> invoke:(Class,LDynamicGreeter;)Object}}
> * the static initializer of both: {{invokestatic
> Greeting$Trait$Helper.$static$init$(Class)}}
> This costs a dynamic dispatch on every instantiation. A GraalVM native image
> has to register every trait helper's {{$init$}} for reflection, or
> construction fails with "No signature of static method: $init$".
> Proposed fix: give the {{$init$}} call its method target, the
> {{$init$(TraitType)}} method the helper declares, so constructors call it
> with {{invokestatic}} in dynamic, {{@TypeChecked}} and {{@CompileStatic}}
> classes.
> PR: https://github.com/apache/groovy/pull/3025
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
