[
https://issues.apache.org/jira/browse/GROOVY-12656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18137338#comment-18137338
]
ASF GitHub Bot commented on GROOVY-12656:
-----------------------------------------
codeconsole opened a new pull request, #3177:
URL: https://github.com/apache/groovy/pull/3177
With closure packing (`@PackedClosures` or
`-Dgroovy.target.closure.pack=true`), the escape walk in
`ClosureWriter.collectEscapingClosures` marked only a closure literal that is
the stored, appended or returned value itself. A literal composed with `>>` or
`<<` was packed even when the composition escapes:
```groovy
class Holder { Closure handler }
@groovy.transform.PackedClosures
class C {
void install(Holder h, Closure c) { h.handler = c >> { x -> x + 1 } }
Closure make(Closure c) { return { x -> x + 1 } << c }
}
def h = new Holder()
new C().install(h, { x -> x * 2 })
h.handler.delegate = 'd' // UnsupportedOperationException: Cannot set a
delegate on a @PackedClosures closure
```
`ComposedClosure` keeps a clone of each operand, and the clone of a packed
closure is still packed. It passes `setDelegate` on to them, so the literal
escapes with the composition. Seen in 6.0.0 too.
The walk now follows a value that is marked as escaping through `>>` and
`<<`, nested compositions included, and through one-argument `rightShift` and
`leftShift` calls. This covers a store to a field, property or index, a `<<`
append, a `return`, and a list or map literal. A composition used only in the
method still packs, for example `(c >> { x -> x + 1 })(n)`. A standalone
`leftShift(...)` call is still not an escape.
Under `@CompileStatic`, `BinaryExpressionTransformer` turns `c >> { ... }`
into a `rightShift` call before class generation, and the walk follows that
form. Returned compositions, compositions stored to a field of the same class,
and compositions in collection literals are therefore declined in every mode. A
composition stored through a property setter or appended with `<<` is rewritten
into a setter or `leftShift` call; under `@CompileStatic` those shapes are
declined only together with #3171 (GROOVY-12650), which runs the escape walk
before that rewrite.
## Tests
`PackedClosuresTransformTest.composedClosuresEscapeWhereTheCompositionDoes`
(dynamic and `@TypeChecked`),
`PackedClosuresTransformTest.compileStaticComposedEscapesAreDeclined` and a new
'returned composed' row in `PackedClosureBoundariesTest.boundaries` fail
without the change. The closure-packing test classes,
`org.codehaus.groovy.classgen.asm.sc.*` and `groovy.transform.stc.*` (including
`ClosuresSTCTest`) pass. The Checkstyle and CodeNarc gates pass.
> Closure packing packs a closure literal composed with >> or << whose
> composition escapes
> ----------------------------------------------------------------------------------------
>
> Key: GROOVY-12656
> URL: https://issues.apache.org/jira/browse/GROOVY-12656
> Project: Groovy
> Issue Type: Bug
> Affects Versions: 7.0.0-alpha-1, 6.0.0
> Reporter: Scott Murphy Heiberg
> Priority: Major
>
> {code:groovy}
> class Holder { Closure handler }
> @groovy.transform.PackedClosures
> class C {
> void install(Holder h, Closure c) { h.handler = c >> { x -> x + 1 } }
> Closure make(Closure c) { return { x -> x + 1 } << c }
> }
> def h = new Holder()
> new C().install(h, { x -> x * 2 })
> h.handler.delegate = 'd'
> def r = new C().make({ x -> x * 2 })
> r.delegate = 'd'
> {code}
> *Actual:* both delegate assignments throw
> {{java.lang.UnsupportedOperationException: Cannot set a delegate on a
> @PackedClosures closure (hoisted method '$packed$closure$0') ...}}. The same
> happens with {{-Dgroovy.target.closure.pack=true}} and under {{@TypeChecked}}
> and {{@CompileStatic}}.
> *Expected:* the closure literal is declined from packing, as it is when
> stored or returned directly, and setting a delegate works as it does without
> packing.
> Cause: the compile-time escape walk in
> {{ClosureWriter.collectEscapingClosures}} marks only a literal that is itself
> the stored, appended or returned value. Composition builds a ComposedClosure
> that keeps a clone of the literal (the clone of a packed closure is still
> packed) and passes setDelegate on to it, so the literal escapes along with
> the composition.
> Verified on 6.0.0 and master.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
