From 58d505961761d95cfd7a02c325baa6d4949ac499 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. --- 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 177093469d121..5546866f60307 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -1450,17 +1450,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 4928eaed4c4c8..3f5ffd0ac7514 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); @@ -1455,6 +1460,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 3e3eb031ca389..5e2c8be746673 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -8297,16 +8297,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 ee5ba48c3d908..64428bf0fbd26 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 {