[
https://issues.apache.org/jira/browse/GROOVY-12655?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18137337#comment-18137337
]
ASF GitHub Bot commented on GROOVY-12655:
-----------------------------------------
codeconsole opened a new pull request, #3176:
URL: https://github.com/apache/groovy/pull/3176
With closure packing (`@PackedClosures` or
`-Dgroovy.target.closure.pack=true`), the compile-time escape gate in
`ClosureWriter` treated every assignment target that is not a `FieldNode` or
`PropertyNode` as a local. A bare name bound to a `DynamicVariable` is not a
local. It is stored through the MOP into the script binding, or into a dynamic
property of the owner, where code outside the method reads it:
```groovy
@groovy.transform.PackedClosures
def install() { handler = { x -> x + 1 } }
install()
handler.delegate = 'd' // UnsupportedOperationException: Cannot set a
delegate on a @PackedClosures closure
```
The same thing happened when a method of an annotated class stores to an
undeclared name that `propertyMissing` takes, when a script method stores to an
`@Field` (outside `run()` still a `DynamicVariable`), and, with the flag, for a
binding store in a script's `run()` and in `@CompileStatic` code whose type
checking extension makes the variable dynamic. Seen in 6.0.0 too.
`ClosureWriter.isLocalTarget` now treats a `DynamicVariable` like a field or
property, so such a store is declined as an escape. Stores to parameters and
declared locals still pack. The `@PackedClosures` Javadoc lists binding
variables among the escapes.
The compound stores of #3172 (GROOVY-12651) go through `isLocalTarget`, so
they pick this up once both are merged.
## Tests
`PackedClosuresTransformTest.bindingAndDynamicPropertyStoresAreDeclinedAtCompileTime`
(script binding, `@Field` from a script method, a dynamic property of an
annotated class) and
`PackedClosuresTransformTest.flagPathDeclinesBindingStores` (script `run()`,
and `@CompileStatic` with a `makeDynamic` extension) 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 stored to a script binding variable or
> dynamic property, which then rejects a delegate
> ----------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12655
> URL: https://issues.apache.org/jira/browse/GROOVY-12655
> Project: Groovy
> Issue Type: Bug
> Affects Versions: 7.0.0-alpha-1, 6.0.0
> Reporter: Scott Murphy Heiberg
> Priority: Major
>
> {code:groovy}
> @groovy.transform.PackedClosures
> def install() { handler = { x -> x + 1 } }
> install()
> handler.delegate = 'd'
> handler(1)
> {code}
> *Actual:* {{java.lang.UnsupportedOperationException: Cannot set a delegate on
> a @PackedClosures closure (hoisted method '$packed$closure$0') ...}}
> *Expected:* the closure is not packed, because it escapes into the script
> binding, and the script returns 2 as it does without the annotation.
> The same happens when a method of an annotated class stores to an undeclared
> name (a dynamic property, e.g. one taken by {{propertyMissing}}) and when a
> script method stores to an {{@Field}}. With
> {{-Dgroovy.target.closure.pack=true}}, a binding store in a script's run
> method is packed too, and so is one in {{@CompileStatic}} code whose type
> checking extension makes the variable dynamic.
> Cause: the escape gate in {{ClosureWriter}} ({{isLocalTarget}}) treats any
> assignment target that is not a field or property node as a local, including
> a {{DynamicVariable}}, which is stored through the MOP where code outside the
> method can read it.
> Verified on 6.0.0 and master.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
