[ 
https://issues.apache.org/jira/browse/GROOVY-11985?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18080137#comment-18080137
 ] 

Paul King commented on GROOVY-11985:
------------------------------------

*Static-method dispatch in traits (GROOVY-11985)*

Inside a trait body, an unqualified static-method call (e.g. \{{m()}}) or a 
\{{this.}}-qualified static-method call (e.g. \{{this.m()}}) to a *public* 
trait static is rewritten to invoke the method on the trait's direct 
implementer — that is, the class whose \{{implements}} clause names the trait 
and which therefore received the trait's compile-time forwarders. An 
implementing class may override a trait's static defaults by declaring a 
same-signature static method, and the override will be visible to trait code 
calling that method.

The direct implementer is fixed at trait-composition time: subclasses of a 
trait implementer do *not* re-compose the trait. Consequently, if class \{{A}} 
implements trait \{{T}} (with static \{{m1}} calling static \{{m2}}) and class 
\{{B}} extends \{{A}}, calling \{{B.m1()}} dispatches \{{m2}} through \{{A}} — 
even if \{{B}} declares its own \{{static m2}}. To make \{{B}}'s override 
visible to \{{m1}}, \{{B}} must also redeclare \{{m1}} (the new forwarder will 
then carry \{{B}} as its receiver). This matches JVM semantics for inherited 
static methods: the static receiver in source code does not influence dispatch.

Worked example:

{code:groovy}
trait T {
    static String m2() \{ 'T.m2' }
    static String m1() \{ m2() }            // unqualified
    static String m1This() \{ this.m2() }   // this.-qualified
}
class A implements T \{ static String m2() { 'A.m2' } }
class B extends A    \{ static String m2() { 'B.m2' } }
class C extends A    {}                    // inherits A.m2
class D implements T {}                    // no override
class E extends D    {}                    // inherits T.m2
class F extends D    \{ static String m2() { 'F.m2' } }
{code}

||Receiver||\{{m2()}} direct||\{{m1()}} (unqualified)||\{{m1This()}}||
|\{{A}}|\{{A.m2}}|\{{A.m2}}|\{{A.m2}}|
|\{{B}}|\{{B.m2}}|\{{A.m2}}|\{{A.m2}}|
|\{{C}}|\{{A.m2}}|\{{A.m2}}|\{{A.m2}}|
|\{{D}}|\{{T.m2}}|\{{T.m2}}|\{{T.m2}}|
|\{{E}}|\{{T.m2}}|\{{T.m2}}|\{{T.m2}}|
|\{{F}}|\{{F.m2}}|\{{T.m2}}|\{{T.m2}}|

The bold cells for \{{B}} and \{{F}} show the load-bearing point: the subclass 
override of \{{m2}} is *not* visible to \{{m1}} called through the subclass, 
because \{{m1}}'s forwarder was baked into the direct implementer (\{{A}} or 
\{{D}}) at composition time.

*Exceptions:*

* A *private* static is not composed onto the implementing class and continues 
to dispatch to the trait's helper directly; the override pattern is 
intentionally unavailable for private statics.
* A static call made from inside a *closure body* inside a trait method also 
dispatches to the helper rather than through the implementer, preserving 
compile-time type checking of trait closure bodies at the cost of polymorphism 
in that position. This is a known difference from Groovy 4 and may be revisited 
in a future revision.

*Qualified by trait name:*

A call qualified by the trait name (e.g. \{{MyTrait.m()}}) inside trait code 
targets the trait's compiled body via the helper. Note that direct external 
access of the form \{{MyTrait.m()}} is *not* supported because the trait 
interface carries no static members.

*Overload selection:*

Overload selection uses the method's natural signature; the call dispatches 
through the direct implementer's copy, hitting an override declared there if 
one exists or the trait's compiled default otherwise.

> Static method override on trait implementer ignored when called via this in 
> trait body
> --------------------------------------------------------------------------------------
>
>                 Key: GROOVY-11985
>                 URL: https://issues.apache.org/jira/browse/GROOVY-11985
>             Project: Groovy
>          Issue Type: Bug
>            Reporter: Paul King
>            Priority: Major
>
> Analysis by AI of the description in the Grails canary build comments yielded 
> the following:
> h2. Summary
> In Groovy 4.x, calling {{this.someStaticMethod()}} from inside a trait's 
> static method body dispatched dynamically and honoured a static method 
> override declared on the implementing class. In Groovy 5.x and 
> 6.0.0-SNAPSHOT, the same call is rewritten by {{TraitReceiverTransformer}} to 
> dispatch through the trait helper, which can never see the implementing-class 
> override. The override is silently lost — no exception, no compile warning, 
> the trait's default value just always wins.
> This was discovered as part of the Grails 8 / Groovy 5 migration 
> (apache/grails-core PRs 
> [#15557|https://github.com/apache/grails-core/pull/15557] and 
> [#15558|https://github.com/apache/grails-core/pull/15558]) and motivated a 
> reflection-based workaround in 
> {{{}Validateable.resolveDefaultNullable(Class){}}}.
> h2. Reproducer
> Standalone repro: 
> [https://github.com/jamesfredley/groovy-trait-static-method-override-bug]
> {code:java|title=Validateable.groovy}
> trait Validateable {
>     static boolean defaultNullable() {
>         false
>     }
>     static boolean defaultNullableSeenByTrait() {
>         // expected to dispatch to the implementing class override
>         this.defaultNullable()
>     }
> }
> {code}
> {code:java|title=MyNullableValidateable.groovy}
> class MyNullableValidateable implements Validateable {
>     static boolean defaultNullable() {
>         true
>     }
> }
> {code}
> {code:java|title=Driver}
> assert MyNullableValidateable.defaultNullable()           // direct call: 
> true on every version
> assert MyNullableValidateable.defaultNullableSeenByTrait() // expected true; 
> gets false on 5.x / 6.0
> {code}
> h2. Observed behaviour
> ||Groovy version||Direct call||{{this.defaultNullable()}} from trait body||
> |4.0.27|true (PASS)|true (PASS)|
> |5.0.5|true (PASS)|*false (FAIL)*|
> |6.0.0-SNAPSHOT|true (PASS)|*false (FAIL)*|
> h2. Root cause
> The bytecode emitted for the trait helper's 
> {{defaultNullableSeenByTrait(Class)}} method changed shape:
> {noformat}
> // Groovy 4.0.27                                 // Groovy 5.0.5 / 
> 6.0.0-SNAPSHOT
> 0: aload_0                                       0: ldc          // 
> Validateable$Trait$Helper.class
> 1: invokedynamic invoke:                         2: aload_0
>    (Ljava/lang/Class;)Ljava/lang/Object;         3: invokedynamic invoke:
>                                                     
> (Ljava/lang/Class;Ljava/lang/Class;)Ljava/lang/Object;
> {noformat}
> In 4.x the indy receiver is {{aload_0}} — the implementing class — so the 
> dynamic dispatch resolves {{defaultNullable()}} against 
> {{MyNullableValidateable}} and finds the override. In 5.x+ the receiver is 
> hard-coded {{Validateable$Trait$Helper.class}} (an {{{}ldc{}}}) and the 
> implementing class is demoted to an argument, so the dispatch resolves 
> {{defaultNullable(Class)}} on the trait helper itself and always lands on the 
> lowered trait default.
> The behaviour change was introduced by commit {{0aa78d0a33}} (GROOVY-8854, 
> Sep 2023):
> {quote}write {{T.m(p)}} as {{this.m($static$self,p)}} not {{$self.m(p)}}
> {quote}
> That commit rewrote {{TraitReceiverTransformer.transformMethodCallOnThis}} so 
> that {{this.someStaticMethod()}} inside a trait body is rewritten as {{(this 
> | T$Trait$Helper).m((Class)$self.getClass(), args)}} — routed through the 
> trait helper's lowered static, with the implementing class passed as the 
> {{$static$self}} argument. The existing trait static method test coverage in 
> {{TraitASTTransformationTest.testTraitStaticMethod}} (including the 
> GROOVY-8854 case at line 2218) doesn't exercise the 
> override-on-implementing-class scenario, so the regression wasn't caught.
> h2. Tradeoff
> Groovy 5's behaviour is arguably closer to Java semantics: static methods are 
> not virtual, and {{this}} inside a Java static method doesn't exist, so 
> "{{{}this.staticMethod(){}}} virtually dispatches to the implementing class's 
> static" was always Groovy-specific magic that relied on MOP. However:
>  * The Groovy 4 behaviour was depended on by real code — Grails 
> {{Validateable}} is the visible canary; the same idiom likely exists 
> elsewhere.
>  * The failure mode is silent — no exception, no compile warning, the trait 
> default just wins every time.
>  * The change surfaced as a side effect of GROOVY-8854 (whose ticket was 
> about something else), not as a deliberate "trait statics are no longer 
> virtual" decision, and it isn't called out in the Groovy 5 release notes.
> h2. Suggested options
>  # Restore Groovy 4 semantics for {{this.staticMethod()}} in trait bodies — 
> emit a dynamic lookup whose receiver is {{$static$self}} (the implementing 
> class) rather than the trait helper.
>  # At minimum, emit a compile-time warning when a trait body calls a 
> same-named static and the dispatch will provably not hit any override on the 
> implementing class.
>  # Document the new contract explicitly in the Groovy 5 release notes and 
> trait docs, so consumers can adapt at the trait level instead of debugging 
> silent no-ops.
> h2. Workaround (already applied in Grails)
> Bypass {{TraitReceiverTransformer}} via plain Java reflection, which it can't 
> see through:
> {code:groovy}
> private static boolean resolveDefaultNullable(Class<?> clazz) {
>     try {
>         return clazz.getMethod('defaultNullable').invoke(null) as boolean
>     } catch (NoSuchMethodException ignored) {
>         return false
>     }
> }
> {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to