Skip to content

GROOVY-12576: SC: resolve Object methods in closures through the delegate and owner - #3102

Open
codeconsole wants to merge 2 commits into
apache:masterfrom
codeconsole:trait-sc-closure-object-methods
Open

codeconsole wants to merge 2 commits into
apache:masterfrom
codeconsole:trait-sc-closure-object-methods

Conversation

@codeconsole

Copy link
Copy Markdown
Contributor

Under @CompileStatic, toString(), hashCode(), equals() and getClass() 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 @TypeChecked code and as they did in 4.0. So new StringBuilder().with { append('w'); toString() } returns 'w', not the closure's toString(). In a lambda, toString() is the enclosing instance's, as in Java. In a static method it is the class's. Methods that Closure declares, such as getDelegate() or getMetaClass(), still resolve against the closure.

The type checker looked up the methods of Closure before the delegate and the owner, and that lookup also returned the methods Closure inherits from Object. It now leaves those out, and the static invocation writer calls a method of Object that 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 is null.

Tests: ClosuresSTCTest.testClosureObjectMethodPrecedence (also run statically compiled by ClosuresStaticCompileTest) and ClosuresStaticCompileTest.testObjectMethodsInClosureWithoutThisObject, which fail without the change. The groovy, gls, bugs and org.codehaus.groovy packages pass (15,513 tests), apart from bugs.Groovy12363, which also fails on master when it runs after bugs.Groovy12142 in 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.

… 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-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.20690% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.6773%. Comparing base (3869743) to head (0a8afae).

Files with missing lines Patch % Lines
...groovy/classgen/asm/sc/StaticInvocationWriter.java 85.7143% 1 Missing and 3 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@                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                
Files with missing lines Coverage Δ
...roovy/transform/stc/StaticTypeCheckingVisitor.java 87.4691% <100.0000%> (+0.0028%) ⬆️
...groovy/classgen/asm/sc/StaticInvocationWriter.java 75.8403% <85.7143%> (+0.3979%) ⬆️

... and 12 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@testlens-app

testlens-app Bot commented Oct 30, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: 0a8afae
▶️ Tests: 99392 executed
⚪️ Checks: 26/26 completed


Learn more about TestLens at testlens.app/docs.

@daniellansun daniellansun left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants