Merge pull request swiftlang#83094 from swiftlang/jepa-main

Sema: Make `@concurrent` not imply `async` on closures

Author: AnthonyLatsis

lib/Sema/ConstraintSystem.cpp

@@ -1451,17 +1451,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<ConcurrentAttr>()) {
-    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();
 }

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<ClosureExpr>(E)) {
+        checkClosure(closure);
+      }
+
       // Specially diagnose some checked casts that are illegal.
       if (auto cast = dyn_cast<CheckedCastExpr>(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<ConcurrentAttr>();
+      if (!attr) {
+        return;
+      }
+
+      if (closure->getAsyncLoc().isValid()) {
+        return;
+      }
+
+      if (closure->getType()->castTo<AnyFunctionType>()->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);

lib/Sema/TypeCheckAttr.cpp

@@ -8296,16 +8296,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(

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}}
   }
 }
 

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>(_: 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 {