Repository navigation
GROOVY-12563: SC: give closures and lambdas of a trait method the trait instance - #3092
codeconsole wants to merge 10 commits into
Conversation
In a @CompileStatic or @TypeChecked trait method, a closure that reads owner, thisObject, delegate (without @DelegatesTo), getOwner() or getThisObject() failed with a GroovyCastException: "Cannot cast object 'C@...' with class 'C' to class 'T$Trait$Helper'", or to java.lang.Class in a field initializer. The type checker types these as the enclosing class, which in a trait is the static helper class (or Class<helper> in the static $init$ method of a field initializer), and the closure's doCall return type or a cast follows that type. But the trait transformation rehydrates every closure written in a trait instance method or instance field initializer to the $self parameter, so at run time they are the instance the trait is applied to. The type checker now types the this object of such a closure, and its owner and delegate when it is not nested in another closure, as the type of $self (the trait, with its @selftype types), as it types this. A closure nested in a lambda is left as it was; the next commit deals with it. Static trait methods are unchanged: their closures are rehydrated to the Class, which the type checker already types as a Class. Owner and thisObject now work in all these closures, including through the adapter with groovy.target.closure.sam=true.
…instance
Under @CompileStatic a lambda is not rehydrated in a trait method, and
is created in the static helper method with the helper class as its
owner and this object. A closure written in the lambda took its this
object from the lambda, so it was the helper class rather than the
instance the trait is applied to: thisObject, an implicit call of a
trait method such as helper(), and getThisObject() failed with a
ClassCastException ("class java.lang.Class cannot be cast to class T").
Under @TypeChecked the lambda is a rehydrated closure and the runtime
values were right, but the type checker typed thisObject as the helper
class there too, with the same failure.
Also, in any class, when the closure read only owner (or delegate with
no other delegate type), the lambda was compiled as non-capturing, so
the closure was created with the class of the lambda as its owner:
`{ owner }` failed with a GroovyCastException, where the owner should
be the lambda, as it is when the lambda captures anything.
Now:
- StaticTypesLambdaAnalyzer counts a read of owner, or of delegate
typed as the owner, in a closure written in the lambda as an access
to instance state, so the lambda captures and the closure's owner is
the lambda instance;
- when such a lambda, one that uses instance state and contains a
closure, is written in a trait helper method, StaticTypesLambdaWriter
passes the $self (or $static$self) parameter as the lambda's owner
and this object instead of the helper class, so a closure in it gets
the instance (or the implementing class in a static method), as it
does without static compilation;
- the type checker types thisObject of a closure in a lambda of a trait
method as the trait, as for any other closure there.
The owner of such a closure is the lambda, as in a class and as without
static compilation (a closure there), and implicit trait method calls,
owner, thisObject and delegate give the results dynamic Groovy gives.
In a closure written in a trait method, the trait transformation turns
`this` into the $self parameter, but left the receiver of an explicit
`this.m(...)` call as it was, unless m is a private or static method of
the trait. Since the closure is written in a static helper method,
`this` there is the helper class, so the call went to the helper class:
- with @CompileStatic: ClassCastException ("class java.lang.Class cannot
be cast to class T$Trait$Helper") for any such call, including in a
field initializer, a nested closure and a lambda;
- with @TypeChecked or dynamic code: MissingMethodException ("No
signature of static method: base for class: T$Trait$Helper") for a
method with arguments, a @selftype method or an abstract method
implemented by the class; this.toString() and this.getClass() gave
the helper class's values; and this.helper() without arguments only
seemed to work because it called the helper's static helper($self)
with null for $self;
- in a static trait method, this.getName() gave the helper's name.
An explicit `this` receiver in a closure (or lambda) of a trait method
is now $self (or $static$self in a static method), as `this` alone
already is. An implicit-this call in a closure is unchanged: it is
still resolved through the closure's owner and delegate.
…ts this object The previous commit passed $self (or $static$self) as owner and this object only to a lambda that contains a closure. A lambda written directly in a trait method under @CompileStatic resolves an implicit call of a trait method through its own this object as well, so `Supplier<String> s = () -> helper()` failed with "class java.lang.Class cannot be cast to class T": the lambda was created with the helper class as its this object. Any lambda that uses instance state (an implicit this, thisObject, or the owner of a closure in it) and is written in a trait helper method now gets the self parameter as its owner and this object. Other lambdas are created as before.
…ing class Under @CompileStatic, calling a static closure field of a trait as a method, `f()` or `this.f()`, failed with "No signature of static method: T__f$get for class: T". The type checker turns such a call into a call of the property's closure, and the static compiler read the property with the static accessor the trait interface declares, T.getF(). That accessor runs the helper with the trait class itself as $static$self, but the field is a field of the class the trait is applied to. This happened in static and instance trait methods, in closures and lambdas in them, and in static and instance field initializers; f.call(), and @TypeChecked or dynamic code, already read the field through the implementing class. - The type checker makes such a property read dynamic when it goes through the $self or $static$self parameter of a trait helper method, or through the implicit this of a closure or lambda in one, and names a static property of the trait, so it reads the property of the class the trait is applied to. - StaticTypesLambdaAnalyzer no longer counts a trait among the outer classes whose static members a non-capturing lambda may reach by class name: for a trait, that is again the trait's own accessor. The lambda now captures, and its this object is $static$self (previous commit).
|
The failed |
daniellansun
left a comment
There was a problem hiding this comment.
Ambitious but coherent fix set for GROOVY-12563: type owner/thisObject via makeThisObject() in trait helpers, rehydrate explicit this.m() to $self/$static$self in TraitReceiverTransformer, pass trait self into lambdas that touch instance members / nested closure owner, skip trait static properties in OuterStaticMemberResolver, and dynamically resolve static trait closure fields on the implementor.
TraitWithClosureOrLambda / LambdaTest additions are thorough (TC and SC, SelfType, nested, static fields-as-methods).
Before approve:
- CI: same coverage CME as #3089 — COMMENT until #3094 lands / coverage re-run green.
- TraitReceiverTransformer:
implicitThisInClosure ? thisExpr : weavedchanges explicit-this in closures — confirm dynamic (non-SC) trait behavior unchanged in your scripted tests (you assertScript without SC too; good — please call that out in the PR body). - Interaction with recent trait-helper / precompiled trait work (e.g. GROOVY-10932 line) — any known conflict?
isStaticPropertyOfTraitSelf+ DYNAMIC_RESOLUTION — failure mode if getter naming does not match (isvsget)?
Leaning positive on substance after green CI and a short note on (2)(3).
|
Hi @codeconsole — the flaky Could you please rebase onto current |
|
@daniellansun thanks.
83f2ebc also corrects the issue number in the new tests' comments (they said GROOVY-12492). The |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3092 +/- ##
==================================================
+ Coverage 74.1016% 74.1107% +0.0090%
- Complexity 43596 43626 +30
==================================================
Files 1692 1692
Lines 149245 149297 +52
Branches 28284 28305 +21
==================================================
+ Hits 110593 110645 +52
+ Misses 28475 28473 -2
- Partials 10177 10179 +2
🚀 New features to boost your workflow:
|
✅ All tests passed ✅🏷️ Commit: 55476c5 Learn more about TestLens at testlens.app/docs. |
daniellansun
left a comment
There was a problem hiding this comment.
Substance looks good for GROOVY-12563 on tip 55476c5f8b45.
Your follow-ups address the earlier points well:
- master merge brings the LSP CME fix;
- the dynamic (non-SC) comparison and
testExplicitThisInDynamicTraitClosuremake the intentional behaviour change explicit — master wrongly routed explicitthisthrough the helper; - no conflict with the recent
@Virtual/ precompiled-trait work inTraitReceiverTransformer; isStaticPropertyOfTraitSelf+DYNAMIC_RESOLUTIONfor static trait closure properties is documented and scoped to$self/$static$self(and implicit-this inside closures of trait helpers).
I am holding at COMMENT only because the LTS matrix on this tip is still incomplete (lts (17, ubuntu/macOS/windows) pending). Once those finish green, I am happy to raise this to APPROVE without further code changes.
|
@daniellansun The LTS matrix has finished green on this tip. |
In a statically compiled trait method, closures and lambdas got the static helper class where the trait instance, or the implementing class in a static method, is expected:
Five commits:
@SelfTypetypes), since the trait transformation rehydrates the closure to$self.owner,thisObject,delegatewithout@DelegatesTo,getOwner()andgetThisObject()failed with a GroovyCastException, under@TypeCheckedtoo.$self(or$static$self) as its owner and this object instead of the helper class. In any class, a lambda whose closure reads onlyownernow captures, so the closure's owner is the lambda and not the lambda's class.this.m()in a closure or lambda of a trait method callsmon the trait instance, asthisalone already resolves. It called the helper class in all three modes: a ClassCastException under@CompileStatic, a MissingMethodException for a method with arguments, a@SelfTypemethod or an abstract method in dynamic and@TypeCheckedcode, and the helper'stoString()andgetClass().$selfas its owner and this object, so() -> helper()works.f()on a static closure field of a trait reads the field of the implementing class, asf.call()and dynamic code do, instead of going through the trait's static accessor.Dynamic code changes only where it called the helper class: an explicit
this.m()in a closure of a trait method went toT$Trait$Helperthere too, giving a MissingMethodException, the helper'stoString()/getClass(), or the trait's own method running with a null$selfinstead of the implementing class's override. Of 51 dynamic scripts compared onmasterand on this branch (this.m(),this.prop,thispassed out, delegate strategies, rehydrated and nested closures, in instance and static trait methods), every casemastergot right gives the same result. The helper ABI is unchanged: only the helper's own closure classes differ, so traits compiled by either release link with classes compiled by the other.Tests
TraitWithClosureOrLambda.testOwnerAndThisObjectOfTraitClosure,testClosureInTraitLambda,testExplicitThisMethodCallInTraitClosure,testStaticClosureFieldCalledAsMethod,testExplicitThisInDynamicTraitClosureandtestStaticBooleanPropertyAndExplicitAccessorsInTraitClosure, andLambdaTest.testOwnerOfClosureInLambda, fail without the change. 12,903 tests in thegroovy,gls,bugs, STC/SC andorg.codehaus.groovy.transformpackages pass, and the Checkstyle and CodeNarc gates pass.Seen in 4.0.33, 5.0.8 and 6.0.0 (the static closure field is a type-checking error there before 6.0.0).