Merge pull request #37263 from hborla/async-closure-diagnostics

[Concurrency] Improve diagnostics for missing `await`

Author: hborla

lib/Sema/CSApply.cpp

@@ -25,6 +25,7 @@
 #include "swift/AST/ASTVisitor.h"
 #include "swift/AST/ASTWalker.h"
 #include "swift/AST/ClangModuleLoader.h"
+#include "swift/AST/Effects.h"
 #include "swift/AST/ExistentialLayout.h"
 #include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/GenericSignature.h"
@@ -7043,6 +7044,38 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
       }
     }
 
+    /// Whether the given effect should be propagated to a closure expression.
+    auto shouldApplyEffect = [&](EffectKind kind) -> bool {
+      auto last = locator.last();
+      if (!(last && last->is<LocatorPathElt::ApplyArgToParam>()))
+        return true;
+
+      // The effect should not be applied if the closure is an argument
+      // to a function where that effect is polymorphic.
+      if (auto *call = getAsExpr<ApplyExpr>(locator.getAnchor())) {
+        if (auto *declRef = dyn_cast<DeclRefExpr>(call->getFn())) {
+          if (auto *fn = dyn_cast<AbstractFunctionDecl>(declRef->getDecl()))
+            return !fn->hasPolymorphicEffect(kind);
+        }
+      }
+
+      return true;
+    };
+
+    // If we have a ClosureExpr, we can safely propagate 'async' to the closure.
+    fromEI = fromFunc->getExtInfo();
+    if (toEI.isAsync() && !fromEI.isAsync() && shouldApplyEffect(EffectKind::Async)) {
+      auto newFromFuncType = fromFunc->withExtInfo(fromEI.withAsync());
+      if (applyTypeToClosureExpr(cs, expr, newFromFuncType)) {
+        fromFunc = newFromFuncType->castTo<FunctionType>();
+
+        // Propagating 'async' might have satisfied the entire conversion.
+        // If so, we're done, otherwise keep converting.
+        if (fromFunc->isEqual(toType))
+          return expr;
+      }
+    }
+
     // If we have a ClosureExpr, then we can safely propagate the 'no escape'
     // bit to the closure without invalidating prior analysis.
     fromEI = fromFunc->getExtInfo();

lib/Sema/TypeCheckConcurrency.cpp

@@ -1727,7 +1727,8 @@ namespace {
       }
 
       // Mark as implicitly async.
-      apply->setImplicitlyAsync(true);
+      if (!fnType->getExtInfo().isAsync())
+        apply->setImplicitlyAsync(true);
 
       // If we don't need to check for sendability, we're done.
       if (!shouldDiagnoseNonSendableViolations(ctx.LangOpts))

lib/Sema/TypeCheckEffects.cpp

@@ -2872,8 +2872,18 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
            }
           continue;
          }
-         Ctx.Diags.diagnose(diag.expr.getStartLoc(),
-                            diag::async_access_without_await, 0);
+
+         auto *call = dyn_cast<ApplyExpr>(&diag.expr);
+         if (call && call->implicitlyAsync()) {
+           // Emit a tailored note if the call is implicitly async, meaning the
+           // callee is isolated to an actor.
+           auto callee = call->getCalledValue();
+           Ctx.Diags.diagnose(diag.expr.getStartLoc(), diag::actor_isolated_sync_func,
+                              callee->getDescriptiveKind(), callee->getName());
+         } else {
+           Ctx.Diags.diagnose(diag.expr.getStartLoc(),
+                              diag::async_access_without_await, 0);
+         }
 
          continue;
         }

test/Concurrency/actor_call_implicitly_async.swift

@@ -207,26 +207,28 @@ extension BankAccount {
   func totalBalance(including other: BankAccount) async -> Int {
     //expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{12-12=await }}
     return balance()
-          + other.balance()  // expected-note{{call is 'async'}}
+          + other.balance()  // expected-note{{calls to instance method 'balance()' from outside of its actor context are implicitly asynchronous}}
   }
 
   func breakAccounts(other: BankAccount) async {
     // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{9-9=await }}
-    _ = other.deposit( // expected-note{{call is 'async'}}
-          other.withdraw( // expected-note{{call is 'async'}}
+    _ = other.deposit( // expected-note{{calls to instance method 'deposit' from outside of its actor context are implicitly asynchronous}}
+          other.withdraw( // expected-note{{calls to instance method 'withdraw' from outside of its actor context are implicitly asynchronous}}
             self.deposit(
-              other.withdraw( // expected-note{{call is 'async'}}
-                other.balance())))) // expected-note{{call is 'async'}}
+              other.withdraw( // expected-note{{calls to instance method 'withdraw' from outside of its actor context are implicitly asynchronous}}
+                other.balance())))) // expected-note{{calls to instance method 'balance()' from outside of its actor context are implicitly asynchronous}}
   }
 }
 
 func anotherAsyncFunc() async {
   let a = BankAccount(initialDeposit: 34)
   let b = BankAccount(initialDeposit: 35)
 
-  // expected-error@+1{{expression is 'async' but is not marked with 'await'}} {{7-7=await }} expected-note@+1{{call is 'async'}}
+  // expected-error@+2{{expression is 'async' but is not marked with 'await'}} {{7-7=await }}
+  // expected-note@+1{{calls to instance method 'deposit' from outside of its actor context are implicitly asynchronous}}
   _ = a.deposit(1)
-  // expected-error@+1{{expression is 'async' but is not marked with 'await'}} {{7-7=await }} expected-note@+1{{call is 'async'}}
+  // expected-error@+2{{expression is 'async' but is not marked with 'await'}} {{7-7=await }}
+  // expected-note@+1{{calls to instance method 'balance()' from outside of its actor context are implicitly asynchronous}}
   _ = b.balance()
 
   _ = b.balance // expected-error {{actor-isolated instance method 'balance()' can only be referenced from inside the actor}}

test/Concurrency/actor_isolation.swift

@@ -71,7 +71,7 @@ actor MyActor: MySuperActor { // expected-error{{actor types do not support inhe
   class func synchronousClass() { }
   static func synchronousStatic() { }
 
-  func synchronous() -> String { text.first ?? "nothing" } // expected-note 20{{calls to instance method 'synchronous()' from outside of its actor context are implicitly asynchronous}}
+  func synchronous() -> String { text.first ?? "nothing" } // expected-note 19{{calls to instance method 'synchronous()' from outside of its actor context are implicitly asynchronous}}
   func asynchronous() async -> String {
     super.superState += 4
     return synchronous()
@@ -202,7 +202,7 @@ extension MyActor {
 
     _ = otherActor.synchronous()
     // expected-error@-1{{expression is 'async' but is not marked with 'await'}}{{9-9=await }}
-    // expected-note@-2{{call is 'async'}}
+    // expected-note@-2{{calls to instance method 'synchronous()' from outside of its actor context are implicitly asynchronous}}
     _ = await otherActor.asynchronous()
     _ = otherActor.text[0]
     // expected-error@-1{{expression is 'async' but is not marked with 'await'}}{{9-9=await }}
@@ -222,7 +222,7 @@ extension MyActor {
 
     // Global actors
     syncGlobalActorFunc() // expected-error{{expression is 'async' but is not marked with 'await'}}{{5-5=await }}
-    // expected-note@-1{{call is 'async'}}
+    // expected-note@-1{{calls to global function 'syncGlobalActorFunc()' from outside of its actor context are implicitly asynchronous}}
 
     await asyncGlobalActorFunc()
 
@@ -399,7 +399,7 @@ actor Crystal {
 @SomeGlobalActor func goo1() async {
   let _ = goo2
   // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{3-3=await }}
-  goo2() // expected-note{{call is 'async'}}
+  goo2() // expected-note{{calls to global function 'goo2()' from outside of its actor context are implicitly asynchronous}}
 }
 @available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *)
 @asyncHandler @SomeOtherGlobalActor func goo2() { await goo1() }
@@ -409,7 +409,7 @@ func testGlobalActorClosures() {
   let _: Int = acceptAsyncClosure { @SomeGlobalActor in
     syncGlobalActorFunc()
     syncOtherGlobalActorFunc() // expected-error{{expression is 'async' but is not marked with 'await'}}{{5-5=await }}
-    // expected-note@-1{{call is 'async'}}
+    // expected-note@-1{{calls to global function 'syncOtherGlobalActorFunc()' from outside of its actor context are implicitly asynchronous}}
 
     await syncOtherGlobalActorFunc()
     return 17
@@ -434,7 +434,7 @@ extension MyActor {
     // expected-note@-1{{property access is 'async'}}
     _ = await mutable
     _ = synchronous() // expected-error{{expression is 'async' but is not marked with 'await'}}{{9-9=await }}
-    // expected-note@-1{{call is 'async'}}
+    // expected-note@-1{{calls to instance method 'synchronous()' from outside of its actor context are implicitly asynchronous}}
     _ = await synchronous()
     _ = text[0] // expected-error{{expression is 'async' but is not marked with 'await'}}
     // expected-note@-1{{property access is 'async'}}
@@ -445,7 +445,7 @@ extension MyActor {
     // we are outside of the actor instance.
     _ = self.immutable
     _ = self.synchronous() // expected-error{{expression is 'async' but is not marked with 'await'}}{{9-9=await }}
-    // expected-note@-1{{call is 'async'}}
+    // expected-note@-1{{calls to instance method 'synchronous()' from outside of its actor context are implicitly asynchronous}}
     _ = await self.synchronous()
 
     _ = await self.asynchronous()
@@ -461,7 +461,7 @@ extension MyActor {
     // expected-note@-1{{property access is 'async'}}
     _ = await super.superState
     super.superMethod() // expected-error{{expression is 'async' but is not marked with 'await'}}{{5-5=await }}
-  // expected-note@-1{{call is 'async'}}
+  // expected-note@-1{{calls to instance method 'superMethod()' from outside of its actor context are implicitly asynchronous}}
 
     await super.superMethod()
     await super.superAsyncMethod()
@@ -473,7 +473,7 @@ extension MyActor {
     // call asychronous methods
     _ = otherActor.immutable // okay
     _ = otherActor.synchronous() // expected-error{{expression is 'async' but is not marked with 'await'}}{{9-9=await }}
-    // expected-note@-1{{call is 'async'}}
+    // expected-note@-1{{calls to instance method 'synchronous()' from outside of its actor context are implicitly asynchronous}}
     _ = otherActor.synchronous  // expected-error{{actor-isolated instance method 'synchronous()' can only be referenced on 'self'}}
     _ = await otherActor.asynchronous()
     _ = otherActor.text[0] // expected-error{{expression is 'async' but is not marked with 'await'}}{{9-9=await }}
@@ -534,7 +534,7 @@ func testGlobalRestrictions(actor: MyActor) async {
 
   // any kind of method can be called from outside the actor, so long as it's marked with 'await'
   _ = actor.synchronous() // expected-error{{expression is 'async' but is not marked with 'await'}}{{7-7=await }}
-  // expected-note@-1{{call is 'async}}
+  // expected-note@-1{{calls to instance method 'synchronous()' from outside of its actor context are implicitly asynchronous}}
   _ = actor.asynchronous() // expected-error{{expression is 'async' but is not marked with 'await'}}{{7-7=await }}
   // expected-note@-1{{call is 'async'}}
 
@@ -751,7 +751,7 @@ class SomeClassWithInits {
       await self.isolated() // expected-warning{{cannot use parameter 'self' with a non-sendable type 'SomeClassWithInits' from concurrently-executed code}}
       self.isolated() // expected-warning{{cannot use parameter 'self' with a non-sendable type 'SomeClassWithInits' from concurrently-executed code}}
       // expected-error@-1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }}
-      // expected-note@-2{{call is 'async'}}
+      // expected-note@-2{{calls to instance method 'isolated()' from outside of its actor context are implicitly asynchronous}}
 
       print(await self.mutableState) // expected-warning{{cannot use parameter 'self' with a non-sendable type 'SomeClassWithInits' from concurrently-executed code}}
     }
@@ -791,10 +791,10 @@ func testCrossActorProtocol<T: P>(t: T) async {
   await t.g()
   t.f()
   // expected-error@-1{{expression is 'async' but is not marked with 'await'}}{{3-3=await }}
-  // expected-note@-2{{call is 'async'}}
+  // expected-note@-2{{calls to instance method 'f()' from outside of its actor context are implicitly asynchronous}}
   t.g()
   // expected-error@-1{{expression is 'async' but is not marked with 'await'}}{{3-3=await }}
-  // expected-note@-2{{call is 'async'}}
+  // expected-note@-2{{calls to instance method 'g()' from outside of its actor context are implicitly asynchronous}}
 }
 
 // ----------------------------------------------------------------------
@@ -807,7 +807,8 @@ func acceptAsyncSendableClosureInheriting<T>(@_inheritActorContext _: @Sendable
 extension MyActor {
   func testSendableAndInheriting() {
     acceptAsyncSendableClosure {
-      synchronous() // expected-error{{actor-isolated instance method 'synchronous()' cannot be referenced from a concurrent closure}}
+      synchronous() // expected-error{{expression is 'async' but is not marked with 'await'}}
+      // expected-note@-1{{calls to instance method 'synchronous()' from outside of its actor context are implicitly asynchronous}}
     }
 
     acceptAsyncSendableClosure {

test/Concurrency/global_actor_from_ordinary_context.swift

@@ -109,7 +109,7 @@ class Taylor {
 func fromAsync() async {
   let x = syncGlobActorFn
   // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{3-3=await }}
-  x() // expected-note{{call is 'async'}}
+  x() // expected-note{{calls to let 'x' from outside of its actor context are implicitly asynchronous}}
 
 
   let y = asyncGlobalActFn
@@ -120,7 +120,7 @@ func fromAsync() async {
 
   let fn = a.method
   // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{3-3=await }}
-  fn() //expected-note{{call is 'async}}
+  fn() //expected-note{{calls to let 'fn' from outside of its actor context are implicitly asynchronous}}
   // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }}
   _ = a.const_memb // expected-note{{property access is 'async'}}
   // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }}

test/Concurrency/global_actor_function_types.swift

@@ -130,12 +130,14 @@ func testTypesConcurrencyContext() async {
   let _: () -> Int = f2 // expected-error{{converting function value of type '@SomeGlobalActor () -> Int' to '() -> Int' loses global actor 'SomeGlobalActor'}}
 
   // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }}
-  _ = f1() //expected-note{{call is 'async}}
+  _ = f1() //expected-note{{calls to let 'f1' from outside of its actor context are implicitly asynchronous}}
   // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }}
-  _ = f2() //expected-note{{call is 'async'}}
+  _ = f2() //expected-note{{calls to let 'f2' from outside of its actor context are implicitly asynchronous}}
 
-  // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }}
-  _ = f1() + f2() // expected-note 2 {{call is 'async'}}
+  // expected-error@+3{{expression is 'async' but is not marked with 'await'}}{{7-7=await }}
+  //expected-note@+2 {{calls to let 'f1' from outside of its actor context are implicitly asynchronous}}
+  // expected-note@+1 {{calls to let 'f2' from outside of its actor context are implicitly asynchronous}}
+  _ = f1() + f2()
 
   _ = await f1()
   _ = await f2()

test/Concurrency/global_actor_inference.swift

@@ -238,7 +238,7 @@ class SubclassWithGlobalActors : SuperclassWithGlobalActors {
     onGenericGlobalActorString() // okay
 
     // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{5-5=await }}
-    onGenericGlobalActorInt() // expected-note{{call is 'async'}}
+    onGenericGlobalActorInt() // expected-note{{calls to instance method 'onGenericGlobalActorInt()' from outside of its actor context are implicitly asynchronous}}
   }
 }
 
@@ -253,7 +253,7 @@ class SubclassWithGlobalActors : SuperclassWithGlobalActors {
 
 func bar() async {
   // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{3-3=await }}
-  foo() // expected-note{{call is 'async'}}
+  foo() // expected-note{{calls to global function 'foo()' from outside of its actor context are implicitly asynchronous}}
 }
 
 // expected-note@+1 {{add '@SomeGlobalActor' to make global function 'barSync()' part of global actor 'SomeGlobalActor'}} {{1-1=@SomeGlobalActor }}
@@ -509,7 +509,7 @@ func acceptClosure<T>(_: () -> T) { }
 // ----------------------------------------------------------------------
 func takesUnsafeMainActor(@_unsafeMainActor fn: () -> Void) { }
 
-@MainActor func onlyOnMainActor() { } // expected-note{{calls to global function 'onlyOnMainActor()' from outside of its actor context are implicitly asynchronous}}
+@MainActor func onlyOnMainActor() { }
 
 func useUnsafeMainActor() {
   takesUnsafeMainActor {
@@ -525,7 +525,8 @@ func acceptAsyncSendableClosureInheriting<T>(@_inheritActorContext _: @Sendable
 
 @MainActor func testCallFromMainActor() {
   acceptAsyncSendableClosure {
-    onlyOnMainActor() // expected-error{{call to main actor-isolated global function 'onlyOnMainActor()' in a synchronous nonisolated context}}
+    onlyOnMainActor() // expected-error{{expression is 'async' but is not marked with 'await'}}
+    // expected-note@-1 {{calls to global function 'onlyOnMainActor()' from outside of its actor context are implicitly asynchronous}}
   }
 
   acceptAsyncSendableClosure {