[
https://issues.apache.org/jira/browse/GROOVY-11792?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18098592#comment-18098592
]
ASF GitHub Bot commented on GROOVY-11792:
-----------------------------------------
codecov-commenter commented on PR #2732:
URL: https://github.com/apache/groovy/pull/2732#issuecomment-5063049846
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2732?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 69.3139%. Comparing base
([`3240fcf`](https://app.codecov.io/gh/apache/groovy/commit/3240fcf05ad12df667e06e673bb3ff6822961029?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`2e90e7f`](https://app.codecov.io/gh/apache/groovy/commit/2e90e7f66daf81cb5ab8b637d4d7dc96abb7b199?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/2732?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #2732 +/- ##
==================================================
+ Coverage 69.2732% 69.3139% +0.0407%
- Complexity 34769 34791 +22
==================================================
Files 1542 1542
Lines 130456 130476 +20
Branches 23786 23787 +1
==================================================
+ Hits 90371 90438 +67
+ Misses 31962 31906 -56
- Partials 8123 8132 +9
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2732?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...org/codehaus/groovy/classgen/asm/CompileStack.java](https://app.codecov.io/gh/apache/groovy/pull/2732?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FCompileStack.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9Db21waWxlU3RhY2suamF2YQ==)
| `86.5707% <100.0000%> (+0.2627%)` | :arrow_up: |
|
[...haus/groovy/classgen/asm/DelegatingController.java](https://app.codecov.io/gh/apache/groovy/pull/2732?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FDelegatingController.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9EZWxlZ2F0aW5nQ29udHJvbGxlci5qYXZh)
| `92.0635% <100.0000%> (+0.1280%)` | :arrow_up: |
|
[.../codehaus/groovy/classgen/asm/StatementWriter.java](https://app.codecov.io/gh/apache/groovy/pull/2732?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FStatementWriter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9TdGF0ZW1lbnRXcml0ZXIuamF2YQ==)
| `99.2537% <100.0000%> (+2.2840%)` | :arrow_up: |
|
[...codehaus/groovy/classgen/asm/WriterController.java](https://app.codecov.io/gh/apache/groovy/pull/2732?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FWriterController.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9Xcml0ZXJDb250cm9sbGVyLmphdmE=)
| `88.4354% <100.0000%> (+2.3243%)` | :arrow_up: |
|
[...vy/classgen/asm/sc/StaticTypesStatementWriter.java](https://app.codecov.io/gh/apache/groovy/pull/2732?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2Fsc%2FStaticTypesStatementWriter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9zYy9TdGF0aWNUeXBlc1N0YXRlbWVudFdyaXRlci5qYXZh)
| `98.4252% <100.0000%> (ø)` | |
|
[...codehaus/groovy/control/CompilerConfiguration.java](https://app.codecov.io/gh/apache/groovy/pull/2732?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fcontrol%2FCompilerConfiguration.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NvbnRyb2wvQ29tcGlsZXJDb25maWd1cmF0aW9uLmphdmE=)
| `72.8188% <100.0000%> (+0.1837%)` | :arrow_up: |
... and [11 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/2732/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>
> for-in loop variable captured by closure/AIC sees final value, not
> per-iteration value
> --------------------------------------------------------------------------------------
>
> Key: GROOVY-11792
> URL: https://issues.apache.org/jira/browse/GROOVY-11792
> Project: Groovy
> Issue Type: Bug
> Reporter: Daniel Sun
> Priority: Major
>
> h2. Problem
> When a *for-in* (enhanced for-each) loop variable is shared with a deferred
> closure or anonymous inner class (AIC), all captures observe the *final* loop
> value after the loop finishes, instead of the value from the iteration that
> created the capture.
> Classic {{for}} / {{while}} loops are a separate construct and are not the
> subject of this report.
> This is the same class of surprise as Java’s historical “effectively final
> loop variable” / deferred-lambda story, but Groovy’s shared variables use a
> single {{groovy.lang.Reference}} updated in place across for-in iterations,
> so deferred use always sees the last assignment.
> h2. Expected behaviour
> Each deferred capture should observe the for-in value (and index, when
> present) from the {*}iteration in which it was created{*}.
> h2. Actual behaviour
> Every deferred capture sees the *last* iteration’s value.
> h2. Reproducer
> {code:groovy}
> import java.util.function.Supplier
> def numbers = [1, 2, 3]
> List suppliers = []
> for (n in numbers) {
> Supplier s = { n * n }
> suppliers << s
> }
> // Expected: [1, 4, 9]
> // Actual: [9, 9, 9]
> assert suppliers.collect { it.get() } == [1, 4, 9]
> {code}
> Colon syntax ({{{}for (Integer n : numbers){}}}), {{{}@CompileStatic{}}},
> indexed for-in ({{{}for (i, v in …){}}}), array/enumeration SC paths, and AIC
> capture (see GROOVY-11818) show the same pattern.
> h2. Workaround
> Introduce a fresh local per iteration so the closure captures a non-shared
> (or newly shared) binding:
> {code:groovy}
> for (n in numbers) {
> def local = n
> suppliers << { local * local }
> }
> {code}
> h2. Root cause (classgen)
> For-in loop heads store into a *single* shared {{Reference}} for
> closure-shared loop variables ({{{}OperandStack.storeVar{}}} →
> {{{}Reference#set{}}}). Closures/AICs created in the body capture that one
> holder; later iterations overwrite it.
> Relevant area: {{org.codehaus.groovy.classgen.asm}} ({{{}StatementWriter{}}},
> {{{}StaticTypesStatementWriter{}}}, {{{}CompileStack{}}},
> {{{}WriterController{}}}).
> h2. Resolution approach
> * When per-iteration capture is enabled (default), each for-in iteration
> that stores a *holder* loop variable allocates a *fresh*
> {{groovy.lang.Reference}} (value and shared index).
> * Within the *same* iteration, assignment to the loop variable remains
> visible to captures created there (still one {{Reference}} per iteration).
> * Non-shared loop variables and non-loop shared variables are unchanged.
> * Dynamic and static compilation for-in paths (iterator, SC array, SC
> enumeration) share the same store/increment helpers.
> h3. Language-compatibility opt-out
> This changes observable capture semantics (default-on). Historical “single
> shared {{Reference}} / final value” behaviour can be restored with either:
> * system property: {{groovy.for.loop.capture=false}}
> * {{{}CompilerConfiguration{}}}: put {{Boolean.FALSE}} for key
> {{CompilerConfiguration.FOR_LOOP_CAPTURE}} ({{{}"forLoopCapture"{}}}) in the
> optimization-options map
> Notes:
> * This is a *language-compatibility* switch, not a performance optimization.
> * Setting optimization option {{"all"}} to {{false}} does *not* disable
> for-in recapture.
> h2. Related issues
> * GROOVY-11818 — for-in variable captured by anonymous inner class (same
> root cause)
> * GROOVY-11751 — indexed for-in with shared index (holder index
> store/increment)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
