[
https://issues.apache.org/jira/browse/GROOVY-12440?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18125110#comment-18125110
]
ASF GitHub Bot commented on GROOVY-12440:
-----------------------------------------
codecov-commenter commented on PR #2959:
URL: https://github.com/apache/groovy/pull/2959#issuecomment-6063341459
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2959?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 67.2959%. Comparing base
([`3cc099b`](https://app.codecov.io/gh/apache/groovy/commit/3cc099b96da23c02a5786755853497fa05582a73?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`8bbf7f9`](https://app.codecov.io/gh/apache/groovy/commit/8bbf7f99256e3481718b94542303bde8b286fe82?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/groovy/pull/2959?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## GROOVY_5_1_X #2959 +/- ##
======================================================
- Coverage 67.2979% 67.2959% -0.0020%
+ Complexity 29652 29651 -1
======================================================
Files 1384 1384
Lines 117118 117120 +2
Branches 20602 20603 +1
======================================================
- Hits 78818 78817 -1
- Misses 31748 31751 +3
Partials 6552 6552
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2959?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...rg/codehaus/groovy/classgen/AsmClassGenerator.java](https://app.codecov.io/gh/apache/groovy/pull/2959?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2FAsmClassGenerator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL0FzbUNsYXNzR2VuZXJhdG9yLmphdmE=)
| `84.8641% <100.0000%> (+0.0223%)` | :arrow_up: |
... and [3 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/2959/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests,
report on failures, and find test suite problems.
- :package: [JS Bundle
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save
yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
> closure on the right-hand side of a property assignment inside a closure
> corrupts NestMembers causing IllegalAccessError
> -------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12440
> URL: https://issues.apache.org/jira/browse/GROOVY-12440
> Project: Groovy
> Issue Type: Bug
> Components: class generator, Static compilation
> Affects Versions: 5.0.0
> Reporter: James Daugherty
> Assignee: Paul King
> Priority: Major
>
> *Affects:* every 5.x release tested, 5.0.0 through 5.1.3.
> *Works in:* 4.0.33 (no NestMembers emitted) and 6.0.0.
> *Related:* GROOVY-10687, GROOVY-11780.
> h3. Description
> Under {{{}@CompileStatic{}}}, a closure on the right-hand side of a property
> assignment made inside another closure is recorded twice in the enclosing
> class's {{NestMembers}} attribute. Two examples are shown below. One entry
> names the real class; the other names a class that is never written. Every
> nested closure compiled after it in the same class is then listed under the
> wrong name, one higher than the class actually written.
> {code:groovy}
> { -> holder.value = list.collect { it } }
> holder.with { value = list.collect { it } }
> {code}
> Those closures name the outer class as their {{{}NestHost{}}}, but the outer
> class doesn't list them, so the JVM rejects their direct calls to its private
> methods:
> {noformat}
> java.lang.IllegalAccessError: class Repro$_callPrivate_closure2$_closure4
> tried to access private method
> 'int Repro.secret(java.lang.Integer)' ... (Type
> Repro$_callPrivate_closure2$_closure4 (loader: 'app') is not a
> nest member of type Repro (loader: 'app'): current type is not listed as a
> nest member)
> {noformat}
> The failing closure can be in any later method of the class, and the method
> containing the assignment never has to run. We hit this in a Grails 8
> application (Groovy 5.1.3): a statically compiled service failed at startup
> because an unrelated method assigned a property inside a {{with}} block, with
> a {{collect}} closure on the right-hand side.
> h3. Reproducer
> Standalone, no build tool:
> [jdaugherty/groovy5-issue-nested-closures|https://github.com/jdaugherty/groovy5-issue-nested-closures]
> {code:bash}
> ./reproduce.sh 5.1.3 # fails
> ./reproduce.sh 6.0.0 # passes
> {code}
> {code:groovy}
> import groovy.transform.CompileStatic
> @CompileStatic
> class Repro {
> static class Holder {
> String value
> }
> // Never called. Compiling it is enough to corrupt the NestMembers
> attribute of Repro.
> void assign(Holder holder) {
> Closure assignment = { -> holder.value = ['a'].collect { it
> }.join(',') }
> assignment.call()
> }
> List<Integer> callPrivate() {
> Closure<List<Integer>> outer = { -> [1, 2].collect { Integer i ->
> secret(i) } }
> outer.call()
> }
> private int secret(Integer i) {
> i * 10
> }
> static void main(String[] args) {
> println new Repro().callPrivate()
> }
> }
> {code}
> *Expected:* prints {{{}[10, 20]{}}}, and {{NestMembers}} lists exactly the
> closure classes written.
> *Actual (5.1.3):* the {{IllegalAccessError}} above. Classes written versus
> the {{NestMembers}} attribute:
> {noformat}
> Classes written NestMembers of Repro
> Repro$_assign_closure1 Repro$_assign_closure1
> Repro$_assign_closure1$_closure3 Repro$_assign_closure1$_closure3
> Repro$_assign_closure1$_closure4
> <- never written
> Repro$_callPrivate_closure2 Repro$_callPrivate_closure2
> Repro$_callPrivate_closure2$_closure4
> Repro$_callPrivate_closure2$_closure5 <- written as ...$_closure4
> {noformat}
> h3. Scope
> Tested on 5.1.3, with the same {{callPrivate()}} method following each
> variant:
> {code:groovy}
> // Fails: whether or not the assignment is the closure's last statement.
> // Each assignment adds one phantom entry; two assignments shift later names
> by two.
> { -> holder.value = list.collect { it } }
> holder.with { value = list.collect { it } }
> holder.with { it.value = list.collect { it } }
> // Passes
> { -> holder.setValue(list.collect { it }) } // explicit setter call
> { -> localVariable = list.collect { it } } // local variable
> { -> fieldOfThisClass = list.collect { it } } // field of the enclosing
> class
> holder.value = list.collect { it } // same assignment directly
> in the method
> { -> holder.value = 'x' } // no closure on the
> right-hand side
> {code}
> The failing source compiled without {{@CompileStatic}} passes and produces a
> correct {{NestMembers}} attribute. Reproduced on JDK 21.0.12 and 25.0.4.
> h3. Analysis
> {{AsmClassGenerator.moreNestmateEntries}} doesn't record the closure classes
> that are actually generated. It predicts the names of closures nested in
> closures by walking each closure's {{doCall}} body with a
> {{CodeVisitorSupport}} and incrementing a counter for each
> {{ClosureExpression}} it meets. The prediction holds only if the walk visits
> each {{ClosureExpression}} exactly once.
> For an assignment whose value is kept, static compilation rewrites the
> property assignment into the following setter form:
> {code:groovy}
> def tmp = val; a.setX(tmp); tmp
> {code}
> {{StaticPropertyAccessHelper.transformToSetterCall}} builds that as
> {{{}PoppingListOfExpressionsExpression(tmp, setX(tmp)){}}}, so the same
> {{TemporaryVariableExpression}} appears twice in the tree.
> {{TemporaryVariableExpression.visit}} evaluates the wrapped expression only
> once when the visitor is an {{{}AsmClassGenerator{}}}. For any other visitor
> it delegates straight to the wrapped expression. The nest-member walker
> therefore counts the right-hand-side closure twice, while code generation
> writes it once.
> GROOVY-11780 (backported to 5.0.x) doesn't cover this case.
> {{moreNestmateEntries}} and {{TemporaryVariableExpression}} look unchanged on
> master, so we haven't identified the change that makes 6.0.0 pass. Possible
> fixes:
> * Emit {{NestMembers}} from the closure classes actually generated rather
> than predicted names.
> * Have the walker skip {{ClosureExpression}} nodes it has already counted
> (identity-based).
> h3. Workaround
> Compute the value outside the closure, or call the setter explicitly:
> {code:groovy}
> String joined = ['a'].collect { it }.join(',')
> holder.with { value = joined }
> holder.with { setValue(['a'].collect { it }.join(',')) }
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
