Tighten up actor isolation checking for closures and local functions.

Make sure that we check the isolation of the context in which a reference
to `self` is made, rather than the context in which `self` is declared,
when checking whether we are within actor-isolated code. This ensures
that we report errors as actor-isolation errors rather than falling
back to the "may execute concurrently with" checking.

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -4261,6 +4261,18 @@ ERROR(actor_isolated_concurrent_access,none,
         "actor-isolated %0 %1 is unsafe to reference in code "
         "that may execute concurrently",
         (DescriptiveDeclKind, DeclName))
+ERROR(actor_isolated_from_concurrent_closure,none,
+        "actor-isolated %0 %1 cannot be referenced from a concurrent closure",
+        (DescriptiveDeclKind, DeclName))
+ERROR(actor_isolated_from_concurrent_function,none,
+        "actor-isolated %0 %1 cannot be referenced from a concurrent function",
+        (DescriptiveDeclKind, DeclName))
+ERROR(actor_isolated_from_async_let,none,
+        "actor-isolated %0 %1 cannot be referenced from 'async let' initializer",
+        (DescriptiveDeclKind, DeclName))
+ERROR(actor_isolated_from_escaping_closure,none,
+        "actor-isolated %0 %1 cannot be referenced from an '@escaping' closure",
+        (DescriptiveDeclKind, DeclName))
 ERROR(local_function_executed_concurrently,none,
       "concurrently-executed %0 %1 must be marked as '@concurrent'",
       (DescriptiveDeclKind, DeclName))

lib/Sema/TypeCheckConcurrency.cpp

@@ -869,9 +869,8 @@ namespace {
     }
 
   private:
-    /// If the expression is a reference to `self`, return the context of
-    /// the 'self' parameter.
-    static DeclContext *getSelfReferenceContext(Expr *expr) {
+    /// If the expression is a reference to `self`, the `self` declaration.
+    static VarDecl *getReferencedSelf(Expr *expr) {
       // Look through identity expressions and implicit conversions.
       Expr *prior;
       do {
@@ -885,24 +884,13 @@ namespace {
 
       // 'super' references always act on self.
       if (auto super = dyn_cast<SuperRefExpr>(expr))
-        return super->getSelf()->getDeclContext();
+        return super->getSelf();
 
       // Declaration references to 'self'.
       if (auto declRef = dyn_cast<DeclRefExpr>(expr)) {
         if (auto var = dyn_cast<VarDecl>(declRef->getDecl())) {
-          if (var->isSelfParameter())
-            return var->getDeclContext();
-
-          // If this is a 'self' capture in a capture list, recurse through
-          // the capture list entry's initializer to find the original 'self'.
-          if (var->isSelfParamCapture()) {
-            for (auto capture : var->getParentCaptureList()->getCaptureList()) {
-              if (capture.Var == var) {
-                expr = capture.Init->getInit(0);
-                return getSelfReferenceContext(expr);
-              }
-            }
-          }
+          if (var->isSelfParameter() || var->isSelfParamCapture())
+            return var;
         }
       }
 
@@ -1268,6 +1256,44 @@ namespace {
       llvm_unreachable("unhandled actor isolation kind!");
     }
 
+    /// Determine the reason for the given declaration context to be
+    /// actor-independent.
+    static Diag<DescriptiveDeclKind, DeclName>
+    findActorIndependentReason(DeclContext *dc) {
+      if (auto autoclosure = dyn_cast<AutoClosureExpr>(dc)) {
+        switch (autoclosure->getThunkKind()) {
+        case AutoClosureExpr::Kind::AsyncLet:
+          return diag::actor_isolated_from_async_let;
+
+        case AutoClosureExpr::Kind::DoubleCurryThunk:
+        case AutoClosureExpr::Kind::SingleCurryThunk:
+          return findActorIndependentReason(dc->getParent());
+
+        case AutoClosureExpr::Kind::None:
+          break;
+        }
+      }
+
+      if (auto closure = dyn_cast<AbstractClosureExpr>(dc)) {
+        if (isConcurrentClosure(closure)) {
+          return diag::actor_isolated_from_concurrent_closure;
+        }
+
+        if (isEscapingClosure(closure)) {
+          return diag::actor_isolated_from_escaping_closure;
+        }
+
+        return findActorIndependentReason(dc->getParent());
+      }
+
+      if (auto func = dyn_cast<AbstractFunctionDecl>(dc)) {
+        if (func->isConcurrent())
+          return diag::actor_isolated_from_concurrent_function;
+      }
+
+      return diag::actor_isolated_self_independent_context;
+    }
+
     /// Check a reference with the given base expression to the given member.
     /// Returns true iff the member reference refers to actor-isolated state
     /// in an invalid or unsafe way such that a diagnostic was emitted.
@@ -1286,8 +1312,8 @@ namespace {
 
       case ActorIsolationRestriction::ActorSelf: {
         // Must reference actor-isolated state on 'self'.
-        auto *selfDC = getSelfReferenceContext(base);
-        if (!selfDC) {
+        auto *selfVar = getReferencedSelf(base);
+        if (!selfVar) {
           // actor-isolated non-self calls are implicitly async and thus OK.
           if (maybeImplicitAsync && isa<AbstractFunctionDecl>(member)) {
             markNearestCallAsImplicitlyAsync();
@@ -1303,8 +1329,9 @@ namespace {
           return true;
         }
 
-        // Check whether the context of 'self' is actor-isolated.
-        switch (auto contextIsolation = getActorIsolationOfContext(selfDC)) {
+        // Check whether the current context is differently-isolated.
+        auto curDC = const_cast<DeclContext *>(getDeclContext());
+        switch (auto contextIsolation = getActorIsolationOfContext(curDC)) {
           case ActorIsolation::ActorInstance:
             // An escaping partial application of something that is part of
             // the actor's isolated state is never permitted.
@@ -1323,15 +1350,15 @@ namespace {
             // Okay
             break;
 
-          case ActorIsolation::Independent:
+          case ActorIsolation::Independent: {
             // The 'self' is for an actor-independent member, which means
             // we cannot refer to actor-isolated state.
-            ctx.Diags.diagnose(
-                memberLoc, diag::actor_isolated_self_independent_context,
-                member->getDescriptiveKind(),
-                member->getName());
+            auto diag = findActorIndependentReason(curDC);
+            ctx.Diags.diagnose(memberLoc, diag, member->getDescriptiveKind(),
+                               member->getName());
             noteIsolatedActorMember(member);
             return true;
+          }
 
           case ActorIsolation::GlobalActor:
             // The 'self' is for a member that's part of a global actor, which
@@ -1347,7 +1374,8 @@ namespace {
 
         // Check whether we are in a context that will not execute concurrently
         // with the context of 'self'.
-        if (mayExecuteConcurrentlyWith(getDeclContext(), selfDC)) {
+        if (mayExecuteConcurrentlyWith(
+                getDeclContext(), selfVar->getDeclContext())) {
           ctx.Diags.diagnose(
               memberLoc, diag::actor_isolated_concurrent_access,
               member->getDescriptiveKind(), member->getName());

test/Concurrency/actor_isolation.swift

@@ -140,16 +140,16 @@ extension MyActor {
 
     // Concurrent closures might run... concurrently.
     acceptConcurrentClosure {
-      _ = self.text[0] // expected-error{{actor-isolated property 'text' is unsafe to reference in code that may execute concurrently}}
-      _ = self.synchronous() // expected-error{{actor-isolated instance method 'synchronous()' is unsafe to reference in code that may execute concurrently}}
+      _ = self.text[0] // expected-error{{actor-isolated property 'text' cannot be referenced from a concurrent closure}}
+      _ = self.synchronous() // expected-error{{actor-isolated instance method 'synchronous()' cannot be referenced from a concurrent closure}}
       _ = localVar // expected-warning{{local var 'localVar' is unsafe to reference in code that may execute concurrently}}
       _ = localConstant
     }
 
     // Escaping closures might run concurrently.
     acceptEscapingClosure {
-      _ = self.text[0] // expected-error{{actor-isolated property 'text' is unsafe to reference in code that may execute concurrently}}
-      _ = self.synchronous() // expected-error{{actor-isolated instance method 'synchronous()' is unsafe to reference in code that may execute concurrently}}
+      _ = self.text[0] // expected-error{{actor-isolated property 'text' cannot be referenced from an '@escaping' closure}}
+      _ = self.synchronous() // expected-error{{actor-isolated instance method 'synchronous()' cannot be referenced from an '@escaping' closure}}
       _ = localVar // expected-warning{{local var 'localVar' is unsafe to reference in code that may execute concurrently}}
       _ = localConstant
     }
@@ -164,8 +164,8 @@ extension MyActor {
 
     @concurrent func localFn2() {
       acceptClosure {
-        _ = text[0]  // expected-error{{actor-isolated property 'text' is unsafe to reference in code that may execute concurrently}}
-        _ = self.synchronous() // expected-error{{actor-isolated instance method 'synchronous()' is unsafe to reference in code that may execute concurrently}}
+        _ = text[0]  // expected-error{{actor-isolated property 'text' cannot be referenced from a concurrent function}}
+        _ = self.synchronous() // expected-error{{actor-isolated instance method 'synchronous()' cannot be referenced from a concurrent function}}
         _ = localVar // expected-warning{{local var 'localVar' is unsafe to reference in code that may execute concurrently}}
         _ = localConstant
       }
@@ -180,7 +180,7 @@ extension MyActor {
 
     // Partial application
     _ = synchronous  // expected-error{{actor-isolated instance method 'synchronous()' can not be partially applied}}
-    _ = super.superMethod // expected-error{{actor-isolated instance method 'superMethod()' is unsafe to reference in code that may execute concurrently}}
+    _ = super.superMethod // expected-error{{actor-isolated instance method 'superMethod()' can not be referenced from an '@actorIndependent' context}}
     acceptClosure(synchronous)
     acceptClosure(self.synchronous)
     acceptClosure(otherActor.synchronous) // expected-error{{actor-isolated instance method 'synchronous()' can only be referenced on 'self'}}
@@ -384,15 +384,15 @@ func checkLocalFunctions() async {
 
 actor class LazyActor {
     var v: Int = 0
-    // expected-note@-1 5 {{mutable state is only available within the actor instance}}
+    // expected-note@-1 6 {{mutable state is only available within the actor instance}}
 
     let l: Int = 0
 
     lazy var l11: Int = { v }()
     lazy var l12: Int = v
     lazy var l13: Int = { self.v }()
     lazy var l14: Int = self.v
-    lazy var l15: Int = { [unowned self] in self.v }()
+    lazy var l15: Int = { [unowned self] in self.v }() // expected-error{{actor-isolated property 'v' can not be referenced from an '@actorIndependent' context}}
 
     lazy var l21: Int = { l }()
     lazy var l22: Int = l

test/Concurrency/async_let_isolation.swift

@@ -10,12 +10,12 @@ actor class MyActor {
 
   func testAsyncLetIsolation() async {
     async let x = self.synchronous()
-    // expected-error @-1{{actor-isolated instance method 'synchronous()' is unsafe to reference in code that may execute concurrently}}
+    // expected-error @-1{{actor-isolated instance method 'synchronous()' cannot be referenced from 'async let' initializer}}
 
     async let y = await self.asynchronous()
 
     async let z = synchronous()
-    // expected-error @-1{{actor-isolated instance method 'synchronous()' is unsafe to reference in code that may execute concurrently}}
+    // expected-error @-1{{actor-isolated instance method 'synchronous()' cannot be referenced from 'async let' initializer}}
 
     var localText = text // expected-note{{var declared here}}
     async let w = localText.removeLast()