[
https://issues.apache.org/jira/browse/GROOVY-12670?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18137444#comment-18137444
]
ASF GitHub Bot commented on GROOVY-12670:
-----------------------------------------
codeconsole opened a new pull request, #3191:
URL: https://github.com/apache/groovy/pull/3191
With `-Dgroovy.target.closure.pack=true`, dynamic code packs a closure whose
body has no free name, using
`ClosureWriter.isSyntacticallyDelegateIndependent`. That check accepted a body
that names `this`: `{ -> this }`, `{ -> this.foo() }`, `this.@f`, `this.&m`,
and `Outer.this` / `Outer.super` in an inner class, including inside nested
closures. A closure class reads these through `getThisObject()`, which
`rehydrate` replaces. The hoisted body of a packed closure keeps the instance
it was created on, so a rehydrated copy answered for the original owner:
```groovy
class D { String foo() { 'D.foo' }; String toString() { 'D' } }
class O {
String name
String foo() { 'foo ' + name }
String toString() { 'O:' + name }
List run(O b, D d) {
Closure c = { -> this }
Closure e = { -> this.foo() }
[c.rehydrate(null, b, b)(), c.rehydrate(null, d, d)(),
e.rehydrate(null, b, b)(), e.rehydrate(null, d, d)()]
}
}
println new O(name: 'a').run(new O(name: 'b'), new D())
// with the flag: [O:a, O:a, foo a, foo a]; without it: [O:b, D, foo b,
D.foo]
```
Seen in 6.0.0 too.
The syntactic check now declines an explicit `this` and an `Outer.this` /
`Outer.super` property expression, so these closures keep their generated
class. Closures with no free name still pack. The `@CompileStatic` proof path
(see #3187, GROOVY-12666) and the `@PackedClosures` trust path are unchanged.
Merging with #3187: its test
`ClosurePackCapabilityTest.syntacticallyPackedClosureRehydratedOntoAnInstanceOfItsOwnerClassUsesIt`
asserts that the dynamic `{ -> this.foo() }` is a `PackedClosure`; after this
change it is a closure class, so that `instanceof PackedClosure` assertion has
to go (the expected values still hold).
## Tests
`ClosurePackCapabilityTest.explicitThisIsNotSyntacticallyIndependent`,
`ClosurePackCapabilityTest.outerThisAndOuterSuperAreNotSyntacticallyIndependent`
and the new 'explicit this' rows of `PackedClosureBoundariesTest.boundaries`
fail without the change. The packing test classes, `groovy.lang.*Closure*`,
`groovy.ClosureTest`, `org.codehaus.groovy.classgen.asm.sc.*`,
`groovy.transform.stc.*` and `groovy.ThisAndSuperTest` pass. The Checkstyle and
CodeNarc gates pass.
> Closure packing (groovy.target.closure.pack) packs a dynamic closure that
> names this, so a rehydrated copy still answers for the original owner
> -----------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12670
> URL: https://issues.apache.org/jira/browse/GROOVY-12670
> Project: Groovy
> Issue Type: Bug
> Affects Versions: 7.0.0-alpha-1, 6.0.0
> Reporter: Scott Murphy Heiberg
> Priority: Major
>
> With {{-Dgroovy.target.closure.pack=true}}, dynamic code (no annotation)
> packs a closure literal whose body has no free name. The check accepts an
> explicit this, a method call on this, this.@field, this.&method, Outer.this
> and Outer.super.
> {code:groovy}
> class D { String foo() { 'D.foo' }; String toString() { 'D' } }
> class O {
> String name
> String foo() { 'foo ' + name }
> String toString() { 'O:' + name }
> List run(O b, D d) {
> Closure c = { -> this }
> Closure e = { -> this.foo() }
> [c.rehydrate(null, b, b)(), c.rehydrate(null, d, d)(),
> e.rehydrate(null, b, b)(), e.rehydrate(null, d, d)()]
> }
> }
> println new O(name: 'a').run(new O(name: 'b'), new D())
> {code}
> *Actual* (with the flag): {{[O:a, O:a, foo a, foo a]}}
> *Expected* (as without the flag): {{[O:b, D, foo b, D.foo]}}
> A closure class reads an explicit this from getThisObject(), which rehydrate
> replaces. The packed closure's body is hoisted into a method of the owner
> class and keeps the instance it was created on. The syntactic no-free-name
> check ({{ClosureWriter.isSyntacticallyDelegateIndependent}}) should treat a
> body that names this as not packable.
> Verified on 6.0.0 and master.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
