From dc1458762e9fd5b3f33b588877773bc48881d5e7 Mon Sep 17 00:00:00 2001 From: Anthony Latsis Date: Wed, 16 Jul 2025 02:57:37 +0100 Subject: [PATCH] Sema: Make `@concurrent` not imply `async` on closures The present approach is not prudent because `@concurrent` synchronous functions, a natural extension, are a likely-to-happen future direction, whereas the current inference rule is entirely grounded on `@concurrent` being exclusive to async functions. If we were to ship this rule, we would have to keep the promise for backwards compatibility when implementing the aforementioned future direction, replacing one inconsistency with another, and possibly introducing new bug-prone expression checking code. ```swift func foo(_: () -> Void) {} func foo(_: () async -> Void) {} // In a future without this change and `@concurrent` synchronous // functions accepted, the first call resolves to the first overload, // and the second call resolves to the second, despite `@concurrent` no // longer implying `async`. foo { } foo { @concurrent in } ``` This change also drops the fix-it for removing `@concurrent` when used on a synchronous closure. With the inference rule gone, and the diagnosis delayed until after solution application, this error raises a fairly balanced choice between removing the attribute and being explicit about the effect, where a unilateral suggestion is quite possibly more harmful than useful. (cherry picked from commit 58d505961761d95cfd7a02c325baa6d4949ac499) --- lib/Sema/ConstraintSystem.cpp | 8 +-- lib/Sema/MiscDiagnostics.cpp | 32 +++++++++ lib/Sema/TypeCheckAttr.cpp | 10 --- .../attr_execution/conversions.swift | 2 +- test/attr/execution_behavior_attrs.swift | 67 +++++++++++++++++-- 5 files changed, 96 insertions(+), 23 deletions(-) diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index e3c83ec12ce3a..eafffc9b565eb 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -1438,17 +1438,11 @@ FunctionType::ExtInfo ClosureEffectsRequest::evaluate( if (!body) return ASTExtInfoBuilder().withSendable(sendable).build(); - // `@concurrent` attribute is only valid on asynchronous function types. - bool asyncFromAttr = false; - if (expr->getAttrs().hasAttribute()) { - asyncFromAttr = true; - } - auto throwFinder = FindInnerThrows(expr); body->walk(throwFinder); return ASTExtInfoBuilder() .withThrows(throwFinder.foundThrow(), /*FIXME:*/Type()) - .withAsync(asyncFromAttr || bool(findAsyncNode(expr))) + .withAsync(bool(findAsyncNode(expr))) .withSendable(sendable) .build(); } diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp index d6cd9f4a88f63..d61f1a2412099 100644 --- a/lib/Sema/MiscDiagnostics.cpp +++ b/lib/Sema/MiscDiagnostics.cpp @@ -305,6 +305,11 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC, } } + // Performs checks on a closure. + if (auto *closure = dyn_cast(E)) { + checkClosure(closure); + } + // Specially diagnose some checked casts that are illegal. if (auto cast = dyn_cast(E)) { checkCheckedCastExpr(cast); @@ -1459,6 +1464,33 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC, } } } + + void checkClosure(ClosureExpr *closure) { + if (closure->isImplicit()) { + return; + } + + auto attr = closure->getAttrs().getAttribute(); + if (!attr) { + return; + } + + if (closure->getAsyncLoc().isValid()) { + return; + } + + if (closure->getType()->castTo()->isAsync()) { + return; + } + + // `@concurrent` is not allowed on synchronous closures, but this is + // likely to change, so diagnose this here, post solution application, + // instead of introducing an "implies `async`" inference rule that won't + // make sense in the nearish future. + Ctx.Diags.diagnose(attr->getLocation(), + diag::execution_behavior_only_on_async_closure, attr); + attr->setInvalid(); + } }; DiagnoseWalker Walker(DC, isExprStmt); diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index eab64da3e5ea1..d7b19ad31a524 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -8243,16 +8243,6 @@ class ClosureAttributeChecker } void checkExecutionBehaviorAttribute(DeclAttribute *attr) { - // execution behavior attribute implies `async`. - if (closure->hasExplicitResultType() && - closure->getAsyncLoc().isInvalid()) { - ctx.Diags - .diagnose(attr->getLocation(), - diag::execution_behavior_only_on_async_closure, attr) - .fixItRemove(attr->getRangeWithAt()); - attr->setInvalid(); - } - if (auto actorType = getExplicitGlobalActor(closure)) { ctx.Diags .diagnose( diff --git a/test/Concurrency/attr_execution/conversions.swift b/test/Concurrency/attr_execution/conversions.swift index 6462cd83a2490..01db388c9d94a 100644 --- a/test/Concurrency/attr_execution/conversions.swift +++ b/test/Concurrency/attr_execution/conversions.swift @@ -93,7 +93,7 @@ do { do { let _: () -> Void = { @concurrent in - // expected-error@-1 {{invalid conversion from 'async' function of type '() async -> Void' to synchronous function type '() -> Void'}} + // expected-error@-1 {{cannot use @concurrent on non-async closure}}{{none}} } } diff --git a/test/attr/execution_behavior_attrs.swift b/test/attr/execution_behavior_attrs.swift index c9babb24a565c..fd0db27bf9986 100644 --- a/test/attr/execution_behavior_attrs.swift +++ b/test/attr/execution_behavior_attrs.swift @@ -125,17 +125,74 @@ struct IsolatedType { @concurrent func test() async {} } -_ = { @concurrent in // Ok +// `@concurrent` on closures does not contribute to `async` inference. +do { + _ = { @concurrent in } + // expected-error@-1:9 {{cannot use @concurrent on non-async closure}}{{none}} + _ = { @concurrent () -> Int in } + // expected-error@-1:9 {{cannot use @concurrent on non-async closure}}{{none}} + // Explicit effect precludes effects inference from the body. + _ = { @concurrent () throws in await awaitMe() } + // expected-error@-1:9 {{cannot use @concurrent on non-async closure}}{{none}} + // expected-error@-2:40 {{'async' call in a function that does not support concurrency}}{{none}} + _ = { @concurrent () async in } + _ = { @concurrent in await awaitMe() } + + func awaitMe() async {} + + do { + func foo(_: () -> Void) {} + func foo(_: () async -> Void) async {} + + func sync() { + foo { @concurrent in } + // expected-error@-1:13 {{cannot use @concurrent on non-async closure}}{{none}} + } + // expected-note@+1:10 {{add 'async' to function 'sync2()' to make it asynchronous}}{{17-17= async}} + func sync2() { + foo { @concurrent in await awaitMe() } + // expected-error@-1:7 {{'async' call in a function that does not support concurrency}}{{none}} + } + func async() async { + // Even though the context is async, the sync overload is favored because + // the closure is sync, so the error is expected. + foo { @concurrent in } + // expected-error@-1:13 {{cannot use @concurrent on non-async closure}}{{none}} + + foo { @concurrent in await awaitMe() } + // expected-error@-1:7 {{expression is 'async' but is not marked with 'await'}}{{7-7=await }} + // expected-note@-2:7 {{call is 'async'}}{{none}} + } + } + + do { + func foo(_: (Int) async -> Void) {} + func foo(_: (Int) -> Void) async {} + + func sync() { + // OK, sync overload picked. + foo { @concurrent _ in } + } + func async() async { + foo { @concurrent _ in } + // expected-error@-1:13 {{cannot use @concurrent on non-async closure}}{{none}} + // expected-error@-2:7 {{expression is 'async' but is not marked with 'await'}}{{7-7=await }} + // expected-note@-3:7 {{call is 'async'}}{{none}} + } + } + + do { + func foo(_: T) {} + + foo({@concurrent in }) + // expected-error@-1:10 {{cannot use @concurrent on non-async closure}}{{none}} + } } _ = { @MainActor @concurrent in // expected-error@-1 {{cannot use @concurrent because function type is isolated to a global actor 'MainActor'}} } -_ = { @concurrent () -> Int in - // expected-error@-1 {{@concurrent on non-async closure}} -} - // Make sure that explicit use of `@concurrent` doesn't interfere with inference of `throws` from the body. do { func acceptsThrowing(_ body: () async throws -> Void) async {