[
https://issues.apache.org/jira/browse/GROOVY-9381?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18061914#comment-18061914
]
ASF GitHub Bot commented on GROOVY-9381:
----------------------------------------
Copilot commented on code in PR #2386:
URL: https://github.com/apache/groovy/pull/2386#discussion_r2868417254
##########
src/antlr/GroovyParser.g4:
##########
@@ -778,13 +780,17 @@ expression
// must come before postfixExpression to resolve the ambiguities between
casting and call on parentheses expression, e.g. (int)(1 / 2)
: castParExpression castOperandExpression
#castExprAlt
+ // async closure/lambda must come before postfixExpression to resolve the
ambiguities between async and method call, e.g. async { ... }
+ | ASYNC nls closureOrLambdaExpression
#asyncClosureExprAlt
+
// qualified names, array expressions, method invocation, post inc/dec
| postfixExpression
#postfixExprAlt
| switchExpression
#switchExprAlt
- // ~(BNOT)/!(LNOT) (level 1)
+ // ~(BNOT)/!(LNOT)/await (level 1)
| (BITNOT | NOT) nls expression
#unaryNotExprAlt
+ | AWAIT nls expression
#awaitExprAlt
Review Comment:
Because `identifier` includes `AWAIT` and `expression` tries
`postfixExpression` before `awaitExprAlt`, input like `await foo` is likely
parsed as a command-chain starting from an identifier named `await` (or
`await(foo)`), so `awaitExprAlt`/`visitAwaitExprAlt` won’t trigger. This breaks
the intended keyword-lowering to `AsyncSupport.await(...)` (especially outside
`@Async` contexts). Consider giving `awaitExprAlt` higher priority than
`postfixExprAlt` (similar to `asyncClosureExprAlt`) or adding a predicate-based
disambiguation so `await <expr>` reliably parses as the await operator.
##########
src/main/java/org/apache/groovy/runtime/async/AsyncStreamGenerator.java:
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.groovy.runtime.async;
+
+import groovy.concurrent.AsyncStream;
+import groovy.concurrent.Awaitable;
+
+import java.util.concurrent.SynchronousQueue;
+
+/**
+ * A producer/consumer implementation of {@link AsyncStream} used by
+ * {@code async} methods that contain {@code yield return} statements.
+ * <p>
+ * The producer (method body) runs on a separate thread and calls
+ * {@link #yield(Object)} for each emitted element. The consumer
+ * calls {@link #moveNext()}/{@link #getCurrent()} — typically via
+ * a {@code for await} loop.
+ * <p>
+ * Uses a {@link SynchronousQueue} to provide natural back-pressure:
+ * the producer thread blocks at each {@code yield return} until the
+ * consumer has consumed the previous element (mirroring C#'s async
+ * iterator suspension semantics).
+ * <p>
+ * This class is an internal implementation detail and should not be referenced
+ * directly by user code.
+ *
+ * @param <T> the element type
+ * @since 6.0.0
+ */
+public class AsyncStreamGenerator<T> implements AsyncStream<T> {
+
+ private static final Object DONE = new Object();
+
+ private final SynchronousQueue<Object> queue = new SynchronousQueue<>();
+ private T current;
+
+ /**
+ * Produces the next element. Called from the generator body when
+ * a {@code yield return expr} statement is executed. Blocks until
+ * the consumer is ready.
+ */
+ public void yield(Object value) {
+ try {
+ queue.put(new Item(value));
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new java.util.concurrent.CancellationException("Interrupted
during yield");
+ }
+ }
Review Comment:
`AsyncStreamGenerator` uses a `SynchronousQueue`, so the producer thread
blocks on every `yield(...)` until the consumer calls `moveNext()`. If the
consumer exits early (break/return/exception in `for await` body), the producer
can block forever, leaking a thread/virtual-thread and potentially exhausting
the fallback fixed thread pool on JDK < 21. Consider adding an explicit
cancellation/close signal (e.g., `AsyncStream` implements `AutoCloseable` with
a no-op default, generator overrides `close()` to unblock the producer) and
ensure the `for await` lowering closes the stream in a `finally` block.
##########
src/main/java/org/codehaus/groovy/transform/AsyncASTTransformation.java:
##########
@@ -0,0 +1,324 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.transform;
+
+import groovy.concurrent.AsyncStream;
+import groovy.concurrent.Awaitable;
+import groovy.transform.Async;
+import org.apache.groovy.runtime.async.AsyncSupport;
+import org.codehaus.groovy.ast.ASTNode;
+import org.codehaus.groovy.ast.AnnotatedNode;
+import org.codehaus.groovy.ast.AnnotationNode;
+import org.codehaus.groovy.ast.ClassCodeExpressionTransformer;
+import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
+import org.codehaus.groovy.ast.ClassHelper;
+import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.FieldNode;
+import org.codehaus.groovy.ast.GenericsType;
+import org.codehaus.groovy.ast.MethodNode;
+import org.codehaus.groovy.ast.Parameter;
+import org.codehaus.groovy.ast.VariableScope;
+import org.codehaus.groovy.ast.CodeVisitorSupport;
+import org.codehaus.groovy.ast.expr.ArgumentListExpression;
+import org.codehaus.groovy.ast.expr.ClassExpression;
+import org.codehaus.groovy.ast.expr.ClosureExpression;
+import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.MethodCallExpression;
+import org.codehaus.groovy.ast.expr.StaticMethodCallExpression;
+import org.codehaus.groovy.ast.expr.VariableExpression;
+import org.codehaus.groovy.ast.stmt.ExpressionStatement;
+import org.codehaus.groovy.ast.stmt.Statement;
+import org.codehaus.groovy.control.CompilePhase;
+import org.codehaus.groovy.control.SourceUnit;
+
+import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.block;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.returnS;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
+import static
org.codehaus.groovy.ast.tools.GenericsUtils.makeClassSafeWithGenerics;
+
+/**
+ * Handles code generation for the {@link Async @Async} annotation.
+ * <p>
+ * Transforms the annotated method so that:
+ * <ol>
+ * <li>{@code await(expr)} calls within the method body are redirected to
+ * {@link AsyncSupport#await(Object) AsyncSupport.await()}</li>
+ * <li>The method body is executed asynchronously via
+ * {@link AsyncSupport#executeAsync AsyncSupport.executeAsync}
+ * (or {@link AsyncSupport#executeAsyncVoid
AsyncSupport.executeAsyncVoid}
+ * for {@code void} methods)</li>
+ * <li>Generator methods (containing {@code yield return}) are transformed to
+ * use {@link AsyncSupport#generateAsyncStream
AsyncSupport.generateAsyncStream},
+ * returning an {@link AsyncStream}{@code <T>}</li>
+ * <li>The return type becomes {@link Awaitable}{@code <T>}
+ * (or {@link AsyncStream}{@code <T>} for generators)</li>
+ * </ol>
+ * <p>
+ * This transformation runs during the {@link CompilePhase#CANONICALIZATION}
+ * phase — before type resolution, which allows the modified return types to
+ * participate in normal type checking.
+ *
+ * @see Async
+ * @see AsyncSupport
+ * @since 6.0.0
+ */
+@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION)
+public class AsyncASTTransformation extends AbstractASTTransformation {
+
+ private static final Class<?> MY_CLASS = Async.class;
+ private static final ClassNode MY_TYPE = ClassHelper.make(MY_CLASS);
+ private static final String MY_TYPE_NAME = "@" +
MY_TYPE.getNameWithoutPackage();
+ private static final ClassNode AWAITABLE_TYPE =
ClassHelper.make(Awaitable.class);
+ private static final ClassNode ASYNC_STREAM_TYPE =
ClassHelper.make(AsyncStream.class);
+ private static final ClassNode ASYNC_UTILS_TYPE =
ClassHelper.make(AsyncSupport.class);
+
+ @Override
+ public void visit(ASTNode[] nodes, SourceUnit source) {
+ init(nodes, source);
+ AnnotatedNode parent = (AnnotatedNode) nodes[1];
+ AnnotationNode anno = (AnnotationNode) nodes[0];
+ if (!MY_TYPE.equals(anno.getClassNode())) return;
+
+ if (!(parent instanceof MethodNode mNode)) return;
+
+ // Validate
+ if (mNode.isAbstract()) {
+ addError(MY_TYPE_NAME + " cannot be applied to abstract method '"
+ mNode.getName() + "'", mNode);
+ return;
+ }
+ if ("<init>".equals(mNode.getName()) ||
"<clinit>".equals(mNode.getName())) {
+ addError(MY_TYPE_NAME + " cannot be applied to constructors",
mNode);
+ return;
+ }
+ ClassNode originalReturnType = mNode.getReturnType();
+ if (AWAITABLE_TYPE.getName().equals(originalReturnType.getName())
+ ||
ASYNC_STREAM_TYPE.getName().equals(originalReturnType.getName())
+ ||
"java.util.concurrent.CompletableFuture".equals(originalReturnType.getName())) {
+ addError(MY_TYPE_NAME + " cannot be applied to a method that
already returns an async type", mNode);
+ return;
+ }
+ Statement originalBody = mNode.getCode();
+ if (originalBody == null) return;
+
+ ClassNode cNode = mNode.getDeclaringClass();
+
+ // Resolve executor expression
+ String executorFieldName = getMemberStringValue(anno, "executor");
+ Expression executorExpr;
+ if (executorFieldName != null && !executorFieldName.isEmpty()) {
+ FieldNode field = cNode.getDeclaredField(executorFieldName);
+ if (field == null) {
+ addError(MY_TYPE_NAME + ": executor field '" +
executorFieldName
+ + "' not found in class " + cNode.getName(), mNode);
+ return;
+ }
+ if (mNode.isStatic() && !field.isStatic()) {
+ addError(MY_TYPE_NAME + ": executor field '" +
executorFieldName
+ + "' must be static for static method '" +
mNode.getName() + "'", mNode);
+ return;
+ }
+ executorExpr = varX(executorFieldName);
Review Comment:
When resolving the custom executor field, `varX(executorFieldName)` creates
an unbound `VariableExpression` and can be accidentally captured by a
same-named local variable/parameter, resulting in using the wrong executor at
runtime. Since you already have the `FieldNode`, bind the expression to that
field explicitly (e.g., create a `VariableExpression` from the `FieldNode` /
set accessed variable) to ensure it always references the intended field.
```suggestion
VariableExpression executorVar = varX(executorFieldName);
executorVar.setAccessedVariable(field);
executorExpr = executorVar;
```
##########
src/test/groovy/org/codehaus/groovy/transform/AsyncVirtualThreadTest.groovy:
##########
@@ -0,0 +1,1115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.transform
+
+import org.junit.jupiter.api.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+/**
+ * Tests for virtual thread integration, executor configuration, exception
+ * handling consistency across async methods/closures/lambdas, and coverage
+ * improvements for the async/await feature.
+ *
+ * @since 6.0.0
+ */
+final class AsyncVirtualThreadTest {
+
+ // --
> Support async/await like ES7
> ----------------------------
>
> Key: GROOVY-9381
> URL: https://issues.apache.org/jira/browse/GROOVY-9381
> Project: Groovy
> Issue Type: New Feature
> Reporter: Daniel Sun
> Priority: Major
>
> Here is an example to show proposed syntax and backend API(Java's
> {{CompletableFuture}} or GPars's {{{}Promise{}}}), but I think it's better
> for Groovy to have its own {{Promise}} to decouple with Java API because
> async/await as a language feature should be as stable as possible.
> {{async}} will generate the {{Awaitable}} instance such as Groovy {{Promise}}
> implementing the {{Awaitable}} interface, and {{await}} can wait for any
> {{Awaitable}} instance to complete and unwrap it for the result.
> {code:java}
> /**
> * 1. An async function that simulates a network API call.
> * The 'async' keyword implies it runs asynchronously without blocking.
> */
> async fetchUserData(userId) {
> println "Starting to fetch data for user ${userId}..."
>
> // Simulate a 1-second network delay.
> Thread.sleep(1000)
>
> println "Fetch successful!"
> // The 'async' function implicitly returns a "CompletableFuture" or
> "Promise" containing this value.
> return [userId: userId, name: 'Daniel']
> }
> /**
> * 2. An async function that uses 'await' to consume the result.
> */
> async processUserData() {
> println "Process started, preparing to fetch user data..."
>
> try {
> // 'await' pauses this function until fetchUserData completes
> // and returns the final result directly.
> def user = await fetchUserData(1)
>
> println "Data received: ${user}"
> return "Processing complete for ${user.name}."
>
> } catch (Exception e) {
> return "An error occurred: ${e.message}"
> }
> }
> // --- Execution ---
> println "Script starting..."
> // Kick off the entire asynchronous process.
> def future = processUserData()
> // This line executes immediately, proving the process is non-blocking.
> println "Script continues to run while user data is being fetched in the
> background..."
> def result = future.get()
> println "Script finished: ${result}"
> {code}
> Use async/await with closure or lambda expression:
> {code}
> // use closure
> def c = async {
> println "Process started, preparing to fetch user data..."
>
> try {
> // 'await' pauses this function until fetchUserData completes
> // and returns the final result directly.
> def user = await fetchUserData(1)
>
> println "Data received: ${user}"
> return "Processing complete for ${user.name}."
>
> } catch (Exception e) {
> return "An error occurred: ${e.message}"
> }
> }
> def future = c()
> {code}
> {code}
> // use lambda expression
> def c = async () -> {
> println "Process started, preparing to fetch user data..."
>
> try {
> // 'await' pauses this function until fetchUserData completes
> // and returns the final result directly.
> def user = await fetchUserData(1)
>
> println "Data received: ${user}"
> return "Processing complete for ${user.name}."
>
> } catch (Exception e) {
> return "An error occurred: ${e.message}"
> }
> }
> def future = c()
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
