[
https://issues.apache.org/jira/browse/GROOVY-12658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18137362#comment-18137362
]
ASF GitHub Bot commented on GROOVY-12658:
-----------------------------------------
codeconsole opened a new pull request, #3179:
URL: https://github.com/apache/groovy/pull/3179
A packed closure (`@PackedClosures`, or `-Dgroovy.target.closure.pack=true`
under `@CompileStatic`) read and wrote owner properties as fields, skipping
user-defined and overriding getters and setters:
```groovy
@groovy.transform.CompileStatic
@groovy.transform.PackedClosures
class Box {
int gets, sets
BigDecimal e = 1
BigDecimal getE() { gets++; e }
void setE(BigDecimal v) { sets++; e = v }
def m() { def r = { -> e++ }(); [r, this.@e, gets, sets] }
}
assert new Box().m() == [1, 2, 1, 1] // packed: [1, 2, 0, 0]
```
The same held for `e`, `this.e`, `e = v`, `e += v` and prefix/postfix forms,
in dynamic and static compilation. It also held for a subclass override of a
generated accessor, static properties, `is` getters, and a dynamic owner
overriding `getProperty`. It made
`AssignmentsStaticCompileTest.testPostfixAndPrefixOnPropertyCallGetterOnce2`
fail with the packing flag. Seen in 6.0.0 too.
The body is hoisted into a method of the owner class.
`AsmClassGenerator.visitPropertyExpression` resolves `this.name` and implicit
`name` to the declared field in a method of the class, but not in a closure
class. A hoisted body now skips that field shortcut, so the access takes the
same route as in the closure class: the accessor under `@CompileStatic` (a
plain field still compiles to direct field access), or the property call site
in dynamic code. `this.@name` is unchanged. These closures stay packed instead
of being declined. This is the same closure-context rule
`StaticTypesCallSiteWriter.makeGroovyObjectGetPropertySite` already applies to
packed bodies.
## Tests
`PackedClosuresTransformTest.packedClosureReadsAndWritesOwnerPropertiesThroughAccessors`,
`packedClosureReachesAccessorsOverriddenOrDeclaredElsewhere` and
`dynamicPackedClosureReadsOwnerPropertyThroughGetProperty` fail without the
change.
`AssignmentsStaticCompileTest.testPostfixAndPrefixOnPropertyCallGetterOnce2`
now passes with `-Pgroovy.target.closure.pack=true`.
`org.codehaus.groovy.classgen.asm.sc.*`, `groovy.transform.stc.*` and the
closure-packing tests pass, and with the packing flag the same set has no new
failures. The Checkstyle and CodeNarc gates pass.
> Packed closures read and write owner properties as fields, skipping getters
> and setters
> ---------------------------------------------------------------------------------------
>
> Key: GROOVY-12658
> URL: https://issues.apache.org/jira/browse/GROOVY-12658
> Project: Groovy
> Issue Type: Bug
> Affects Versions: 7.0.0-alpha-1, 6.0.0
> Reporter: Scott Murphy Heiberg
> Priority: Major
>
> A closure packed by closure packing (GEP-27, via {{@PackedClosures}} or
> {{-Dgroovy.target.closure.pack=true}}) has its body moved into a method of
> the owner class. There, a property of the owner compiles to direct field
> access, so the getter and setter are no longer called. The closure class it
> replaces calls them.
> {code:groovy}
> @groovy.transform.CompileStatic // or @groovy.transform.PackedClosures
> without @CompileStatic
> @groovy.transform.PackedClosures
> class Box {
> int gets, sets
> BigDecimal e = 1
> BigDecimal getE() { gets++; e }
> void setE(BigDecimal v) { sets++; e = v }
> def m() { def r = { -> e++ }(); [r, this.@e, gets, sets] }
> }
> println new Box().m()
> {code}
> *Actual:* {{[1, 2, 0, 0]}}
> *Expected:* {{[1, 2, 1, 1]}}, as without packing.
> The same happens for reads, assignments, compound assignments, prefix and
> postfix forms, and both {{e}} and {{this.e}}. It also affects a subclass
> override of a generated accessor, static properties, {{is}} getters, and
> (dynamic) an owner that overrides {{getProperty}}.
> {{AsmClassGenerator.visitPropertyExpression}} resolves {{this.name}} to the
> declared field in any method of the class; the hoisted body is such a method,
> so it bypasses the accessors.
> Verified on 6.0.0 and master.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
