Merge pull request #81719 from xedin/rdar-151720646

[TypeChecker] Improve diagnostics for access to actor-isolated values…

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -5626,6 +5626,9 @@ ERROR(actor_isolated_non_self_reference,none,
         "from a nonisolated autoclosure}2",
         (const ValueDecl *, unsigned, unsigned, Type,
          ActorIsolation))
+ERROR(actor_isolated_access_outside_of_actor_context,none,
+      "%0 %kind1 cannot be %select{accessed|called}2 from outside of the actor",
+      (ActorIsolation, const ValueDecl *, bool))
 ERROR(distributed_actor_isolated_non_self_reference,none,
       "distributed actor-isolated %kind0 can not be accessed from a "
       "nonisolated context",

lib/Sema/TypeCheckEffects.cpp

@@ -4718,6 +4718,12 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
           return diag.downgradeToWarning;
         });
 
+    // If there is a single problem, let's attempt to produce a tailed
+    // diagnostic about accessing isolated values outside of their actors.
+    if (errors.size() == 1 &&
+        diagnoseAccessOutsideOfIsolationContext(anchor, errors.front()))
+      return;
+
     Ctx.Diags.diagnose(anchor->getStartLoc(), diag::async_expr_without_await)
       .warnUntilSwiftVersionIf(downgradeToWarning, 6)
       .fixItInsert(loc, insertText)
@@ -4800,6 +4806,76 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
     }
   }
 
+  /// Check whether the given error points to an attempt to access
+  /// an isolated value or call an isolated function from outside
+  /// its actor and diagnose if so.
+  /// \returns true if problem was diagnosed, false otherwise.
+  bool diagnoseAccessOutsideOfIsolationContext(
+      const Expr *anchor, const DiagnosticInfo &errorInfo) const {
+    auto diagnoseAccessOutsideOfActor = [&](SourceLoc loc,
+                                            ConcreteDeclRef declRef,
+                                            bool isCall = false) {
+      auto declIsolation = getActorIsolation(declRef.getDecl());
+
+      // If the access is to a unspecified/nonisolated value, let's diagnose
+      // it with a generic error/warning about expression being `async`.
+      if (declIsolation.isUnspecified() || declIsolation.isNonisolated())
+        return false;
+
+      const auto &[fixItLoc, insertText] =
+          getFixItForUncoveredSite(anchor, "await");
+
+      Ctx.Diags
+          .diagnose(loc, diag::actor_isolated_access_outside_of_actor_context,
+                    declIsolation, declRef.getDecl(), isCall)
+          .warnUntilSwiftVersionIf(errorInfo.downgradeToWarning, 6)
+          .fixItInsert(fixItLoc, insertText)
+          .highlight(anchor->getSourceRange());
+      return true;
+    };
+
+    switch (errorInfo.reason.getKind()) {
+    case PotentialEffectReason::Kind::AsyncLet:
+    case PotentialEffectReason::Kind::PropertyAccess:
+    case PotentialEffectReason::Kind::SubscriptAccess:
+      if (auto *declRef = dyn_cast<DeclRefExpr>(&errorInfo.expr)) {
+        return diagnoseAccessOutsideOfActor(declRef->getLoc(),
+                                            declRef->getDecl());
+      }
+
+      if (auto *memberRef = dyn_cast<MemberRefExpr>(&errorInfo.expr)) {
+        return diagnoseAccessOutsideOfActor(memberRef->getLoc(),
+                                            memberRef->getDecl());
+      }
+
+      if (auto *lookupExpr = dyn_cast<LookupExpr>(&errorInfo.expr)) {
+        return diagnoseAccessOutsideOfActor(lookupExpr->getLoc(),
+                                            lookupExpr->getMember());
+      }
+
+      break;
+
+    case PotentialEffectReason::Kind::Apply: {
+      auto *call = dyn_cast<ApplyExpr>(&errorInfo.expr);
+      if (call && call->getIsolationCrossing()) {
+        if (auto callee =
+                call->getCalledValue(/*skipFunctionConversions=*/true)) {
+          return diagnoseAccessOutsideOfActor(call->getLoc(), callee,
+                                              /*isCall=*/true);
+        }
+      }
+      break;
+    }
+
+    case PotentialEffectReason::Kind::ByClosure:
+    case PotentialEffectReason::Kind::ByDefaultClosure:
+    case PotentialEffectReason::Kind::ByConformance:
+      break;
+    }
+
+    return false;
+  }
+
   void diagnoseUncoveredUnsafeSite(
       const Expr *anchor, ArrayRef<UnsafeUse> unsafeUses) {
     if (!Ctx.LangOpts.hasFeature(Feature::StrictMemorySafety, /*allowMigration=*/true))

test/Concurrency/actor_call_implicitly_async.swift

@@ -168,7 +168,7 @@ func someAsyncFunc() async {
 
   _ = await a.deposit(b.withdraw(a.deposit(b.withdraw(b.balance()))))
 
-  // expected-error@+1 {{expression is 'async' but is not marked with 'await'}}{{3-3=await }} expected-note@+1 {{call is 'async'}}
+  // expected-error@+1 {{actor-isolated instance method 'testSelfBalance()' cannot be called from outside of the actor}} {{3-3=await }}
   a.testSelfBalance()
 
   await a.testThrowing() // expected-error {{call can throw, but it is not marked with 'try' and the error is not handled}}
@@ -177,16 +177,16 @@ func someAsyncFunc() async {
   // effectful properties from outside the actor instance
 
   // expected-warning@+2 {{non-Sendable type 'Box' of property 'effPropA' cannot exit actor-isolated context}}
-  // expected-error@+1{{expression is 'async' but is not marked with 'await'}} {{7-7=await }} expected-note@+1{{property access is 'async'}}
+  // expected-error@+1{{actor-isolated property 'effPropA' cannot be accessed from outside of the actor}} {{7-7=await }}
   _ = a.effPropA
 
   // expected-warning@+3 {{non-Sendable type 'Box' of property 'effPropT' cannot exit actor-isolated context}}
   // expected-error@+2{{property access can throw, but it is not marked with 'try' and the error is not handled}}
-  // expected-error@+1{{expression is 'async' but is not marked with 'await'}} {{7-7=await }} expected-note@+1{{property access is 'async'}}
+  // expected-error@+1{{actor-isolated property 'effPropT' cannot be accessed from outside of the actor}} {{7-7=await }}
   _ = a.effPropT
 
   // expected-error@+2{{property access can throw, but it is not marked with 'try' and the error is not handled}}
-  // expected-error@+1{{expression is 'async' but is not marked with 'await'}} {{7-7=await }} expected-note@+1{{property access is 'async'}}
+  // expected-error@+1{{actor-isolated property 'effPropAT' cannot be accessed from outside of the actor}} {{7-7=await }}
   _ = a.effPropAT
 
   // (mostly) corrected ones
@@ -204,9 +204,9 @@ func someAsyncFunc() async {
 
 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{{calls to instance method 'balance()' from outside of its actor context are implicitly asynchronous}}
+      + other.balance()
+    // expected-error@-1 {{actor-isolated instance method 'balance()' cannot be called from outside of the actor}}{{207:12-12=await }}
   }
 
   func breakAccounts(other: BankAccount) async {
@@ -223,19 +223,17 @@ func anotherAsyncFunc() async {
   let a = BankAccount(initialDeposit: 34)
   let b = BankAccount(initialDeposit: 35)
 
-  // 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}}
+  // expected-error@+1{{actor-isolated instance method 'deposit' cannot be called from outside of the actor}} {{7-7=await }}
   _ = a.deposit(1)
-  // 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}}
+  // expected-error@+1{{actor-isolated instance method 'balance()' cannot be called from outside of the actor}} {{7-7=await }}
   _ = b.balance()
 
   _ = b.balance // expected-error {{actor-isolated instance method 'balance()' can not be partially applied}}
 
   // expected-error@+2{{actor-isolated property 'owner' can not be mutated from a nonisolated context}}
   // expected-note@+1{{consider declaring an isolated method on 'BankAccount' to perform the mutation}}
   a.owner = "cat"
-  // expected-error@+1{{expression is 'async' but is not marked with 'await'}} {{7-7=await }} expected-note@+1{{property access is 'async'}}
+  // expected-error@+1{{actor-isolated property 'owner' cannot be accessed from outside of the actor}} {{7-7=await }}
   _ = b.owner
   _ = await b.owner == "cat"
 
@@ -334,7 +332,7 @@ func walkChain(chain : Chain) async {
 
 @OrangeActor func quinoa() async {
 
-  // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{3-3=await }} expected-note@+1 {{call is 'async'}}
+  // expected-error@+1{{global actor 'BananaActor'-isolated global function 'rice()' cannot be called from outside of the actor}}{{3-3=await }}
   rice()
 }
 
@@ -458,21 +456,21 @@ func tryEffPropsFromSync() {
 }
 
 @OrangeActor func tryEffPropertiesFromGlobalActor() async throws {
-  // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }} expected-note@+1 {{property access is 'async'}}
+  // expected-error@+1{{global actor 'BananaActor'-isolated var 'effPropA' cannot be accessed from outside of the actor}}{{7-7=await }}
   _ = effPropA
 
   // expected-note@+5{{did you mean to handle error as optional value?}}
   // expected-note@+4{{did you mean to use 'try'?}}
   // expected-note@+3{{did you mean to disable error propagation?}}
   // expected-error@+2{{property access can throw but is not marked with 'try'}}
-  // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }} expected-note@+1 {{property access is 'async'}}
+  // expected-error@+1{{global actor 'BananaActor'-isolated var 'effPropT' cannot be accessed from outside of the actor}}{{7-7=await }}
   _ = effPropT
 
   // expected-note@+5{{did you mean to handle error as optional value?}}
   // expected-note@+4{{did you mean to use 'try'?}}
   // expected-note@+3{{did you mean to disable error propagation?}}
   // expected-error@+2{{property access can throw but is not marked with 'try'}}
-  // expected-error@+1{{expression is 'async' but is not marked with 'await'}}{{7-7=await }} expected-note@+1 {{property access is 'async'}}
+  // expected-error@+1{{global actor 'BananaActor'-isolated var 'effPropAT' cannot be accessed from outside of the actor}}{{7-7=await }}
   _ = effPropAT
 
   _ = await effPropA
@@ -494,7 +492,7 @@ actor SubscriptA {
 
   func f() async {
 
-    // expected-error@+1{{expression is 'async' but is not marked with 'await'}} {{9-9=await }} expected-note@+1{{subscript access is 'async'}}
+    // expected-error@+1{{actor-isolated subscript 'subscript(_:)' cannot be accessed from outside of the actor}} {{9-9=await }}
     _ = self[0]
   }
 }
@@ -543,7 +541,7 @@ actor SubscriptAT {
 }
 
 func tryTheActorSubscripts(a : SubscriptA, t : SubscriptT, at : SubscriptAT) async throws {
-  // expected-error@+1 {{expression is 'async' but is not marked with 'await'}}{{7-7=await }} expected-note@+1 {{subscript access is 'async'}}
+  // expected-error@+1 {{actor-isolated subscript 'subscript(_:)' cannot be accessed from outside of the actor}}{{7-7=await }}
   _ = a[0]
 
   _ = await a[0]
@@ -552,7 +550,7 @@ func tryTheActorSubscripts(a : SubscriptA, t : SubscriptT, at : SubscriptAT) asy
   // expected-note@+4{{did you mean to use 'try'?}}
   // expected-note@+3{{did you mean to disable error propagation?}}
   // expected-error@+2{{subscript access can throw but is not marked with 'try'}}
-  // expected-error@+1 {{expression is 'async' but is not marked with 'await'}}{{7-7=await }} expected-note@+1 {{subscript access is 'async'}}
+  // expected-error@+1 {{actor-isolated subscript 'subscript(_:)' cannot be accessed from outside of the actor}}{{7-7=await }}
   _ = t[0]
 
   _ = try await t[0]
@@ -563,7 +561,7 @@ func tryTheActorSubscripts(a : SubscriptA, t : SubscriptT, at : SubscriptAT) asy
   // expected-note@+4{{did you mean to use 'try'?}}
   // expected-note@+3{{did you mean to disable error propagation?}}
   // expected-error@+2{{subscript access can throw but is not marked with 'try'}}
-  // expected-error@+1 {{expression is 'async' but is not marked with 'await'}}{{7-7=await }} expected-note@+1 {{subscript access is 'async'}}
+  // expected-error@+1 {{actor-isolated subscript 'subscript(_:)' cannot be accessed from outside of the actor}}{{7-7=await }}
   _ = at[0]
 
   _ = try await at[0]
@@ -583,8 +581,7 @@ final class IsolatedOperator: @preconcurrency Equatable {
 
   nonisolated func callEqual() async -> Bool {
     let foo = await IsolatedOperator()
-    // expected-error@+2{{expression is 'async' but is not marked with 'await'}}
-    // expected-note@+1{{calls to operator function '==' from outside of its actor context are implicitly asynchronous}}
+    // expected-error@+1{{main actor-isolated operator function '==' cannot be called from outside of the actor}} {{12-12=await }}
     return foo == self
   }
 }