paulk-asert commented on PR #3102:
URL: https://github.com/apache/groovy/pull/3102#issuecomment-6535519149
Thanks, this looks good overall. Two things before merge:
**1. Closures using `TO_SELF` no longer compile.** A closure whose resolve
strategy is `TO_SELF` can't call `toString()` any more:
class C {
String toString() { 'C' }
static Object run(@DelegatesTo(value=String,
strategy=Closure.TO_SELF) Closure c) {
c.resolveStrategy = Closure.TO_SELF; c.delegate = 'd'; c()
}
Object m() { run { -> toString() } }
}
On master it compiles, and both dynamic and `@CompileStatic` return the
closure's own `toString()`. With this PR, both `@CompileStatic` and
`@TypeChecked` fail with `[Static type checking] - Cannot find matching method
C#toString()`. With `TO_SELF` only the closure itself is searched, and its
`Object` methods have now been removed from that search. Could the `removeIf`
be skipped when the strategy is `TO_SELF`? A test for it would be good too.
**2. The 4.0 claim in the description.** Results for `toString()` called
inside a closure:
| | 4.0.32 | 5.0.6 | master | this PR |
|---|---|---|---|---|
| dynamic | closure | closure | owner | owner |
| `@CompileStatic` | owner | closure | closure | owner |
Dynamic code only started going to the delegate or owner with GROOVY-11858,
which is on master and 6.0.x but not on 5.1.x. So in 4.0, SC and dynamic
disagreed, and in 5.x they agree. This PR lines SC up with 6.0+ dynamic
behaviour.
For that reason I'd backport to 6.0.x (with the `TO_SELF` fix) but not to
5.1.x. On 5.1.x, the `@TypeChecked` variant of
`testClosureObjectMethodPrecedence` fails, because it runs dynamically, while
the statically compiled one passes. Backporting would split SC from dynamic
there.
Minor and optional: with `rehydrate(null, owner, null)`, dynamic code calls
the owner's method, while this PR falls back to the closure. Trying
`getOwner()` before the closure would match dynamic in that case. Not a blocker.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
