[Distributed] Implement isolation check aware of Task/Task.detached etc

Author: ktoso

include/swift/AST/DiagnosticsSema.def

@@ -4329,6 +4329,12 @@ NOTE(note_add_async_to_function,none,
 NOTE(note_add_nonisolated_to_decl,none,
      "add 'nonisolated' to %0 to make this %1 not isolated to the actor",
      (DeclName, DescriptiveDeclKind))
+NOTE(note_add_distributed_to_decl,none,
+     "add 'distributed' to %0 to make this %1 witness the protocol requirement",
+     (DeclName, DescriptiveDeclKind))
+NOTE(note_distributed_requirement_defined_here,none,
+     "distributed function requirement %0 declared here",
+     (DeclName))
 NOTE(note_add_globalactor_to_function,none,
      "add '@%0' to make %1 %2 part of global actor %3", 
      (StringRef, DescriptiveDeclKind, DeclName, Type))
@@ -4398,8 +4404,8 @@ ERROR(actor_isolated_non_self_reference,none,
         "from the main actor|from a non-isolated context}3",
         (DescriptiveDeclKind, DeclName, unsigned, unsigned, Type))
 ERROR(distributed_actor_isolated_non_self_reference,none,
-      "distributed actor-isolated %0 %1 can only be referenced "
-      "inside the distributed actor",
+      "distributed actor-isolated %0 %1 can only be referenced inside the "
+      "distributed actor",
       (DescriptiveDeclKind, DeclName))
 ERROR(distributed_actor_needs_explicit_distributed_import,none,
       "'_Distributed' module not imported, required for 'distributed actor'",

lib/Sema/TypeCheckConcurrency.cpp

@@ -118,7 +118,7 @@ class ActorIsolationRestriction {
 
     /// References to declarations that are part of a distributed actor are
     /// only permitted if they are async.
-    DistributedActorSelf,
+    CrossDistributedActorSelf, // FIXME(distributed): remove this case entirely rdar://83713366
   };
 
 private:
@@ -150,7 +150,7 @@ class ActorIsolationRestriction {
   NominalTypeDecl *getActorType() const {
     assert(kind == ActorSelf || 
            kind == CrossActorSelf || 
-           kind == DistributedActorSelf);
+           kind == CrossDistributedActorSelf);
     return data.actorType;
   }
 
@@ -174,6 +174,7 @@ class ActorIsolationRestriction {
   /// the current actor or is a cross-actor access.
   static ActorIsolationRestriction forActorSelf(
       NominalTypeDecl *actor, bool isCrossActor) {
+
     ActorIsolationRestriction result(isCrossActor? CrossActorSelf : ActorSelf,
                                      isCrossActor);
     result.data.actorType = actor;
@@ -184,7 +185,8 @@ class ActorIsolationRestriction {
   /// the current actor.
   static ActorIsolationRestriction forDistributedActorSelf(
       NominalTypeDecl *actor, bool isCrossActor) {
-    ActorIsolationRestriction result(DistributedActorSelf, isCrossActor);
+    ActorIsolationRestriction result(isCrossActor ? CrossActorSelf : ActorSelf,
+                                     isCrossActor);
     result.data.actorType = actor;
     return result;
   }

lib/Sema/TypeCheckConcurrency.h

@@ -455,7 +455,7 @@ static bool checkObjCActorIsolation(const ValueDecl *VD,
   case ActorIsolationRestriction::GlobalActor:
     // FIXME: Consider whether to limit @objc on global-actor-qualified
     // declarations.
-  case ActorIsolationRestriction::DistributedActorSelf:
+  case ActorIsolationRestriction::CrossDistributedActorSelf:
     // we do not allow distributed + objc actors.
     return false;
   case ActorIsolationRestriction::Unrestricted:

lib/Sema/TypeCheckDeclObjC.cpp

@@ -2824,24 +2824,6 @@ bool ConformanceChecker::checkActorIsolation(
               ActorIsolationRestriction::forDeclaration(
                   witness, witness->getDeclContext(),
                   /*fromExpression=*/false)) {
-  case ActorIsolationRestriction::DistributedActorSelf: {
-    if (witness->isSynthesized()) {
-      // we only have two 'distributed-actor-nonisolated' properties,
-      // the address and transport; if we see any such marked property,
-      // we're free to automatically assume those are fine and accessible always.
-      if (witness->isDistributedActorIndependent())
-        return false;
-    }
-
-    if (auto funcDecl = dyn_cast<AbstractFunctionDecl>(witness)) {
-      // A 'distributed func' may witness a distributed isolated function requirement.
-      if (funcDecl->isDistributed())
-        return false;
-    }
-
-    // continue checking ActorSelf rules
-    LLVM_FALLTHROUGH;
-  }
   case ActorIsolationRestriction::ActorSelf: {
     auto requirementIsolation = getActorIsolation(requirement);
 
@@ -2850,23 +2832,60 @@ bool ConformanceChecker::checkActorIsolation(
     if (requirementIsolation == ActorIsolation::ActorInstance)
       return false;
 
+    auto witnessFunc = dyn_cast<AbstractFunctionDecl>(witness);
+    auto requirementFunc = dyn_cast<AbstractFunctionDecl>(requirement);
+
+    /// Distributed actors can witness protocol requirements either with
+    /// nonisolated or distributed members.
+    auto witnessClass = dyn_cast<ClassDecl>(witness->getDeclContext());
+    if (witnessClass && witnessClass->isDistributedActor()) {
+        // we only have two 'distributed-actor-nonisolated' properties,
+        // the address and transport; if we see any such marked property,
+        // we're free to automatically assume those are fine and accessible always.
+      if (witness->isSynthesized() && witness->isDistributedActorIndependent()) {
+        return false;
+      }
+
+      // Maybe we're dealing with a 'distributed func' which is witness to
+      // a distributed function requirement, this is ok.
+      if (requirementFunc && requirementFunc->isDistributed() &&
+          witnessFunc && witnessFunc->isDistributed()) {
+          return false;
+      }
+    }
+
     witness->diagnose(diag::actor_isolated_witness,
                       witness->getDescriptiveKind(),
                       witness->getName());
     {
       auto witnessVar = dyn_cast<VarDecl>(witness);
       if ((witnessVar && !witnessVar->hasStorage()) || !witnessVar) {
-        auto independentNote = witness->diagnose(
-            diag::note_add_nonisolated_to_decl,
-            witness->getName(), witness->getDescriptiveKind());
-        independentNote.fixItInsert(witness->getAttributeInsertionLoc(true),
-            "nonisolated ");
+        if (requirementFunc && requirementFunc->isDistributed()) {
+          // a distributed protocol requirement can be witnessed with a
+          // distributed function:
+          witness
+              ->diagnose(diag::note_add_distributed_to_decl,
+                         witness->getName(), witness->getDescriptiveKind())
+              .fixItInsert(witness->getAttributeInsertionLoc(true),
+                           "distributed ");
+          requirement
+              ->diagnose(diag::note_distributed_requirement_defined_here,
+                         requirement->getName());
+        } else {
+          // the only other way to witness is to use a nonisolated member:
+          witness
+              ->diagnose(diag::note_add_nonisolated_to_decl, witness->getName(),
+                         witness->getDescriptiveKind())
+              .fixItInsert(witness->getAttributeInsertionLoc(true),
+                           "nonisolated ");
+        }
       }
     }
 
     return true;
   }
 
+  case ActorIsolationRestriction::CrossDistributedActorSelf:
   case ActorIsolationRestriction::CrossActorSelf:
     return diagnoseNonSendableTypesInReference(
         witness, DC->getParentModule(), witness->getLoc(),

lib/Sema/TypeCheckProtocol.cpp

@@ -142,7 +142,7 @@ func test_outside(
   _ = DistributedActor_1.staticFunc()
 
   // ==== non-distributed functions
-  _ = await distributed.hello() // expected-error{{only 'distributed' functions can be called from outside the distributed actor}}
+  distributed.hello() // expected-error{{only 'distributed' functions can be called from outside the distributed actor}}
   _ = await distributed.helloAsync() // expected-error{{only 'distributed' functions can be called from outside the distributed actor}}
   _ = try await distributed.helloAsyncThrows() // expected-error{{only 'distributed' functions can be called from outside the distributed actor}}
 }

test/Distributed/distributed_actor_isolation.swift

@@ -0,0 +1,65 @@
+// RUN: %target-typecheck-verify-swift -enable-experimental-distributed -disable-availability-checking
+// REQUIRES: concurrency
+// REQUIRES: distributed
+
+import _Distributed
+
+struct SomeLogger {}
+struct Logger {
+  let label: String
+  func info(_: String) {}
+}
+
+distributed actor Philosopher {
+  let log: Logger
+  // expected-note@-1{{distributed actor state is only available within the actor instance}}
+  var variable = 12
+  var variable_fromDetach = 12 // expected-note{{distributed actor state is only available within the actor instance}}
+  let INITIALIZED: Int
+  let outside: Int = 1
+
+  init(transport: ActorTransport) {
+    self.log = Logger(label: "name")
+    self.INITIALIZED = 1
+  }
+
+  distributed func dist() -> Int {}
+
+  func test() {
+    _ = self.id
+    _ = self.actorTransport
+    Task {
+      _ = self.id
+      _ = self.actorTransport
+
+      self.log.info("READY!")
+      _ = self.variable
+      _ = self.dist()
+    }
+
+    Task.detached {
+      _ = self.id
+      _ = self.actorTransport
+
+      // This is an interesting case, since we have a real local `self` and
+      // yet are not isolated to the same actor in this detached task...
+      // the call to it is implicitly async, however it is NOT implicitly throwing
+      // because we KNOW this is a local call -- and there is no transport in
+      // between that will throw.
+      _ = await self.dist() // notice lack of 'try' even though 'distributed func'
+      _ = self.variable_fromDetach // expected-error{{distributed actor-isolated property 'variable_fromDetach' can only be referenced inside the distributed actor}}
+    }
+  }
+}
+
+func test_outside(transport: ActorTransport) async throws {
+  _ = try await Philosopher(transport: transport).dist()
+  _ = Philosopher(transport: transport).log // expected-error{{distributed actor-isolated property 'log' can only be referenced inside the distributed actor}}
+
+  _ = Philosopher(transport: transport).id
+  _ = Philosopher(transport: transport).actorTransport
+}
+
+func test_outside_isolated(phil: isolated Philosopher) async throws {
+  phil.log.info("works on isolated")
+}

test/Distributed/distributed_actor_isolation_and_tasks.swift

@@ -1,15 +1,13 @@
-// RUN: %target-typecheck-verify-swift -enable-experimental-distributed -verify-ignore-unknown
+// RUN: %target-typecheck-verify-swift -enable-experimental-distributed -disable-availability-checking -verify-ignore-unknown
 // REQUIRES: concurrency
 // REQUIRES: distributed
 
 import _Distributed
 
-@available(SwiftStdlib 5.5, *)
 distributed actor D1 {
   var x: Int = 17
 }
 
-@available(SwiftStdlib 5.5, *)
 distributed actor D2 {
   // expected-error@-1{{actor 'D2' has no initializers}}
   let actorTransport: String
@@ -18,25 +16,32 @@ distributed actor D2 {
   // expected-note@-3{{stored property 'actorTransport' without initial value prevents synthesized initializers}}
 }
 
-@available(SwiftStdlib 5.5, *)
 distributed actor D3 {
   var id: Int { 0 }
   // expected-error@-1{{property 'id' cannot be defined explicitly, as it conflicts with distributed actor synthesized stored property}}
   // expected-error@-2{{invalid redeclaration of synthesized implementation for protocol requirement 'id'}}
 }
 
-@available(SwiftStdlib 5.5, *)
 distributed actor D4 {
   // expected-error@-1{{actor 'D4' has no initializers}}
   let actorTransport: String
   // expected-error@-1{{invalid redeclaration of synthesized implementation for protocol requirement 'actorTransport'}}
   // expected-error@-2{{property 'actorTransport' cannot be defined explicitly, as it conflicts with distributed actor synthesized stored property}}
   // expected-note@-3{{stored property 'actorTransport' without initial value prevents synthesized initializers}}
   let id: AnyActorIdentity
-  // expected-error@-1{{actor-isolated property 'id' cannot be used to satisfy a protocol requirement}}
-  // expected-error@-2{{property 'id' cannot be defined explicitly, as it conflicts with distributed actor synthesized stored property}}
-  // expected-error@-3{{actor-isolated property 'id' cannot be used to satisfy a protocol requirement}}
-  // expected-note@-4{{stored property 'id' without initial value prevents synthesized initializers}}
+  // expected-error@-1{{property 'id' cannot be defined explicitly, as it conflicts with distributed actor synthesized stored property}}
+  // expected-note@-2{{stored property 'id' without initial value prevents synthesized initializers}}
+}
+
+protocol P1: DistributedActor {
+  distributed func dist() -> String
+  // expected-note@-1{{distributed function requirement 'dist()' declared here}}
+}
+
+distributed actor D5: P1 {
+  func dist() -> String { "" }
+  // expected-error@-1{{actor-isolated instance method 'dist()' cannot be used to satisfy a protocol requirement}}
+  // expected-note@-2{{add 'distributed' to 'dist()' to make this instance method witness the protocol requirement}}{{3-3=distributed }}
 }
 
 // ==== Tests ------------------------------------------------------------------