[ 
https://issues.apache.org/jira/browse/GROOVY-12181?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18097618#comment-18097618
 ] 

ASF GitHub Bot commented on GROOVY-12181:
-----------------------------------------

codecov-commenter commented on PR #2725:
URL: https://github.com/apache/groovy/pull/2725#issuecomment-5025254483

   ## 
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2725?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 Report
   :x: Patch coverage is `87.05882%` with `22 lines` in your changes missing 
coverage. Please review.
   :white_check_mark: Project coverage is 69.2166%. Comparing base 
([`cb27031`](https://app.codecov.io/gh/apache/groovy/commit/cb27031d8e432f868d54b19ac290d0e61769c78e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
 to head 
([`305bc11`](https://app.codecov.io/gh/apache/groovy/commit/305bc11cc35353c9ef4f66793b9c540d5c483e3e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   | [Files with missing 
lines](https://app.codecov.io/gh/apache/groovy/pull/2725?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Patch % | Lines |
   |---|---|---|
   | 
[...rg/apache/groovy/runtime/async/AsyncExecutors.java](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fruntime%2Fasync%2FAsyncExecutors.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9ydW50aW1lL2FzeW5jL0FzeW5jRXhlY3V0b3JzLmphdmE=)
 | 69.4444% | [9 Missing and 2 partials :warning: 
](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 |
   | 
[.../apache/groovy/runtime/async/AwaitCombinators.java](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fruntime%2Fasync%2FAwaitCombinators.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9ydW50aW1lL2FzeW5jL0F3YWl0Q29tYmluYXRvcnMuamF2YQ==)
 | 93.3333% | [2 Missing and 3 partials :warning: 
](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 |
   | 
[...odehaus/groovy/transform/AsyncTransformHelper.java](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2FAsyncTransformHelper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3RyYW5zZm9ybS9Bc3luY1RyYW5zZm9ybUhlbHBlci5qYXZh)
 | 85.7143% | [3 Missing and 2 partials :warning: 
](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 |
   | 
[.../org/apache/groovy/runtime/async/AsyncSupport.java](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fruntime%2Fasync%2FAsyncSupport.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9ydW50aW1lL2FzeW5jL0FzeW5jU3VwcG9ydC5qYXZh)
 | 95.2381% | [0 Missing and 1 partial :warning: 
](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   [![Impacted file tree 
graph](https://app.codecov.io/gh/apache/groovy/pull/2725/graphs/tree.svg?width=650&height=150&src=pr&token=1r45138NfQ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   ```diff
   @@                Coverage Diff                 @@
   ##               master      #2725        +/-   ##
   ==================================================
   + Coverage     69.2109%   69.2166%   +0.0057%     
   - Complexity      34491      34514        +23     
   ==================================================
     Files            1539       1541         +2     
     Lines          129913     129924        +11     
     Branches        23669      23666         -3     
   ==================================================
   + Hits            89914      89929        +15     
   + Misses          31942      31935         -7     
   - Partials         8057       8060         +3     
   ```
   
   | [Files with missing 
lines](https://app.codecov.io/gh/apache/groovy/pull/2725?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Coverage Δ | |
   |---|---|---|
   | 
[src/main/java/groovy/concurrent/AwaitResult.java](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Fgroovy%2Fconcurrent%2FAwaitResult.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9ncm9vdnkvY29uY3VycmVudC9Bd2FpdFJlc3VsdC5qYXZh)
 | `96.6667% <100.0000%> (+3.3333%)` | :arrow_up: |
   | 
[src/main/java/groovy/concurrent/Awaitable.java](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Fgroovy%2Fconcurrent%2FAwaitable.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9ncm9vdnkvY29uY3VycmVudC9Bd2FpdGFibGUuamF2YQ==)
 | `92.8571% <ø> (ø)` | |
   | 
[...va/org/apache/groovy/parser/antlr4/AstBuilder.java](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fparser%2Fantlr4%2FAstBuilder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9wYXJzZXIvYW50bHI0L0FzdEJ1aWxkZXIuamF2YQ==)
 | `86.9490% <100.0000%> (+0.0575%)` | :arrow_up: |
   | 
[...org/apache/groovy/runtime/async/GroovyPromise.java](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fruntime%2Fasync%2FGroovyPromise.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9ydW50aW1lL2FzeW5jL0dyb292eVByb21pc2UuamF2YQ==)
 | `82.8571% <ø> (ø)` | |
   | 
[.../org/apache/groovy/runtime/async/AsyncSupport.java](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fruntime%2Fasync%2FAsyncSupport.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9ydW50aW1lL2FzeW5jL0FzeW5jU3VwcG9ydC5qYXZh)
 | `80.1282% <95.2381%> (+1.2437%)` | :arrow_up: |
   | 
[.../apache/groovy/runtime/async/AwaitCombinators.java](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fruntime%2Fasync%2FAwaitCombinators.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9ydW50aW1lL2FzeW5jL0F3YWl0Q29tYmluYXRvcnMuamF2YQ==)
 | `93.3333% <93.3333%> (ø)` | |
   | 
[...odehaus/groovy/transform/AsyncTransformHelper.java](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2FAsyncTransformHelper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3RyYW5zZm9ybS9Bc3luY1RyYW5zZm9ybUhlbHBlci5qYXZh)
 | `87.5000% <85.7143%> (+11.8902%)` | :arrow_up: |
   | 
[...rg/apache/groovy/runtime/async/AsyncExecutors.java](https://app.codecov.io/gh/apache/groovy/pull/2725?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fruntime%2Fasync%2FAsyncExecutors.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9ydW50aW1lL2FzeW5jL0FzeW5jRXhlY3V0b3JzLmphdmE=)
 | `69.4444% <69.4444%> (ø)` | |
   
   ... and [8 files with indirect coverage 
changes](https://app.codecov.io/gh/apache/groovy/pull/2725/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>




> Refactor async runtime and AST helpers introduced by GROOVY-9381
> ----------------------------------------------------------------
>
>                 Key: GROOVY-12181
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12181
>             Project: Groovy
>          Issue Type: Improvement
>            Reporter: Daniel Sun
>            Priority: Major
>
> h2. Context
> GROOVY-9381 landed native {{{}async{}}}/{{{}await{}}}/{{{}defer{}}}: public 
> API in {{{}groovy.concurrent{}}}, runtime in 
> {{{}org.apache.groovy.runtime.async{}}}, and parser desugaring via 
> {{AstBuilder}} + {{{}AsyncTransformHelper{}}}.
> The first implementation correctly prioritised a working feature, but two 
> types accumulated too many responsibilities:
>  * {{AsyncSupport}} — executor/scheduler setup, all {{await}} overloads, task 
> launch, combinators 
> ({{{}all{}}}/{{{}any{}}}/{{{}first{}}}/{{{}allSettled{}}}), defer scopes, and 
> generator bridges
>  * {{AstBuilder}} — multi-step rewrites for {{for await}} and {{async}} 
> closures inlined next to general parse-tree walking
> This ticket covers a focused refactor of that code: clearer module 
> boundaries, a thinner parser visitor, small API/codegen polish, and unit 
> tests for the extracted pieces. *No new language syntax.*
> h2. Goals
>  # Single responsibility for runtime pieces (entry point vs pool config vs 
> combinators).
>  # Keep feature-specific AST rewrites out of {{{}AstBuilder{}}}; helpers own 
> the desugaring.
>  # Preserve the public {{groovy.concurrent}} surface and combinator 
> {*}names{*}.
>  # Make compiler entry points and executor reset behaviour explicit and 
> tested.
> h2. Changes
> h3. Runtime split (package-private)
> ||Concern||Before||After||
> |Entry point|{{AsyncSupport}} (monolith)|{{AsyncSupport}} — facades used by 
> compiler-generated code and {{Awaitable}}|
> |Executors / scheduler|embedded in {{AsyncSupport}}|{{AsyncExecutors}}|
> |Combinators|embedded in {{AsyncSupport}}|{{AwaitCombinators}} ({{{}all{}}}, 
> {{{}any{}}}, {{{}first{}}}, {{{}allSettled{}}})|
> Public methods on {{AsyncSupport}} / {{Awaitable}} remain the stable surface; 
> algorithms and pool wiring move behind package-private types. Package-info 
> under {{groovy.concurrent}} and {{org.apache.groovy.runtime.async}} updated 
> to match.
> h3. AST / parser
> Higher-level rewrites move from {{AstBuilder}} into 
> {{{}AsyncTransformHelper{}}}:
>  * {{transformAsyncClosure}} — defer-scope wrap, generator param injection, 
> {{async}} / {{asyncGenerator}} call
>  * {{wrapForAwaitLoop}} — {{toIterable}} + {{{}try{}}}/{{{}finally{}}} 
> {{closeIterable}}
> {{AstBuilder}} only dispatches (one call site per construct).
> Related codegen polish in the same pass:
>  * Single-arg {{await}} emits {{AsyncSupport.awaitAny(Object)}} instead of 
> {{(Object) expr}} cast to force the {{await(Object)}} overload when the value 
> implements several async interfaces (e.g. {{CompletableFuture}} as both 
> {{CompletionStage}} and {{{}Future{}}}).
>  * {{for await}} temps use {{$_{_}forAwaitSource_N{_}}} _(monotonic counter) 
> instead of {{}}_{{_forAwaitSource}} + parse-context {{{}hashCode(){}}}, 
> avoiding synthetic-name clashes across nested/repeated loops.
> h3. Small API polish
>  * {{AsyncSupport.setExecutor(null)}} / {{Awaitable.setExecutor(null)}} — 
> restore the platform default (same as {{{}resetExecutor(){}}}). Previously 
> null was rejected.
>  * {{AwaitResult.success(T)}} — typed parameter (was {{Object}} + unchecked 
> cast).
>  * Javadoc for {{Awaitable.first}} all-fail path aligned with the existing 
> aggregate {{CompletionException}} behaviour (cause = first failure; remaining 
> as suppressed; {{await}} transparency rethrows the cause). *Exception shape 
> unchanged.*
> h2. Compatibility
>  * Follow-up to GROOVY-9381; intended for the 6.0 async feature line.
>  * {{setExecutor(null)}} is no longer an error; it resets the default 
> executor.
>  * Combinator *names* and public package layout ({{{}groovy.concurrent{}}}) 
> are unchanged.
>  * {{Awaitable.first}} all-fail path remains an aggregate 
> {{CompletionException}} (no change to exception type).



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to