Repository navigation
GROOVY-12576: SC: resolve Object methods in closures through the delegate and owner - #3102
codeconsole wants to merge 2 commits into
Conversation
… and owner
Under static compilation, an implicit-this call to a method of Object in
a closure or lambda, such as toString(), hashCode(), equals(Object) or
getClass(), was made on the closure itself:
@CompileStatic
class C {
String m() { new StringBuilder().with { append('w'); toString() } }
}
new C().m() returned "C$_m_closure1@1b2c3d4" instead of "w". The type
checker looks up the methods of Closure before the delegate and the
owner, and that lookup also returned the methods Closure inherits from
Object. With with, tap, identity or a @DelegatesTo(strategy =
DELEGATE_FIRST) method this ignored the delegate. In a plain closure it
ignored the owner, and in a lambda it ignored the enclosing instance,
which Java and dynamic Groovy both use. This happened in methods,
constructors, field initializers and static initializers alike.
Dynamic Groovy only tries the closure first for methods that Closure or
GroovyObject declare; a method the closure inherits from elsewhere goes
to the delegate or the owner according to the resolve strategy. The type
checker now leaves the methods of Object out of that first lookup, so
the delegate or owner is chosen, and the static invocation writer calls
a method of Object that was resolved against the owner on the closure's
this object (the class itself in a static context), as it already did
for other owner methods. Methods declared by Closure, such as
getDelegate() or getMetaClass(), still resolve against the closure.
@TypeChecked code already behaved like dynamic Groovy at run time; only
the inferred types of such calls change there.
…is object is null
Under static compilation, toString(), hashCode(), equals(Object) and
getClass() called without a receiver in a closure now go to the
closure's this object, as other owner methods do. A closure can have no
this object: dehydrate() returns a copy whose owner, delegate and this
object are all null, which is how closures are made serializable or
shared between threads. For such a closure these calls failed with a
NullPointerException:
@CompileStatic
class C {
Closure<Integer> c() { { -> hashCode() } }
}
new C().c().dehydrate()() threw "Cannot invoke "Object.hashCode()"
because the return value of "C$_c_closure1.getThisObject()" is null".
Before the previous fixes these calls always went to the closure itself.
When the method is a public method of Object, or overrides one, the
static invocation writer now loads the this object and uses the closure
instead if it is null, and calls the method as declared by Object, so
the override of the this object still runs when there is one:
ALOAD 0
INVOKEVIRTUAL getThisObject()
DUP
IFNONNULL L
POP
ALOAD 0
L: INVOKEVIRTUAL java/lang/Object.hashCode()I
Lambdas are left alone, since their enclosing instance is never null.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3102 +/- ##
==================================================
+ Coverage 73.6728% 73.6773% +0.0045%
- Complexity 42587 42600 +13
==================================================
Files 1678 1678
Lines 146913 146938 +25
Branches 27792 27796 +4
==================================================
+ Hits 108235 108260 +25
Misses 28526 28526
Partials 10152 10152
🚀 New features to boost your workflow:
|
✅ All tests passed ✅🏷️ Commit: 0a8afae Learn more about TestLens at testlens.app/docs. |
daniellansun
left a comment
There was a problem hiding this comment.
LGTM for GROOVY-12576.
Under @CompileStatic, bare toString/hashCode/equals/getClass in a closure/lambda must follow resolve strategy to delegate/owner (Groovy 4 / dynamic / @TypeChecked), not the closure’s own Object methods. Dropping Object-declared methods from the closure-method short-circuit in StaticTypeCheckingVisitor, and teaching StaticInvocationWriter to rewrite the receiver (including ThisObjectOrClosureExpression for dehydrated / null-thisObject closures), is the right split.
Tests cover with/tap/identity, ctor/field/static contexts, DELEGATE_FIRST, lambdas vs closures, and dehydrate/rehydrate fallbacks in ClosuresStaticCompileTest.
CI green. Codecov ~86% patch (a few StaticInvocationWriter partials on the null-thisObject branch) — non-blocking given the dedicated dehydrate test.
Note: overlaps StaticInvocationWriter with #3103/#3106/#3107 — rebase carefully.
Under
@CompileStatic,toString(),hashCode(),equals()andgetClass()called without a receiver inside a closure or lambda now go to the delegate or the owner according to the resolve strategy, as they do in dynamic and@TypeCheckedcode and as they did in 4.0. Sonew StringBuilder().with { append('w'); toString() }returns'w', not the closure'stoString(). In a lambda,toString()is the enclosing instance's, as in Java. In a static method it is the class's. Methods thatClosuredeclares, such asgetDelegate()orgetMetaClass(), still resolve against the closure.The type checker looked up the methods of
Closurebefore the delegate and the owner, and that lookup also returned the methodsClosureinherits fromObject. It now leaves those out, and the static invocation writer calls a method ofObjectthat was resolved against the owner on the closure's this object, as it already did for other owner methods.A closure returned by
dehydrate()has no this object. If it calls one of these methods without a receiver, it uses its own method, as before; the writer loads the this object and falls back to the closure when it isnull.Tests:
ClosuresSTCTest.testClosureObjectMethodPrecedence(also run statically compiled byClosuresStaticCompileTest) andClosuresStaticCompileTest.testObjectMethodsInClosureWithoutThisObject, which fail without the change. Thegroovy,gls,bugsandorg.codehaus.groovypackages pass (15,513 tests), apart frombugs.Groovy12363, which also fails on master when it runs afterbugs.Groovy12142in the same JVM (GROOVY-12580 fixes it). The Checkstyle and CodeNarc gates pass.A regression in 5.0.0: 4.0.33 is correct, 5.0.8 and 6.0.0 are not.