[
https://issues.apache.org/jira/browse/GROOVY-12659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18137368#comment-18137368
]
ASF GitHub Bot commented on GROOVY-12659:
-----------------------------------------
codeconsole opened a new pull request, #3180:
URL: https://github.com/apache/groovy/pull/3180
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 in a ternary or elvis
branch, or behind a call that wraps or copies it, was packed even when that
value escapes:
```groovy
class Holder { Closure handler }
@groovy.transform.PackedClosures
class C {
void install(Holder h, boolean b) { h.handler = b ? { Integer x -> x + 1
} : null }
Closure make() { return { Integer x, Integer y -> x + y }.curry(1) }
}
def h = new Holder()
new C().install(h, true)
h.handler.delegate = 'd' // UnsupportedOperationException: Cannot set a
delegate on a @PackedClosures closure
```
`CurriedClosure` (from `curry`, `rcurry`, `ncurry`) and the `asWritable`
wrapper pass `setDelegate` on to the closure they hold. `clone`, `dehydrate`
and `rehydrate` return a copy, and the copy of a packed closure is still
packed. Seen in 6.0.0 too.
The walk now follows a value marked as escaping through both branches of a
ternary or elvis, through a cast to `Object` or a `Closure` type, and into the
receiver of `curry`, `rcurry`, `ncurry`, `asWritable`, `clone`, `dehydrate` and
`rehydrate`. This covers a store to a field, property or index, a `<<` append,
a `return` (implicit or explicit), and a list or map literal. `trampoline` and
the `memoize` family are not followed: their wrappers keep a delegate of their
own and never pass it on, so a packed closure inside them behaves as an
unpacked one does. A SAM coercion is not followed either. Uses inside the
method still pack, for example `(b ? { .. } : { .. })(1)` and `{ ..
}.curry(1)(2)`.
Under `@CompileStatic` a property store or `<<` append is rewritten into a
setter or `leftShift` call before class generation; those two shapes are
declined there only together with #3171 (GROOVY-12650). Returns, stores to a
field of the same class, and collection literals are declined in every mode.
Merging with the other escape-walk PRs: with #3177 (GROOVY-12656) it merges
cleanly; afterwards its composition branches can call `markValue` instead of
`mark` so a ternary inside a composition is followed too. With #3172
(GROOVY-12651) one hunk in `visitBinaryExpression` conflicts: keep #3172's
operator conditions and call `markValue` in both branches.
## Tests
`PackedClosuresTransformTest.conditionalAndWrappedClosuresEscapeWhereTheirValueDoes`
(dynamic and `@TypeChecked`),
`PackedClosuresTransformTest.compileStaticConditionalAndWrappedEscapesAreDeclined`,
and a new 'returned from a ternary' 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 that escapes through a ternary, an
> elvis or a curry/asWritable/clone call
> -----------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12659
> URL: https://issues.apache.org/jira/browse/GROOVY-12659
> 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, boolean b) { h.handler = b ? { Integer x -> x + 1
> } : null }
> Closure make() { return { Integer x, Integer y -> x + y }.curry(1) }
> }
> def h = new Holder()
> new C().install(h, true)
> h.handler.delegate = 'd'
> new C().make().delegate = 'd'
> {code}
> *Actual:* both delegate assignments throw {{UnsupportedOperationException:
> Cannot set a delegate on a @PackedClosures closure}}. The same happens for an
> elvis and for a literal behind rcurry, ncurry, asWritable, clone, dehydrate
> or rehydrate, when the result is stored to a field, property or index,
> appended with {{<<}}, returned, or placed in a list or map literal.
> *Expected:* the literal is declined from packing, as it is when it is itself
> the stored or returned value, and the delegate can be set.
> Cause: the escape walk in {{ClosureWriter.collectEscapingClosures}} marks a
> literal only when it is the escaping expression itself. It does not follow
> the value through ternary and elvis branches, through casts, or into the
> curried or wrapped closure. CurriedClosure and the asWritable wrapper pass
> setDelegate on to the packed closure, and clone, dehydrate and rehydrate
> return a packed copy.
> Verified on 6.0.0 and master.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
