[SE-0306] Require 'nonisolated' on lets for synchronous cross-actor access.

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -4426,6 +4426,9 @@ NOTE(actor_isolated_sync_func,none,
 NOTE(actor_mutable_state,none,
      "mutation of this %0 is only permitted within the actor",
      (DescriptiveDeclKind))
+NOTE(actor_isolated_let,none,
+     "use `nonisolated` to allow synchronous access to 'let' from outside "
+     "the actor", ())
 WARNING(shared_mutable_state_access,none,
         "reference to %0 %1 is not concurrency-safe because it involves "
         "shared mutable state", (DescriptiveDeclKind, DeclName))
@@ -4458,13 +4461,12 @@ WARNING(non_concurrent_property_type,none,
 WARNING(non_concurrent_keypath_capture,none,
         "cannot form key path that captures non-sendable type %0",
         (Type))
-WARNING(non_concurrent_keypath_access,none,
-        "cannot form key path that accesses non-sendable type %0",
-        (Type))
 ERROR(non_concurrent_type_member,none,
       "%select{stored property %1|associated value %1}0 of "
       "'Sendable'-conforming %2 %3 has non-sendable type %4",
       (bool, DeclName, DescriptiveDeclKind, DeclName, Type))
+ERROR(non_sendable_nonisolated_let,none,
+      "non-isolated let property %0 has non-Sendable type %1", (DeclName, Type))
 ERROR(concurrent_value_class_mutable_property,none,
       "stored property %0 of 'Sendable'-conforming %1 %2 is mutable",
       (DeclName, DescriptiveDeclKind, DeclName))
@@ -4480,21 +4482,13 @@ ERROR(concurrent_value_inherit,none,
       "%select{| other than 'NSObject'}0",
       (bool, DeclName))
 
-ERROR(actorindependent_let,none,
-      "'@actorIndependent' is meaningless on 'let' declarations because "
-      "they are immutable",
-      ())
 ERROR(actorindependent_mutable_storage,none,
       "'@actorIndependent' can not be applied to stored properties",
       ())
 ERROR(actorindependent_local_var,none,
       "'@actorIndependent' can not be applied to local variables",
       ())
 
-ERROR(nonisolated_let,none,
-      "'nonisolated' is meaningless on 'let' declarations because "
-      "they are immutable",
-      ())
 ERROR(nonisolated_mutable_storage,none,
       "nonisolated' can not be applied to stored properties",
       ())

lib/Sema/TypeCheckAttr.cpp

@@ -5418,17 +5418,14 @@ void AttributeChecker::visitActorIndependentAttr(ActorIndependentAttr *attr) {
   // that do not have storage.
   auto dc = D->getDeclContext();
   if (auto var = dyn_cast<VarDecl>(D)) {
-    // @actorIndependent is meaningless on a `let`.
-    if (var->isLet()) {
-      diagnoseAndRemoveAttr(attr, diag::actorindependent_let);
-      return;
-    }
-
-    // @actorIndependent can not be applied to stored properties, unless if
+    // @actorIndependent can not be applied to mutable stored properties, unless if
     // the 'unsafe' option was specified
     if (var->hasStorage()) {
       switch (attr->getKind()) {
         case ActorIndependentKind::Safe:
+          if (var->isLet())
+            break;
+
           diagnoseAndRemoveAttr(attr, diag::actorindependent_mutable_storage);
           return;
 
@@ -5460,16 +5457,20 @@ void AttributeChecker::visitNonisolatedAttr(NonisolatedAttr *attr) {
   // that do not have storage.
   auto dc = D->getDeclContext();
   if (auto var = dyn_cast<VarDecl>(D)) {
-    // 'nonisolated' is meaningless on a `let`.
-    if (var->isLet()) {
-      diagnoseAndRemoveAttr(attr, diag::nonisolated_let);
-      return;
-    }
-
-    // 'nonisolated' can not be applied to stored properties.
+    // 'nonisolated' can only be applied to 'let' stored properties.
+    // Those must be Sendable.
     if (var->hasStorage()) {
-      diagnoseAndRemoveAttr(attr, diag::nonisolated_mutable_storage);
-      return;
+      if (!var->isLet()) {
+        diagnoseAndRemoveAttr(attr, diag::nonisolated_mutable_storage);
+        return;
+      }
+
+      // nonisolated lets must have Sendable type.
+      if (shouldDiagnoseNonSendableViolations(dc->getASTContext().LangOpts) &&
+          !isSendableType(dc, var->getType())) {
+        var->diagnose(
+            diag::non_sendable_nonisolated_let, var->getName(), var->getType());
+      }
     }
 
     // @actorIndependent can not be applied to local properties.

lib/Sema/TypeCheckConcurrency.cpp

@@ -636,21 +636,14 @@ ActorIsolationRestriction ActorIsolationRestriction::forDeclaration(
     if (cast<ValueDecl>(decl)->isLocalCapture())
       return forUnrestricted();
 
-    // 'let' declarations are immutable, so they can be accessed across
-    // actors.
-    bool isAccessibleAcrossActors = false;
-    if (auto var = dyn_cast<VarDecl>(decl)) {
-      if (var->isLet())
-        isAccessibleAcrossActors = true;
-    }
-
     // A function that provides an asynchronous context has no restrictions
     // on its access.
     //
     // FIXME: technically, synchronous functions are allowed to be cross-actor.
     // The call-sites are just conditionally async based on where they appear
     // (outside or inside the actor). This suggests that the implicitly-async
     // concept could be merged into the CrossActorSelf concept.
+    bool isAccessibleAcrossActors = false;
     if (auto func = dyn_cast<AbstractFunctionDecl>(decl)) {
       if (func->isAsyncContext())
         isAccessibleAcrossActors = true;
@@ -963,8 +956,7 @@ static bool diagnoseNonConcurrentProperty(
 
 /// Whether we should diagnose cases where Sendable conformances are
 /// missing.
-static bool shouldDiagnoseNonSendableViolations(
-    const LangOptions &langOpts) {
+bool swift::shouldDiagnoseNonSendableViolations(const LangOptions &langOpts) {
   return langOpts.WarnConcurrency;
 }
 
@@ -1468,11 +1460,17 @@ namespace {
         // was it an attempt to mutate an actor instance's isolated state?
       } else if (auto environment = kindOfUsage(decl, context)) {
 
-        if (environment.getValue() == VarRefUseEnv::Read)
+        if (isa<VarDecl>(decl) && cast<VarDecl>(decl)->isLet()) {
+          auto diag = decl->diagnose(diag::actor_isolated_let);
+          SourceLoc fixItLoc =
+              decl->getAttributeInsertionLoc(/*forModifier=*/true);
+          if (fixItLoc.isValid())
+            diag.fixItInsert(fixItLoc, "nonisolated ");
+        } else if (environment.getValue() == VarRefUseEnv::Read) {
           decl->diagnose(diag::kind_declared_here, decl->getDescriptiveKind());
-        else
+        } else {
           decl->diagnose(diag::actor_mutable_state, decl->getDescriptiveKind());
-
+        }
       } else {
         decl->diagnose(diag::kind_declared_here, decl->getDescriptiveKind());
       }
@@ -1645,11 +1643,6 @@ namespace {
 
       // is it an access to a property?
       if (isPropOrSubscript(decl)) {
-        // we assume let-bound properties are taken care of elsewhere,
-        // since they are never implicitly async.
-        assert(!isa<VarDecl>(decl) || cast<VarDecl>(decl)->isLet() == false
-               && "unexpected let-bound property; never implicitly async!");
-
         if (auto declRef = dyn_cast_or_null<DeclRefExpr>(context)) {
           if (usageEnv(declRef) == VarRefUseEnv::Read) {
 
@@ -1979,27 +1972,6 @@ namespace {
     bool checkKeyPathExpr(KeyPathExpr *keyPath) {
       bool diagnosed = false;
 
-      // returns None if it is not a 'let'-bound var decl. Otherwise,
-      // the bool indicates whether a diagnostic was emitted.
-      auto checkLetBoundVarDecl = [&](KeyPathExpr::Component const& component)
-                                                            -> Optional<bool> {
-        auto decl = component.getDeclRef().getDecl();
-        if (auto varDecl = dyn_cast<VarDecl>(decl)) {
-          if (varDecl->isLet()) {
-            auto type = component.getComponentType();
-            if (shouldDiagnoseNonSendableViolations(ctx.LangOpts)
-                && !isSendableType(getDeclContext(), type)) {
-              ctx.Diags.diagnose(
-                  component.getLoc(), diag::non_concurrent_keypath_access,
-                  type);
-              return true;
-            }
-            return false;
-          }
-        }
-        return None;
-      };
-
       // check the components of the keypath.
       for (const auto &component : keyPath->getComponents()) {
         // The decl referred to by the path component cannot be within an actor.
@@ -2027,13 +1999,6 @@ namespace {
             LLVM_FALLTHROUGH; // otherwise, it's invalid so diagnose it.
 
           case ActorIsolationRestriction::CrossActorSelf:
-            // 'let'-bound decls with this isolation are OK, just check them.
-            if (auto wasLetBound = checkLetBoundVarDecl(component)) {
-              diagnosed = wasLetBound.getValue();
-              break;
-            }
-            LLVM_FALLTHROUGH; // otherwise, it's invalid so diagnose it.
-
           case ActorIsolationRestriction::ActorSelf: {
             auto decl = concDecl.getDecl();
             ctx.Diags.diagnose(component.getLoc(),

lib/Sema/TypeCheckConcurrency.h

@@ -38,6 +38,7 @@ class EnumElementDecl;
 class Expr;
 class FuncDecl;
 class Initializer;
+class LangOptions;
 class PatternBindingDecl;
 class ProtocolConformance;
 class TopLevelCodeDecl;
@@ -213,6 +214,10 @@ bool diagnoseNonConcurrentTypesInReference(
     ConcurrentReferenceKind refKind,
     DiagnosticBehavior behavior = DiagnosticBehavior::Unspecified);
 
+/// Whether we should diagnose cases where Sendable conformances are
+/// missing.
+bool shouldDiagnoseNonSendableViolations(const LangOptions &langOpts);
+
 /// How the concurrent value check should be performed.
 enum class SendableCheck {
   /// Sendable conformance was explicitly stated and should be

test/Concurrency/Runtime/actor_keypaths.swift

@@ -7,7 +7,7 @@
 // UNSUPPORTED: back_deployment_runtime
 
 actor Page {
-    let initialNumWords : Int
+    nonisolated let initialNumWords : Int
 
     @actorIndependent(unsafe)
     var numWords : Int
@@ -19,7 +19,7 @@ actor Page {
 }
 
 actor Book {
-    let pages : [Page]
+    nonisolated let pages : [Page]
 
     init(_ numPages : Int) {
         var stack : [Page] = []

test/Concurrency/actor_isolation.swift

@@ -49,7 +49,7 @@ class Point {
 
 @available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *)
 actor MyActor: MySuperActor {
-  let immutable: Int = 17
+  nonisolated let immutable: Int = 17
   // expected-note@+2 2{{property declared here}}
   // expected-note@+1 6{{mutation of this property is only permitted within the actor}}
   var mutable: Int = 71
@@ -58,7 +58,8 @@ actor MyActor: MySuperActor {
   // expected-note@+1 4{{property declared here}}
   var text: [String] = []
 
-  let point : Point = Point()
+  nonisolated let point : Point = Point() // expected-error{{non-isolated let property 'point' has non-Sendable type 'Point'}}
+  let otherPoint = Point()
 
   @MainActor
   var name : String = "koala" // expected-note{{property declared here}}
@@ -102,7 +103,10 @@ func checkAsyncPropertyAccess() async {
 
   act.text[0] += "hello" // expected-error{{actor-isolated property 'text' can only be mutated from inside the actor}}
 
-  _ = act.point // expected-warning{{cannot use property 'point' with a non-sendable type 'Point' across actors}}
+  _ = act.point
+  _ = act.otherPoint // expected-error{{property access is 'async' but is not marked with 'await'}}
+  // expected-warning@-1{{cannot use property 'otherPoint' with a non-sendable type 'Point' across actors}}
+  _ = await act.otherPoint // expected-warning{{cannot use property 'otherPoint' with a non-sendable type 'Point' across actors}}
 }
 
 @available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *)
@@ -615,7 +619,7 @@ actor LazyActor {
     var v: Int = 0
     // expected-note@-1 6 {{property declared here}}
 
-    let l: Int = 0
+    nonisolated let l: Int = 0
 
     lazy var l11: Int = { v }()
     lazy var l12: Int = v

test/Concurrency/actor_isolation_objc.swift

@@ -28,7 +28,7 @@ actor A {
     _ = #keyPath(A.z)
   }
 
-  let w: Int = 0 // expected-note{{add '@objc' to expose this property to Objective-C}}
+  nonisolated let w: Int = 0 // expected-note{{add '@objc' to expose this property to Objective-C}}
 
   var x: Int = 0 // expected-note{{add '@objc' to expose this property to Objective-C}}
 

test/Concurrency/actor_keypath_isolation.swift

@@ -6,10 +6,10 @@ class Box {
 }
 
 actor Door {
-    let immutable : Int = 0
+    nonisolated let immutable : Int = 0
     let letBox : Box? = nil
     let letDict : [Int : Box] = [:]
-    let immutableNeighbor : Door? = nil
+    nonisolated let immutableNeighbor : Door? = nil
 
 
     var mutableNeighbor : Door? = nil
@@ -47,7 +47,7 @@ func tryKeyPathsMisc(d : Door) {
 
     // in combination with other key paths
 
-    _ = (\Door.letBox).appending(path:  // expected-warning {{cannot form key path that accesses non-sendable type 'Box?'}}
+    _ = (\Door.letBox).appending(path:  // expected-error {{cannot form key path to actor-isolated property 'letBox'}}
                                        \Box?.?.size)
 
     _ = (\Door.varBox).appending(path:  // expected-error {{cannot form key path to actor-isolated property 'varBox'}}
@@ -61,9 +61,9 @@ func tryKeyPathsFromAsync() async {
 }
 
 func tryNonSendable() {
-    _ = \Door.letDict[0] // expected-warning {{cannot form key path that accesses non-sendable type '[Int : Box]'}}
+    _ = \Door.letDict[0] // expected-error {{cannot form key path to actor-isolated property 'letDict'}}
     _ = \Door.varDict[0] // expected-error {{cannot form key path to actor-isolated property 'varDict'}}
-    _ = \Door.letBox!.size // expected-warning {{cannot form key path that accesses non-sendable type 'Box?'}}
+    _ = \Door.letBox!.size // expected-error {{cannot form key path to actor-isolated property 'letBox'}}
 }
 
 func tryKeypaths() {

test/Concurrency/concurrent_value_checking.swift

@@ -37,7 +37,7 @@ extension A1 {
     _ = await self.asynchronous(nil)
 
     // Across to a different actor, so Sendable restriction is enforced.
-    _ = other.localLet // expected-warning{{cannot use property 'localLet' with a non-sendable type 'NotConcurrent' across actors}}
+    _ = await other.localLet // expected-warning{{cannot use property 'localLet' with a non-sendable type 'NotConcurrent' across actors}}
     _ = await other.synchronous() // expected-warning{{cannot call function returning non-sendable type 'NotConcurrent?' across actors}}
     _ = await other.asynchronous(nil) // expected-warning{{cannot pass argument of non-sendable type 'NotConcurrent?' across actors}}
   }
@@ -67,7 +67,7 @@ func globalAsync(_: NotConcurrent?) async {
 }
 
 func globalTest() async {
-  let a = globalValue // expected-warning{{cannot use let 'globalValue' with a non-sendable type 'NotConcurrent?' across actors}}
+  let a = await globalValue // expected-warning{{cannot use let 'globalValue' with a non-sendable type 'NotConcurrent?' across actors}}
   await globalAsync(a) // expected-warning{{cannot pass argument of non-sendable type 'NotConcurrent?' across actors}}
   await globalSync(a)  // expected-warning{{cannot pass argument of non-sendable type 'NotConcurrent?' across actors}}
 }
@@ -88,7 +88,7 @@ class ClassWithGlobalActorInits {
 
 @MainActor
 func globalTestMain(nc: NotConcurrent) async {
-  let a = globalValue // expected-warning{{cannot use let 'globalValue' with a non-sendable type 'NotConcurrent?' across actors}}
+  let a = await globalValue // expected-warning{{cannot use let 'globalValue' with a non-sendable type 'NotConcurrent?' across actors}}
   await globalAsync(a) // expected-warning{{cannot pass argument of non-sendable type 'NotConcurrent?' across actors}}
   await globalSync(a)  // expected-warning{{cannot pass argument of non-sendable type 'NotConcurrent?' across actors}}
   _ = await ClassWithGlobalActorInits(nc) // expected-warning{{cannot pass argument of non-sendable type 'NotConcurrent' across actors}}

test/Concurrency/global_actor_from_ordinary_context.swift

@@ -31,7 +31,7 @@ actor Alex {
 func referenceGlobalActor() async {
   let a = Alex()
   _ = a.method
-  _ = a.const_memb
+  _ = a.const_memb // expected-error{{property access is 'async' but is not marked with 'await'}}
   _ = a.mut_memb  // expected-error{{property access is 'async' but is not marked with 'await'}}
 
   _ = a[1]  // expected-error{{subscript access is 'async' but is not marked with 'await'}}
@@ -110,7 +110,7 @@ func fromAsync() async {
   let a = Alex()
   let fn = a.method
   fn() // expected-error{{call is 'async' but is not marked with 'await'}}
-  _ = a.const_memb
+  _ = a.const_memb // expected-error{{property access is 'async' but is not marked with 'await'}}
   _ = a.mut_memb  // expected-error{{property access is 'async' but is not marked with 'await'}}
 
   _ = a[1]  // expected-error{{subscript access is 'async' but is not marked with 'await'}}