[ 
https://issues.apache.org/jira/browse/GROOVY-12181?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Daniel Sun updated GROOVY-12181:
--------------------------------
    Description: 
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).

> 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