[
https://issues.apache.org/jira/browse/GROOVY-12666?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18137427#comment-18137427
]
ASF GitHub Bot commented on GROOVY-12666:
-----------------------------------------
codeconsole opened a new pull request, #3187:
URL: https://github.com/apache/groovy/pull/3187
A `@CompileStatic` closure class runs its body against `getThisObject()`. So
a copy made by `rehydrate` reads the new this object's members, and throws
`ClassCastException` when called if that object is not an instance of the
enclosing class. A packed closure (`@PackedClosures` or
`-Dgroovy.target.closure.pack=true`) dispatched to the receiver captured at
construction, so the copy silently kept using the original owner:
```groovy
@CompileStatic @PackedClosures
class O {
String name
String foo() { 'foo ' + name }
List run(O b) {
Closure c = { -> foo() }
[c.rehydrate(null, b, b)(), c.rehydrate(new D(), new D(), new D())()]
}
}
new O(name: 'a').run(new O(name: 'b')) // was [foo a, foo a]; the closure
class gives foo b, then ClassCastException
```
Seen in 6.0.0 too.
On the non-strict path, `PackedClosure.rehydrate` now gives the copy the new
this object as its receiver:
- always, when the compiler found that the hoisted body uses its receiver
(`this`, an instance field or property, an implicit-this instance method call
other than an extension method, or an inner class creation). For an object that
is not an instance of the enclosing class, the dispatch table's checkcast then
throws the same `ClassCastException` as the closure class;
- otherwise, only when the object is an instance of the receiver's class,
which is always sound.
A body that does not use its receiver keeps the original one for an
unrelated this object, so rehydrating it onto a builder (the DSL idiom) keeps
working. So does a null this object (`dehydrate`).
`StaticTypesClosureWriter.hoistedBodyUsesReceiver` computes the bit. It
reaches the adapter through a new ten-argument
`ScriptBytecodeAdapter.packedClosure` (`@GroovyABI(since = "7.0.0")`, added to
`compatibility/groovy-abi-surface.json`). The nine-argument factory stays for
class files compiled by 6.0.0 and passes false, so those get the always-sound
instance swap. A 6.0.x backport would need the patch version in the ABI
annotation instead. The dynamic `@PackedClosures` trust path is not changed.
Merging with #3182 (GROOVY-12661): `PackedClosure.rehydrate` conflicts; put
#3182's `if (strict) { ... }` guard first, followed by this change's non-strict
receiver swap.
Two shapes the hoisted body cannot reproduce for a this object that is not
an instance of the enclosing class: an implicit-this call to an `Object` method
now throws `ClassCastException` instead of silently using the old receiver, and
an implicit-this extension method that reads its self argument keeps the old
receiver, because the `println`/`sprintf` family must keep working there. For
an instance of the enclosing class both are correct.
## Tests
`PackedClosuresTransformTest.compileStaticRehydrateDispatchesToTheNewThisObject`
(16 shapes against an instance, a subclass instance and a foreign object,
compared with the same literals compiled unpacked),
`rehydratedCompileStaticPackedClosureKeepsItsReceiverForNullThisObject` and
`packedClosureFromTheEarlierFactoryRehydratesOntoAnInstanceOfItsReceiverClass`,
and
`ClosurePackCapabilityTest.autoPackedClosureRehydratedOntoAnotherOwnerUsesIt`
and `syntacticallyPackedClosureRehydratedOntoAnInstanceOfItsOwnerClassUsesIt`
fail without the change. The packing test classes, `groovy.lang.*Closure*`,
`groovy.ClosureTest`, `org.codehaus.groovy.classgen.asm.sc.*` and
`groovy.transform.stc.*` pass. The Checkstyle and CodeNarc gates pass.
> Rehydrating a @CompileStatic packed closure ignores the new this object and
> keeps calling the original owner
> ------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12666
> URL: https://issues.apache.org/jira/browse/GROOVY-12666
> Project: Groovy
> Issue Type: Bug
> Affects Versions: 7.0.0-alpha-1, 6.0.0
> Reporter: Scott Murphy Heiberg
> Priority: Major
>
> {code:groovy}
> import groovy.transform.CompileStatic
> import groovy.transform.PackedClosures
> class D {}
> @CompileStatic
> @PackedClosures
> class O {
> String name
> String foo() { 'foo ' + name }
> List run(O b) {
> Closure c1 = { -> foo() }
> Closure c2 = { -> name }
> [c1.rehydrate(null, b, b)(), c2.rehydrate(null, b, b)(),
> c1.rehydrate(new D(), new D(), new D())()]
> }
> }
> println new O(name: 'a').run(new O(name: 'b'))
> {code}
> *Actual:* {{[foo a, a, foo a]}}. The same happens without the annotation when
> compiling with {{-Dgroovy.target.closure.pack=true}}.
> *Expected*, as with the closure class compiled without packing: {{[foo b,
> b]}}, then a ClassCastException from the third call, because D is not an O.
> A @CompileStatic closure class runs its body against getThisObject(). A
> packed closure dispatches its hoisted method to the receiver captured at
> construction, and Closure.rehydrate only replaces the visible owner, delegate
> and this object, so the new this object is silently ignored.
> Verified on 6.0.0 and master.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
