Merge pull request #70909 from hborla/nonisolated-nonsendable

Author: hborla

include/swift/AST/DiagnosticsSema.def

@@ -5602,9 +5602,6 @@ WARNING(non_sendable_property_type,none,
 WARNING(non_sendable_keypath_capture,none,
         "cannot form key path that captures non-sendable type %0",
         (Type))
-WARNING(non_sendable_keypath_access,none,
-        "cannot form key path that accesses non-sendable type %0",
-        (Type))
 ERROR(non_concurrent_type_member,none,
       "%select{stored property %2|associated value %2}1 of "
       "'Sendable'-conforming %kind3 has non-sendable type %0",
@@ -5668,6 +5665,10 @@ NOTE(nonisolated_mutable_storage_note,none,
      "convert %0 to a 'let' constant or consider declaring it "
      "'nonisolated(unsafe)' if manually managing concurrency safety",
      (const VarDecl *))
+ERROR(nonisolated_non_sendable,none,
+      "'nonisolated' can not be applied to variable with non-'Sendable' "
+      "type %0",
+      (Type))
 ERROR(nonisolated_local_var,none,
       "'nonisolated' can not be applied to local variables",
       ())

lib/SILOptimizer/Mandatory/FlowIsolation.cpp

@@ -497,9 +497,8 @@ static bool accessIsConcurrencySafe(ModuleDecl *module,
                                     RefElementAddrInst *inst) {
   VarDecl *var = inst->getField();
 
-  // must be accessible from nonisolated and Sendable
-  return isLetAccessibleAnywhere(module, var)
-      && var->getTypeInContext()->isSendableType();
+  // must be accessible from nonisolated.
+  return isLetAccessibleAnywhere(module, var);
 }
 
 /// \returns true iff the ref_element_addr instruction is only used

lib/Sema/TypeCheckAttr.cpp

@@ -6880,6 +6880,17 @@ void AttributeChecker::visitNonisolatedAttr(NonisolatedAttr *attr) {
         return;
       }
 
+      // 'nonisolated' without '(unsafe)' is not allowed on non-Sendable variables.
+      auto type = var->getTypeInContext();
+      if (!attr->isUnsafe() && !type->hasError() &&
+          !type->isSendableType()) {
+        Ctx.Diags.diagnose(attr->getLocation(),
+                           diag::nonisolated_non_sendable,
+                           type)
+          .warnUntilSwiftVersion(6);
+        return;
+      }
+
       if (auto nominal = dyn_cast<NominalTypeDecl>(dc)) {
         // 'nonisolated' can not be applied to stored properties inside
         // distributed actors. Attempts of nonisolated access would be

lib/Sema/TypeCheckConcurrency.cpp

@@ -543,8 +543,22 @@ static bool varIsSafeAcrossActors(const ModuleDecl *fromModule,
         return false;
     }
 
+    bool accessWithinModule =
+        (fromModule == var->getDeclContext()->getParentModule());
+
+    // If the type is not 'Sendable', it's unsafe
+    if (!var->getTypeInContext()->isSendableType()) {
+      // Compiler versions <= 5.10 treated this variable as nonisolated,
+      // so downgrade async access errors in the effects checker to
+      // warnings prior to Swift 6.
+      if (accessWithinModule)
+        options = ActorReferenceResult::Flags::Preconcurrency;
+
+      return false;
+    }
+
     // If it's actor-isolated but in the same module, then it's OK too.
-    return (fromModule == var->getDeclContext()->getParentModule());
+    return accessWithinModule;
   }
 }
 
@@ -3639,34 +3653,26 @@ namespace {
             LLVM_FALLTHROUGH;
           }
 
-          case ActorIsolation::ActorInstance:
-            // If this entity is always accessible across actors, just check
-            // Sendable.
+          case ActorIsolation::ActorInstance: {
+            ActorReferenceResult::Options options = llvm::None;
             if (isAccessibleAcrossActors(decl, isolation, getDeclContext(),
-                                         llvm::None)) {
-              if (diagnoseNonSendableTypes(
-                             component.getComponentType(), getDeclContext(),
-                             /*inDerivedConformance*/Type(),
-                             component.getLoc(),
-                             diag::non_sendable_keypath_access)) {
-                diagnosed = true;
-              }
+                                         options)) {
               break;
             }
 
-            {
-              auto diagnostic = ctx.Diags.diagnose(
-                  component.getLoc(), diag::actor_isolated_keypath_component,
-                  isolation, decl);
+            bool downgrade = isolation.isGlobalActor() ||
+              options.contains(
+                  ActorReferenceResult::Flags::Preconcurrency);
 
-              if (isolation == ActorIsolation::ActorInstance)
-                diagnosed = true;
-              else
-                diagnostic.warnUntilSwiftVersion(6);
-            }
+            ctx.Diags.diagnose(
+                component.getLoc(), diag::actor_isolated_keypath_component,
+                isolation, decl)
+              .warnUntilSwiftVersionIf(downgrade, 6);
 
+            diagnosed = !downgrade;
             break;
           }
+          }
         }
 
         // With `InferSendableFromCaptures` feature enabled the solver is

lib/Sema/TypeCheckEffects.cpp

@@ -1040,6 +1040,7 @@ class ApplyClassifier {
 
 public:
   ASTContext &Ctx;
+  DeclContext *DC = nullptr;
   DeclContext *RethrowsDC = nullptr;
   DeclContext *ReasyncDC = nullptr;
 
@@ -1083,9 +1084,12 @@ class ApplyClassifier {
 
     if (auto *var = dyn_cast<VarDecl>(decl)) {
       ActorReferenceResult::Options options = llvm::None;
-      // The newly-diagnosed cases are invalid regardless of the module context
-      // of the caller, i.e. isolated static and global 'let' variables.
-      auto *module = var->getDeclContext()->getParentModule();
+      ModuleDecl *module;
+      if (DC != nullptr) {
+        module = DC->getParentModule();
+      } else {
+        module = var->getDeclContext()->getParentModule();
+      }
       if (!isLetAccessibleAnywhere(module, var, options)) {
         return options.contains(ActorReferenceResult::Flags::Preconcurrency);
       }
@@ -3064,6 +3068,7 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
 
   ApplyClassifier getApplyClassifier() const {
     ApplyClassifier classifier(Ctx);
+    classifier.DC = CurContext.getDeclContext();
     classifier.RethrowsDC = RethrowsDC;
     classifier.ReasyncDC = ReasyncDC;
     return classifier;

test/Concurrency/actor_isolation.swift

@@ -114,7 +114,9 @@ func checkAsyncPropertyAccess() async {
 
   act.text[0] += "hello" // expected-error{{actor-isolated property 'text' can not be mutated from a non-isolated context}}
 
-  _ = act.point  // expected-warning{{non-sendable type 'Point' in asynchronous access to actor-isolated property 'point' cannot cross actor boundary}}
+  _ = act.point  // expected-warning{{non-sendable type 'Point' in implicitly asynchronous access to actor-isolated property 'point' cannot cross actor boundary}}
+  // expected-warning@-1 {{expression is 'async' but is not marked with 'await'}}
+  // expected-note@-2 {{property access is 'async'}}
 }
 
 /// ------------------------------------------------------------------
@@ -1538,11 +1540,11 @@ class OverridesNonsiolatedInit: SuperWithNonisolatedInit {
   }
 }
 
-// expected-note@+1 2 {{class 'NonSendable' does not conform to the 'Sendable' protocol}}
+// expected-note@+1 {{class 'NonSendable' does not conform to the 'Sendable' protocol}}
 class NonSendable {}
 
 actor ProtectNonSendable {
-  // expected-note@+1 {{property declared here}}
+  // expected-note@+1 2 {{property declared here}}
   let ns = NonSendable()
 
   init() {}
@@ -1560,7 +1562,9 @@ class ReferenceActor {
   init() async {
     self.a = ProtectNonSendable()
 
-    // expected-warning@+1 {{non-sendable type 'NonSendable' in asynchronous access to actor-isolated property 'ns' cannot cross actor boundary}}
+    // expected-warning@+3 {{non-sendable type 'NonSendable' in implicitly asynchronous access to actor-isolated property 'ns' cannot cross actor boundary}}
+    // expected-warning@+2 {{expression is 'async' but is not marked with 'await'}}
+    // expected-note@+1 {{property access is 'async'}}
     _ = a.ns
   }
 }
@@ -1571,7 +1575,7 @@ actor AnotherActor {
   init() {
     self.a = ProtectNonSendable()
 
-    // expected-warning@+1 {{non-sendable type 'NonSendable' in asynchronous access to actor-isolated property 'ns' cannot cross actor boundary}}
+    // expected-warning@+1 {{actor-isolated property 'ns' can not be referenced from a non-isolated context}}
     _ = a.ns
   }
 }

test/Concurrency/actor_keypath_isolation.swift

@@ -4,7 +4,7 @@
 // REQUIRES: concurrency
 // REQUIRES: asserts
 
-class Box { // expected-note 3{{class 'Box' does not conform to the 'Sendable' protocol}}
+class Box {
     let size : Int = 0
 }
 
@@ -48,7 +48,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-warning {{cannot form key path to actor-isolated property 'letBox'; this is an error in Swift 6}}
                                        \Box?.?.size)
 
     _ = (\Door.varBox).appending(path:  // expected-error {{cannot form key path to actor-isolated property 'varBox'}}
@@ -62,9 +62,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-warning {{cannot form key path to actor-isolated property 'letDict'; this is an error in Swift 6}}
     _ = \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-warning {{cannot form key path to actor-isolated property 'letBox'; this is an error in Swift 6}}
 }
 
 func tryKeypaths() {

test/Concurrency/actor_keypath_isolation_swift6.swift

@@ -3,7 +3,7 @@
 
 // REQUIRES: concurrency && asserts
 
-class Box { // expected-note 3{{class 'Box' does not conform to the 'Sendable' protocol}}
+class Box {
     let size : Int = 0
 }
 
@@ -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

@@ -86,7 +86,9 @@ extension A1 {
     _ = await self.asynchronous(nil)
 
     // Across to a different actor, so Sendable restriction is enforced.
-    _ = other.localLet // expected-warning{{non-sendable type 'NotConcurrent' in asynchronous access to actor-isolated property 'localLet' cannot cross actor boundary}}
+    _ = other.localLet // expected-warning{{non-sendable type 'NotConcurrent' in implicitly asynchronous access to actor-isolated property 'localLet' cannot cross actor boundary}}
+    // expected-warning@-1 {{expression is 'async' but is not marked with 'await'}}
+    // expected-note@-2 {{property access is 'async'}}
     _ = await other.synchronous() // expected-warning{{non-sendable type 'NotConcurrent?' returned by call to actor-isolated function cannot cross actor boundary}}
     _ = await other.asynchronous(nil) // expected-complete-warning{{passing argument of non-sendable type 'NotConcurrent?' into actor-isolated context may introduce data races}}
   }
@@ -230,7 +232,7 @@ func testKeyPaths(dict: [NC: Int], nc: NC) {
 // Sendable restriction on nonisolated declarations.
 // ----------------------------------------------------------------------
 actor ANI {
-  nonisolated let nc = NC()
+  nonisolated let nc = NC() // expected-warning {{'nonisolated' can not be applied to variable with non-'Sendable' type 'NC'; this is an error in Swift 6}}
   nonisolated func f() -> NC? { nil }
 }
 

test/Concurrency/flow_isolation.swift

@@ -140,19 +140,22 @@ actor ExampleFromProposal {
   let immutableSendable = SendableType()
   var mutableSendable = SendableType()
   let nonSendable = NonSendableType()
+  nonisolated(unsafe) let unsafeNonSendable = NonSendableType()
   var nsItems: [NonSendableType] = []
   var sItems: [SendableType] = []
 
   init() {
     _ = self.immutableSendable  // ok
     _ = self.mutableSendable    // ok
     _ = self.nonSendable        // ok
+    _ = self.unsafeNonSendable
 
     f() // expected-note 2 {{after calling instance method 'f()', only non-isolated properties of 'self' can be accessed from this init}}
 
     _ = self.immutableSendable  // ok
     _ = self.mutableSendable    // expected-warning {{cannot access property 'mutableSendable' here in non-isolated initializer; this is an error in Swift 6}}
     _ = self.nonSendable        // expected-warning {{cannot access property 'nonSendable' here in non-isolated initializer; this is an error in Swift 6}}
+    _ = self.unsafeNonSendable // ok
   }