[Distributed] introduce static resolve func, and fix static isolation on dist actors (swiftlang#38530)

Author: ktoso

include/swift/AST/DiagnosticsSema.def

@@ -4436,8 +4436,8 @@ NOTE(actor_isolated_sync_func,none,
      "implicitly asynchronous",
      (DescriptiveDeclKind, DeclName))
 NOTE(distributed_actor_isolated_method_note,none,
-      "only 'distributed' functions can be called from outside the distributed actor",
-      ())
+     "only 'distributed' functions can be called from outside the distributed actor", // TODO: improve error message
+     ())
 ERROR(distributed_actor_isolated_method,none,
       "only 'distributed' functions can be called from outside the distributed actor", // TODO: improve error message to be more like 'non-distributed' ... defined here
       ())

include/swift/Runtime/Concurrency.h

@@ -579,9 +579,19 @@ SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
 void swift_defaultActor_deallocateResilient(HeapObject *actor);
 
 /// Initialize the runtime storage for a distributed remote actor.
+// TODO: this may end up being removed as we move to the "proxy creation" below
 SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
 void swift_distributedActor_remote_initialize(DefaultActor *actor);
 
+/// Create a proxy object that will serve as remote distributed actor instance.
+SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
+OpaqueValue* swift_distributedActor_remote_create(
+  /* +1 */OpaqueValue *identity,
+  /* +1 */OpaqueValue *transport
+  // metadata for identity
+  // metadata for transport
+);
+
 /// Destroy the runtime storage for a default actor.
 SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
 void swift_distributedActor_destroy(DefaultActor *actor);

include/swift/Runtime/RuntimeFunctions.def

@@ -1671,6 +1671,16 @@ FUNCTION(DistributedActorInitializeRemote,
          ARGS(RefCountedPtrTy), // TODO also address and transport?
          ATTRS(NoUnwind))
 
+// OpaqueValue* swift_distributedActor_remote_create(
+//     OpaqueValue *identity,
+//     OpaqueValue *transport);
+FUNCTION(DistributedActorRemoteCreate,
+         swift_distributedActor_remote_create, SwiftCC,
+         ConcurrencyAvailability,
+         RETURNS(OpaquePtrTy), // TODO: is this the right type here?
+         ARGS(OpaquePtrTy, OpaquePtrTy),
+         ATTRS(NoUnwind))
+
 // void swift_distributedActor_destroy(DefaultActor *actor); // TODO: ProxyActor *proxy?
 FUNCTION(DistributedActorDestroy,
          swift_distributedActor_destroy, SwiftCC,

lib/Sema/CodeSynthesisDistributedActor.cpp

@@ -231,6 +231,7 @@ createDistributedActor_init_local(ClassDecl *classDecl,
 }
 
 // ==== Distributed Actor: Resolve Initializer ---------------------------------
+// TODO: remove resolve initializer in favor of resolve static function
 
 /// Synthesizes the body for
 ///

lib/Sema/TypeCheckConcurrency.cpp

@@ -385,6 +385,7 @@ ActorIsolationRestriction ActorIsolationRestriction::forDeclaration(
       if (func->isAsyncContext())
         isAccessibleAcrossActors = true;
 
+      // FIXME: move diagnosis out of this function entirely (!)
       if (func->isDistributed()) {
         if (auto classDecl = dyn_cast<ClassDecl>(decl->getDeclContext())) {
           if (!classDecl->isDistributedActor()) {
@@ -2273,9 +2274,11 @@ namespace {
         if (dyn_cast<ConstructorDecl>(member))
           return false;
 
-        // While the member may be unrestricted, perhaps
+        // While the member may be unrestricted, perhaps it is in a
+        // distributed actor, in which case we need to diagnose it.
         if (auto classDecl = dyn_cast<ClassDecl>(member->getDeclContext())) {
           if (classDecl->isDistributedActor()) {
+            fprintf(stderr, "[%s:%d] (%s) HERE\n", __FILE__, __LINE__, __FUNCTION__);
             ctx.Diags.diagnose(memberLoc, diag::distributed_actor_isolated_method);
             noteIsolatedActorMember(member, context);
             return true;
@@ -2312,32 +2315,43 @@ namespace {
             !(isolatedActor.isActorSelf() &&
               member->isInstanceMember() &&
               isActorInitOrDeInitContext(getDeclContext()))) {
-          // invocation on not-'self', is only okey if this is a distributed func
+          // cross actor invocation is only ok for a distributed or static func
           if (auto func = dyn_cast<FuncDecl>(member)) {
-            if (!func->isDistributed()) {
+            if (func->isStatic()) {
+              // FIXME: rather, never end up as distributed actor self isolated
+              //        at all for static funcs
+              continueToCheckingLocalIsolation = true;
+            } else if (func->isDistributed()) {
+              tryMarkImplicitlyAsync(memberLoc, memberRef, context);
+              tryMarkImplicitlyThrows(memberLoc, memberRef, context);
+
+              // distributed func reference, that passes all checks, great!
+              continueToCheckingLocalIsolation = true;
+            } else {
+              // the func is neither static or distributed
               ctx.Diags.diagnose(memberLoc, diag::distributed_actor_isolated_method);
               // TODO: offer a fixit to add 'distributed' on the member; how to test fixits? See also https://github.com/apple/swift/pull/35930/files
               noteIsolatedActorMember(member, context);
               return true;
             }
 
-            assert(func->isDistributed());
-            tryMarkImplicitlyAsync(memberLoc, memberRef, context);
-            tryMarkImplicitlyThrows(memberLoc, memberRef, context);
-
-            // distributed func reference, that passes all checks, great!
-            continueToCheckingLocalIsolation = true;
           } // end FuncDecl
 
           if (!continueToCheckingLocalIsolation) {
             // it wasn't a function (including a distributed function),
             // so we need to perform some more checks
             if (auto var = dyn_cast<VarDecl>(member)) {
+              // TODO: we want to remove _distributedActorIndependent in favor of nonisolated
+              //
               // @_distributedActorIndependent decls are accessible always,
               // regardless of distributed actor-isolation; e.g. actorAddress
               if (member->getAttrs().hasAttribute<DistributedActorIndependentAttr>())
                 return false;
 
+              // nonisolated decls are accessible always
+              if (member->getAttrs().hasAttribute<DistributedActorIndependentAttr>())
+                return false;
+
               // otherwise, no other properties are accessible on a distributed actor
               if (!continueToCheckingLocalIsolation) {
                 ctx.Diags.diagnose(
@@ -2347,13 +2361,16 @@ namespace {
                 noteIsolatedActorMember(member, context);
                 return true;
               }
-            }
+            } // end VarDecl
 
             // TODO: would have to also consider subscripts and other things
           }
         } // end !isolatedActor
 
-        return false;
+        if (!continueToCheckingLocalIsolation)
+          return false;
+
+        LLVM_FALLTHROUGH;
       }
 
       case ActorIsolationRestriction::ActorSelf: {
@@ -2428,9 +2445,10 @@ namespace {
       case ActorIsolationRestriction::Unsafe:
         // This case is hit when passing actor state inout to functions in some
         // cases. The error is emitted by diagnoseInOutArg.
-
-        if (auto classDecl = dyn_cast<ClassDecl>(member->getDeclContext())) {
-          if (classDecl->isDistributedActor()) {
+        auto classDecl = dyn_cast<ClassDecl>(member->getDeclContext());
+        if (classDecl && classDecl->isDistributedActor()) {
+          auto funcDecl = dyn_cast<AbstractFunctionDecl>(member);
+          if (funcDecl && !funcDecl->isStatic()) {
             member->diagnose(diag::distributed_actor_isolated_method);
             return true;
           }
@@ -3164,7 +3182,8 @@ ActorIsolation ActorIsolationRequest::evaluate(
     }
 
     if (auto nominal = value->getDeclContext()->getSelfNominalTypeDecl()) {
-      if (nominal->isDistributedActor()) {
+      /// Unless the function is static, it is isolated to the dist actor
+      if (nominal->isDistributedActor() && !func->isStatic()) {
         defaultIsolation = ActorIsolation::forDistributedActorInstance(nominal);
       }
     }

stdlib/public/Concurrency/Actor.cpp

@@ -1756,13 +1756,19 @@ void swift::swift_defaultActor_deallocateResilient(HeapObject *actor) {
                       metadata->getInstanceAlignMask());
 }
 
-// TODO: most likely where we'd need to create the "proxy instance" instead?
-void swift::swift_distributedActor_remote_initialize(DefaultActor *_actor) { // FIXME: !!!!! remove distributed C++ impl not needed?
+// TODO: most likely where we'd need to create the "proxy instance" instead? (most likely remove this and use swift_distributedActor_remote_create instead)
+void swift::swift_distributedActor_remote_initialize(DefaultActor *_actor) { // FIXME: remove distributed C++ impl not needed?
   auto actor = asImpl(_actor);
   actor->initialize(/*remote=*/true);
 }
 
-void swift::swift_distributedActor_destroy(DefaultActor *_actor) { // FIXME: !!!!! remove distributed C++ impl not needed?
+// TODO: missing implementation of creating a proxy for the remote actor
+OpaqueValue* swift::swift_distributedActor_remote_create(OpaqueValue *identity,
+                                                         OpaqueValue *transport) {
+  assert(false && "swift_distributedActor_remote_create is not implemented yet!");
+}
+
+void swift::swift_distributedActor_destroy(DefaultActor *_actor) { // FIXME: remove distributed C++ impl not needed?
   // TODO: need to resign the address before we destroy:
   //       something like: actor.transport.resignIdentity(actor.address)
 

stdlib/public/Distributed/ActorTransport.swift

@@ -48,7 +48,7 @@ public protocol ActorTransport: Sendable {
   ///
   /// Detecting liveness of such remote actors shall be offered / by transport libraries
   /// by other means, such as "watching an actor for termination" or similar.
-  func resolve<Act>(_ identity: Act.ID, as actorType: Act.Type) throws -> ActorResolved<Act>
+  func resolve<Act>(_ identity: AnyActorIdentity, as actorType: Act.Type) throws -> ActorResolved<Act>
       where Act: DistributedActor
 
   // ==== ---------------------------------------------------------------------

stdlib/public/Distributed/DistributedActor.swift

@@ -46,20 +46,23 @@ public protocol DistributedActor: AnyActor, Identifiable, Hashable, Codable {
     ///   associated with.
     init(transport: ActorTransport)
 
-    /// Resolves the passed in `address` against the `transport`,
-    /// returning either a local or remote actor reference.
+    @available(*, deprecated, renamed: "SomeDistributedActor.resolve(_:using:)")
+    init(resolve id: AnyActorIdentity, using transport: ActorTransport) throws
+
+    /// Resolves the passed in `identity` against the `transport`, returning
+    /// either a local or remote actor reference.
     ///
-    /// The transport will be asked to `resolve` the address and return either
-    /// a local instance or determine that a proxy instance should be created
-    /// for this address. A proxy actor will forward all invocations through
+    /// The transport will be asked to `resolve` the identity and return either
+    /// a local instance or request a proxy to be created for this identity.
+    ///
+    /// A remote distributed actor reference will forward all invocations through
     /// the transport, allowing it to take over the remote messaging with the
     /// remote actor instance.
     ///
-    /// - Parameter address: the address to resolve, and produce an instance or proxy for.
-    /// - Parameter transport: transport which should be used to resolve the `address`.
-    // FIXME: replace with static func resolve(_:using)
-    // TODO: make this <ID>(resolve id: ID, ...)
-    init(resolve id: AnyActorIdentity, using transport: ActorTransport) throws
+    /// - Parameter identity: identity uniquely identifying a, potentially remote, actor in the system
+    /// - Parameter transport: `transport` which should be used to resolve the `identity`, and be associated with the returned actor
+    static func resolve<Identity>(_ identity: Identity, using transport: ActorTransport)
+      throws -> Self where Identity: ActorIdentity
 
     /// The `ActorTransport` associated with this actor.
     /// It is immutable and equal to the transport passed in the local/resolve
@@ -83,6 +86,23 @@ public protocol DistributedActor: AnyActor, Identifiable, Hashable, Codable {
     nonisolated var id: AnyActorIdentity { get }
 }
 
+@available(SwiftStdlib 5.5, *)
+extension DistributedActor {
+
+  public static func resolve<Identity>(_ identity: Identity, using transport: ActorTransport)
+      throws -> Self where Identity: ActorIdentity {
+    switch try transport.resolve(AnyActorIdentity(identity), as: Self.self) {
+    case .resolved(let instance):
+      return instance
+
+    case .makeProxy:
+      // FIXME: this needs actual implementation of distributedActorRemoteCreate
+      let remote: Any = distributedActorRemoteCreate(identity: identity, transport: transport)
+      return remote as! Self
+    }
+  }
+}
+
 // ==== Hashable conformance ---------------------------------------------------
 
 @available(SwiftStdlib 5.5, *)
@@ -220,6 +240,9 @@ func __isLocalActor(_ actor: AnyObject) -> Bool {
 @_silgen_name("swift_distributedActor_remote_initialize")
 func _distributedActorRemoteInitialize(_ actor: AnyObject)
 
+@_silgen_name("swift_distributedActor_remote_create")
+func distributedActorRemoteCreate(identity: Any, transport: Any) -> Any // TODO: make it typed
+
 /// Called to destroy the default actor instance in an actor.
 /// The implementation will call this within the actor's deinit.
 ///

test/Concurrency/actor_isolation.swift

@@ -853,6 +853,29 @@ actor MyActorP: P {
   func h() { g() }
 }
 
+@available(SwiftStdlib 5.5, *)
+protocol SP {
+  static func s()
+}
+
+@available(SwiftStdlib 5.5, *)
+actor ASP: SP {
+  static func s() { }
+}
+
+@available(SwiftStdlib 5.5, *)
+protocol SPD {
+  static func sd()
+}
+@available(SwiftStdlib 5.5, *)
+extension SPD {
+  static func sd() { }
+}
+
+@available(SwiftStdlib 5.5, *)
+actor ASPD: SPD {
+}
+
 @available(SwiftStdlib 5.5, *)
 func testCrossActorProtocol<T: P>(t: T) async {
   await t.f()
@@ -863,6 +886,8 @@ func testCrossActorProtocol<T: P>(t: T) async {
   t.g()
   // expected-error@-1{{expression is 'async' but is not marked with 'await'}}{{3-3=await }}
   // expected-note@-2{{calls to instance method 'g()' from outside of its actor context are implicitly asynchronous}}
+  ASP.s()
+  ASPD.sd()
 }
 
 // ----------------------------------------------------------------------

test/Distributed/distributed_actor_initialization.swift

@@ -13,7 +13,7 @@ distributed actor OK1 {
   // ok, since all fields are initialized, the constructors can be synthesized
 }
 
-// TODO: test all the FIXITs in this file (!!!)
+// TODO: test all the FIXITs in this file
 
 @available(SwiftStdlib 5.5, *)
 distributed actor Bad1 {

test/Distributed/distributed_actor_isolation.swift

@@ -89,6 +89,17 @@ distributed actor DistributedActor_1 {
     fatalError()
   }
 
+  static func staticFunc() -> String { "" } // ok
+
+// TODO: should be able to handle a static, global actor isolated function as well
+//  @MainActor
+//  static func staticMainActorFunc() -> String { "" } // ok
+
+  static distributed func staticDistributedFunc() -> String {
+    // expected-error@-1{{'distributed' functions cannot be 'static'}}{10-21=}
+    fatalError()
+  }
+
   func test_inside() async throws {
     _ = self.name
     _ = self.computedMutable
@@ -126,12 +137,31 @@ func test_outside(
   _ = distributed.id
   _ = distributed.actorTransport
 
+  // ==== static functions
+  _ = distributed.staticFunc() // expected-error{{static member 'staticFunc' cannot be used on instance of type 'DistributedActor_1'}}
+  _ = DistributedActor_1.staticFunc()
+
   // ==== non-distributed functions
   _ = await 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}}
 }
 
+// ==== Protocols and static (non isolated functions)
+
+@available(SwiftStdlib 5.5, *)
+protocol P {
+  static func hello() -> String
+}
+@available(SwiftStdlib 5.5, *)
+extension P {
+  static func hello() -> String { "" }
+}
+
+@available(SwiftStdlib 5.5, *)
+distributed actor ALL: P {
+}
+
 // ==== Codable parameters and return types ------------------------------------
 
 @available(SwiftStdlib 5.5, *)

test/Distributed/distributed_actor_resolve.swift

@@ -0,0 +1,23 @@
+// RUN: %target-typecheck-verify-swift -enable-experimental-distributed
+// REQUIRES: concurrency
+// REQUIRES: distributed
+
+import _Distributed
+
+@available(SwiftStdlib 5.5, *)
+distributed actor Capybara { }
+
+//@available(SwiftStdlib 5.5, *)
+//protocol Wheeker: DistributedActor { }
+//@available(SwiftStdlib 5.5, *)
+//distributed actor GuineaPing: Wheeker { }
+
+@available(SwiftStdlib 5.5, *)
+func test<Identity: ActorIdentity>(identity: Identity, transport: ActorTransport) async throws {
+  let _: Capybara = try Capybara.resolve(identity, using: transport)
+
+// TODO: implement resolve being able to be called on a distributed actor protocol
+//       (yes, normally it is not allowed to call such function... so we need to
+//        discuss and figure out how we want to expose the resolve of a protocol)
+//  let c: Wheeker = try Wheeker.resolve(.init(identity), using: transport)
+}