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.
--
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]
