Merge pull request #69267 from hborla/stored-property-sendable-violation

[Concurrency] Close a hole that allowed non-`Sendable` types to cross isolation boundaries in initializers.

Author: hborla

lib/Sema/TypeCheckConcurrency.cpp

@@ -5838,6 +5838,27 @@ ActorReferenceResult ActorReferenceResult::forReference(
     return forSameConcurrencyDomain(declIsolation);
   }
 
+  // Initializing an actor isolated stored property with a value effectively
+  // passes that value from the init context into the actor isolated context.
+  // It's only okay for the value to cross isolation boundaries if the property
+  // type is Sendable. Note that if the init is a nonisolated actor init,
+  // Sendable checking is already performed on arguments at the call-site.
+  if ((declIsolation.isActorIsolated() && contextIsolation.isGlobalActor()) ||
+      declIsolation.isGlobalActor()) {
+    auto *init = dyn_cast<ConstructorDecl>(fromDC);
+    auto *decl = declRef.getDecl();
+    if (init && init->isDesignatedInit() && isStoredProperty(decl)) {
+      auto type =
+          fromDC->mapTypeIntoContext(declRef.getDecl()->getInterfaceType());
+      if (!isSendableType(fromDC->getParentModule(), type)) {
+        // Treat the decl isolation as 'preconcurrency' to downgrade violations
+        // to warnings, because violating Sendable here is accepted by the
+        // Swift 5.9 compiler.
+        return forEntersActor(declIsolation, Flags::Preconcurrency);
+      }
+    }
+  }
+
   // If there is an instance and it is checked by flow isolation, treat it
   // as being in the same concurrency domain.
   if (actorInstance &&
@@ -5856,13 +5877,8 @@ ActorReferenceResult ActorReferenceResult::forReference(
 
   // If there is an instance that corresponds to 'self',
   // we are in a constructor or destructor, and we have a stored property of
-  // global-actor-qualified type, pretend we are in the same concurrency
-  // domain.
-  // FIXME: This is an odd carve-out that probably shouldn't have been allowed.
-  // It should at the very least be diagnosed, and either subsumed by flow
-  // isolation or banned outright.
-  // FIXME: At the very least, we should consistently use
-  // isActorInitOrDeInitContext here, but it only wants to think about actors.
+  // global-actor-qualified type, then we have problems if the stored property
+  // type is non-Sendable. Note that if we get here, the type must be Sendable.
   if (actorInstance && actorInstance->isSelf() &&
       isNonInheritedStorage(declRef.getDecl(), fromDC) &&
       declIsolation.isGlobalActor() &&

test/Concurrency/actor_isolation.swift

@@ -824,6 +824,7 @@ actor SomeActorWithInits {
   // expected-note@+1 3 {{mutation of this property is only permitted within the actor}}
   var mutableState: Int = 17
   var otherMutableState: Int
+  // expected-note@+1 {{mutation of this property is only permitted within the actor}}
   let nonSendable: SomeClass
 
   // Sema should not complain about referencing non-sendable members
@@ -887,7 +888,7 @@ actor SomeActorWithInits {
   @MainActor init(i5 x: SomeClass) {
     self.mutableState = 42
     self.otherMutableState = 17
-    self.nonSendable = x
+    self.nonSendable = x // expected-warning {{actor-isolated property 'nonSendable' can not be mutated from the main actor; this is an error in Swift 6}}
 
     self.isolated() // expected-warning{{actor-isolated instance method 'isolated()' can not be referenced from the main actor; this is an error in Swift 6}}
     self.nonisolated()

test/Concurrency/global_actor_inference.swift

@@ -324,17 +324,30 @@ struct WrapperOnActor<Wrapped: Sendable> {
 public struct WrapperOnMainActor<Wrapped> {
   // Make sure inference of @MainActor on wrappedValue doesn't crash.
 
+  // expected-note@+1 {{mutation of this property is only permitted within the actor}}
   public var wrappedValue: Wrapped
 
   public var accessCount: Int
 
   nonisolated public init(wrappedValue: Wrapped) {
+    // expected-warning@+1 {{main actor-isolated property 'wrappedValue' can not be mutated from a non-isolated context; this is an error in Swift 6}}
     self.wrappedValue = wrappedValue
   }
 }
 
 @propertyWrapper
-public struct WrapperOnMainActor2<Wrapped> {
+public struct WrapperOnMainActorNonSendable<Wrapped> {
+  // expected-note@+1 {{mutation of this property is only permitted within the actor}}
+  @MainActor public var wrappedValue: Wrapped
+
+  public init(wrappedValue: Wrapped) {
+    // expected-warning@+1 {{main actor-isolated property 'wrappedValue' can not be mutated from a non-isolated context; this is an error in Swift 6}}
+    self.wrappedValue = wrappedValue
+  }
+}
+
+@propertyWrapper
+public struct WrapperOnMainActorSendable<Wrapped: Sendable> {
   @MainActor public var wrappedValue: Wrapped
 
   public init(wrappedValue: Wrapped) {

test/Concurrency/global_actor_inference_swift6.swift

@@ -69,11 +69,13 @@ struct WrapperOnActor<Wrapped: Sendable> {
 public struct WrapperOnMainActor<Wrapped> {
   // Make sure inference of @MainActor on wrappedValue doesn't crash.
 
+  // expected-note@+1 {{mutation of this property is only permitted within the actor}}
   public var wrappedValue: Wrapped // expected-note {{property declared here}}
 
   public var accessCount: Int
 
   nonisolated public init(wrappedValue: Wrapped) {
+    // expected-error@+1 {{main actor-isolated property 'wrappedValue' can not be mutated from a non-isolated context}}
     self.wrappedValue = wrappedValue
   }
 }