[5.8][Distributed] correct take semantics for synthesized ID assignments

lib/SILGen/SILGenDistributed.cpp

@@ -54,18 +54,19 @@ static SILValue emitActorPropertyReference(
 /// \param value the value to use when initializing the property.
 static void initializeProperty(SILGenFunction &SGF, SILLocation loc,
                                SILValue actorSelf,
-                               VarDecl* prop, SILValue value) {
+                               VarDecl* prop, SILValue value,
+                               IsTake_t isTake) {
   Type formalType = SGF.F.mapTypeIntoContext(prop->getInterfaceType());
   SILType loweredType = SGF.getLoweredType(formalType);
 
   auto fieldAddr = emitActorPropertyReference(SGF, loc, actorSelf, prop);
 
   if (loweredType.isAddressOnly(SGF.F)) {
-    SGF.B.createCopyAddr(loc, value, fieldAddr, IsNotTake, IsInitialization);
+    SGF.B.createCopyAddr(loc, value, fieldAddr, isTake, IsInitialization);
   } else {
     if (value->getType().isAddress()) {
       SGF.emitSemanticLoadInto(loc, value, SGF.F.getTypeLowering(value->getType()),
-          fieldAddr, SGF.getTypeLowering(loweredType), IsTake, IsInitialization);
+          fieldAddr, SGF.getTypeLowering(loweredType), isTake, IsInitialization);
     } else {
       value = SGF.B.emitCopyValueOperation(loc, value);
       SGF.B.emitStoreValueOperation(
@@ -152,10 +153,10 @@ static SILArgument *findFirstDistributedActorSystemArg(SILFunction &F) {
 /// For the initialization of a local distributed actor instance, emits code to
 /// initialize the instance's stored property corresponding to the system.
 static void emitActorSystemInit(SILGenFunction &SGF,
-                                        ConstructorDecl *ctor,
-                                        SILLocation loc,
-                                        ManagedValue actorSelf,
-                                        SILValue systemValue) {
+                                ConstructorDecl *ctor,
+                                SILLocation loc,
+                                ManagedValue actorSelf,
+                                SILValue systemValue) {
   assert(ctor->isImplicit() && "unexpected explicit dist actor init");
   assert(ctor->isDesignatedInit());
 
@@ -166,9 +167,8 @@ static void emitActorSystemInit(SILGenFunction &SGF,
   // exactly one ActorSystem-conforming argument to the constructor,
   // so we grab the first one from the params.
   VarDecl *var = classDecl->getDistributedActorSystemProperty();
-  assert(var);
-
-  initializeProperty(SGF, loc, actorSelf.getValue(), var, systemValue);
+
+  initializeProperty(SGF, loc, actorSelf.getValue(), var, systemValue, IsNotTake);
 }
 
 /// Emits the distributed actor's identity (`id`) initialization.
@@ -209,7 +209,7 @@ void SILGenFunction::emitDistActorIdentityInit(ConstructorDecl *ctor,
       { temp, selfMetatypeValue });
 
   // --- initialize the property.
-  initializeProperty(*this, loc, borrowedSelfArg, var, temp);
+  initializeProperty(*this, loc, borrowedSelfArg, var, temp, IsTake);
 }
 
 // TODO(distributed): rename to DistributedActorID
@@ -443,14 +443,16 @@ void SILGenFunction::emitDistributedActorFactory(FuncDecl *fd) { // TODO(distrib
     loc.markAutoGenerated();
     auto *dc = fd->getDeclContext();
     auto classDecl = dc->getSelfClassDecl();
-    
+
     initializeProperty(*this, loc, remote,
                        classDecl->getDistributedActorIDProperty(),
-                       idArg);
+                       idArg,
+                       IsNotTake);
 
     initializeProperty(*this, loc, remote,
                        classDecl->getDistributedActorSystemProperty(),
-                       actorSystemArg);
+                       actorSystemArg,
+                       IsNotTake);
 
     // ==== Branch to return the fully initialized remote instance
     B.createBranch(loc, returnBB, {remote});

test/Distributed/Inputs/FakeDistributedActorSystems.swift

@@ -34,6 +34,36 @@ public struct ActorAddress: Hashable, Sendable, Codable {
   }
 }
 
+/// This type is same as ActorAddress however for purposes of SIL tests we make it not-loadable.
+///
+/// By adding the `Any` we don't know the full size of the struct making the type in SIL `$*ActorAddress`
+/// when we try to store or pass it around, which triggers `isAddressOnly` guarded paths which we need to test.
+public struct NotLoadableActorAddress: Hashable, Sendable, Codable {
+  public let address: String
+  public let any: Sendable = "" // DO NOT REMOVE, this makes this struct address-only which is crucial for testing.
+
+  public init(parse address: String) {
+    self.address = address
+  }
+
+  public init(from decoder: Decoder) throws {
+    let container = try decoder.singleValueContainer()
+    self.address = try container.decode(String.self)
+  }
+
+  public func encode(to encoder: Encoder) throws {
+    var container = encoder.singleValueContainer()
+    try container.encode(self.address)
+  }
+
+  public func hash(into hasher: inout Swift.Hasher) {
+  }
+
+  public static func ==(lhs: NotLoadableActorAddress, rhs: NotLoadableActorAddress) -> Bool {
+    lhs.address == rhs.address
+  }
+}
+
 // ==== Noop Transport ---------------------------------------------------------
 
 @available(SwiftStdlib 5.7, *)
@@ -109,9 +139,81 @@ public struct FakeActorSystem: DistributedActorSystem, CustomStringConvertible {
   }
 }
 
+@available(SwiftStdlib 5.7, *)
+public struct FakeNotLoadableAddressActorSystem: DistributedActorSystem, CustomStringConvertible {
+  public typealias ActorID = NotLoadableActorAddress
+  public typealias InvocationDecoder = FakeInvocationDecoder
+  public typealias InvocationEncoder = FakeInvocationEncoder
+  public typealias SerializationRequirement = Codable
+  public typealias ResultHandler = FakeRoundtripResultHandler
+
+  // just so that the struct does not become "trivial"
+  let someValue: String = ""
+  let someValue2: String = ""
+  let someValue3: String = ""
+  let someValue4: String = ""
+
+  public init() {
+    print("Initialized new FakeActorSystem")
+  }
+
+  public func resolve<Act>(id: ActorID, as actorType: Act.Type) throws -> Act?
+      where Act: DistributedActor,
+      Act.ID == ActorID  {
+    nil
+  }
+
+  public func assignID<Act>(_ actorType: Act.Type) -> ActorID
+      where Act: DistributedActor,
+      Act.ID == ActorID {
+    NotLoadableActorAddress(parse: "xxx")
+  }
+
+  public func actorReady<Act>(_ actor: Act)
+      where Act: DistributedActor,
+            Act.ID == ActorID {
+  }
+
+  public func resignID(_ id: ActorID) {
+  }
+
+  public func makeInvocationEncoder() -> InvocationEncoder {
+    .init()
+  }
+
+  public func remoteCall<Act, Err, Res>(
+      on actor: Act,
+      target: RemoteCallTarget,
+      invocation invocationEncoder: inout InvocationEncoder,
+      throwing: Err.Type,
+      returning: Res.Type
+  ) async throws -> Res
+    where Act: DistributedActor,
+          Act.ID == ActorID,
+          Err: Error,
+          Res: SerializationRequirement {
+    throw ExecuteDistributedTargetError(message: "\(#function) not implemented.")
+  }
+
+  public func remoteCallVoid<Act, Err>(
+    on actor: Act,
+    target: RemoteCallTarget,
+    invocation invocationEncoder: inout InvocationEncoder,
+    throwing: Err.Type
+  ) async throws
+    where Act: DistributedActor,
+          Act.ID == ActorID,
+          Err: Error {
+    throw ExecuteDistributedTargetError(message: "\(#function) not implemented.")
+  }
+
+  public nonisolated var description: Swift.String {
+    "\(Self.self)()"
+  }
+}
+
 // ==== Fake Roundtrip Transport -----------------------------------------------
 
-// TODO(distributed): not thread safe...
 @available(SwiftStdlib 5.7, *)
 public final class FakeRoundtripActorSystem: DistributedActorSystem, @unchecked Sendable {
   public typealias ActorID = ActorAddress

test/Distributed/SIL/distributed_actor_default_init_sil_8.swift

@@ -0,0 +1,102 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/../Inputs/FakeDistributedActorSystems.swift
+// RUN: %target-swift-frontend -module-name default_deinit -primary-file %s -emit-sil -verify -disable-availability-checking -I %t | %FileCheck %s --enable-var-scope --dump-input=fail
+// REQUIRES: concurrency
+// REQUIRES: distributed
+
+/// The convention in this test is that the Swift declaration comes before its FileCheck lines.
+
+import Distributed
+import FakeDistributedActorSystems
+
+/// This actor system is a class, therefore SIL is slightly different
+typealias DefaultDistributedActorSystem = FakeRoundtripActorSystem
+
+// ==== ----------------------------------------------------------------------------------------------------------------
+
+class SomeClass {}
+
+enum Err : Error {
+  case blah
+}
+
+func getSomeClass() throws -> SomeClass { throw Err.blah }
+func getSystem() throws -> FakeRoundtripActorSystem { throw Err.blah }
+
+distributed actor MyDistActor {
+  var someField: SomeClass
+
+  init() throws {
+    do {
+      actorSystem = try getSystem()
+    } catch {
+      actorSystem = FakeRoundtripActorSystem()
+    }
+    someField = try getSomeClass()
+  }
+
+// CHECK:  sil hidden @$s14default_deinit11MyDistActorCACyKcfc : $@convention(method) (@owned MyDistActor) -> (@owned MyDistActor, @error any Error) {
+// CHECK:  bb0([[SELF:%[0-9]+]] : $MyDistActor):
+// CHECK:    builtin "initializeDefaultActor"([[SELF]] : $MyDistActor)
+// CHECK:    try_apply {{%[0-9]+}}() : $@convention(thin) () -> (@owned FakeRoundtripActorSystem, @error any Error), normal [[SYSTEM_SUCCESS_BB:bb[0-9]+]], error [[SYSTEM_ERROR_BB:bb[0-9]+]]
+
+// CHECK:  [[SYSTEM_SUCCESS_BB]]([[SYSTEM_VAL:%[0-9]+]] : $FakeRoundtripActorSystem):
+                // *** save system ***
+// CHECK:    [[TP_FIELD1:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
+// CHECK:    store [[SYSTEM_VAL]] to [[TP_FIELD1]] : $*FakeRoundtripActorSystem
+                // *** obtain an identity ***
+// CHECK:    [[TP_FIELD2:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
+// CHECK:    [[RELOADED_SYS1:%[0-9]+]] = load [[TP_FIELD2]] : $*FakeRoundtripActorSystem
+// CHECK:    [[SELF_METATYPE:%[0-9]+]] = metatype $@thick MyDistActor.Type
+// CHECK:    strong_retain [[RELOADED_SYS1]] : $FakeRoundtripActorSystem
+// CHECK:    [[ASSIGN_ID_FN:%[0-9]+]] = function_ref @$s27FakeDistributedActorSystems0a9RoundtripC6SystemC8assignIDyAA0C7AddressVxm0B00bC0RzlF
+// CHECK:    [[ID:%[0-9]+]] = apply [[ASSIGN_ID_FN]]<MyDistActor>([[SELF_METATYPE]], [[RELOADED_SYS1]])
+// CHECK:    strong_release [[RELOADED_SYS1]] : $FakeRoundtripActorSystem
+                // *** save identity ***
+// CHECK:    [[ID_FIELD:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.id
+// CHECK:    store [[ID]] to [[ID_FIELD]] : $*ActorAddress
+// CHECK-NOT: apply
+// CHECK:    br [[JOIN_PT:bb[0-9]+]]
+
+// CHECK:  [[JOIN_PT]]:
+// CHECK:    try_apply {{.*}}() : $@convention(thin) () -> (@owned SomeClass, @error any Error), normal [[CLASS_SUCCESS_BB:bb[0-9]+]], error [[CLASS_ERROR_BB:bb[0-9]+]]
+
+// CHECK:  [[CLASS_SUCCESS_BB]]{{.*}}:
+// CHECK:    store {{.*}} to {{.*}} : $*SomeClass
+                // *** invoke actorReady ***
+// CHECK:    [[TP_FIELD3:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
+// CHECK:    [[RELOADED_SYS2:%[0-9]+]] = load [[TP_FIELD3]] : $*FakeRoundtripActorSystem
+// CHECK:    [[READY_FN:%[0-9]+]] = function_ref @$s27FakeDistributedActorSystems0a9RoundtripC6SystemC10actorReadyyyx0B00bC0RzAA0C7AddressV2IDRtzlF
+// CHECK:    = apply [[READY_FN]]<MyDistActor>([[SELF]], [[RELOADED_SYS2]])
+// CHECK:    return [[SELF]] : $MyDistActor
+
+// CHECK:  [[SYSTEM_ERROR_BB]]([[ERROR_ARG:%[0-9]+]] : $any Error):
+// CHECK:    strong_retain [[ERROR_ARG]] : $any Error
+// CHECK:    function_ref @$s27FakeDistributedActorSystems0a9RoundtripC6SystemCACycfC
+// CHECK:    store {{.*}} to {{.*}} : $*FakeRoundtripActorSystem
+// CHECK:    store {{.*}} to {{.*}} : $*ActorAddress
+// CHECK:    br [[JOIN_PT]]
+
+// CHECK:  [[CLASS_ERROR_BB]]{{.*}}:
+            // ** deinit the id **
+// CHECK:    [[REF_ID_D:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.id
+// CHECK:    [[ID_D:%[0-9]+]] = begin_access [deinit] [static] [[REF_ID_D]] : $*ActorAddress
+// CHECK:    [[REF_SYS_D:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
+// CHECK:    [[ID:%[0-9]+]] = load [[ID_D]] : $*ActorAddress
+// CHECK:    [[SYS:%[0-9]+]] = load [[REF_SYS_D]] : $*FakeRoundtripActorSystem
+// CHECK:    [[RESIGN_FN:%[0-9]+]] = function_ref @$s27FakeDistributedActorSystems0a9RoundtripC6SystemC8resignIDyyAA0C7AddressVF
+// CHECK:    = apply [[RESIGN_FN]]([[ID]], [[SYS]]) : $@convention(method) (@guaranteed ActorAddress, @guaranteed FakeRoundtripActorSystem) -> ()
+// CHECK:    destroy_addr [[ID_D]] : $*ActorAddress
+// CHECK:    end_access [[ID_D]] : $*ActorAddress
+            // ** deinit the system **
+// CHECK:    [[REF_SYS_D2:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
+// CHECK:    [[SYSTEM_ACC:%[0-9]+]] = begin_access [deinit] [static] [[REF_SYS_D2]] : $*FakeRoundtripActorSystem
+// CHECK:    destroy_addr [[SYSTEM_ACC]] : $*FakeRoundtripActorSystem
+// CHECK:    end_access [[SYSTEM_ACC]] : $*FakeRoundtripActorSystem
+// CHECK:    builtin "destroyDefaultActor"([[SELF]] : $MyDistActor) : $()
+// CHECK:    dealloc_partial_ref [[SELF]]
+// CHECK:    throw
+
+
+}
+

test/Distributed/SIL/distributed_actor_resolve_sil.swift

@@ -0,0 +1,33 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/../Inputs/FakeDistributedActorSystems.swift
+// RUN: %target-swift-frontend -module-name default_deinit -primary-file %s -emit-sil -verify -disable-availability-checking -I %t | %FileCheck %s --enable-var-scope --dump-input=fail
+// REQUIRES: concurrency
+// REQUIRES: distributed
+
+/// The convention in this test is that the Swift declaration comes before its FileCheck lines.
+
+import Distributed
+import FakeDistributedActorSystems
+
+typealias DefaultDistributedActorSystem = FakeRoundtripActorSystem
+
+// ==== ----------------------------------------------------------------------------------------------------------------
+
+distributed actor MyDistActor {
+
+// protocol witness for DistributedActorSystem.resolve<A>(id:as:) in conformance FakeRoundtripActorSystem
+// CHECK:  sil hidden @$s14default_deinit11MyDistActorC7resolve2id5usingAC015FakeDistributedE7Systems0E7AddressV_AG0i9RoundtripE6SystemCtKFZ
+// CHECK:  bb0([[ACTOR_ID_ARG:%[0-9]+]] : $ActorAddress, [[SYSTEM_ARG:%[0-9]+]] : $FakeRoundtripActorSystem, [[TYPE_ARG:%[0-9]+]] : $@thick MyDistActor.Type):
+// CHECK: [[SYS_RESOLVE_RESULT:%[0-9]+]] = function_ref @$s27FakeDistributedActorSystems0a9RoundtripC6SystemC7resolve2id2asxSgAA0C7AddressV_xmtK0B00bC0RzlF
+
+// CHECK: // makeProxyBB
+// CHECK: [[ACTOR_INSTANCE:%[0-9]+]] = builtin "initializeDistributedRemoteActor"(%7 : $@thick MyDistActor.Type) : $MyDistActor
+// CHECK: [[ID_PROPERTY:%[0-9]+]] = ref_element_addr [[ACTOR_INSTANCE]] : $MyDistActor, #MyDistActor.id
+// CHECK: retain_value [[ACTOR_ID_ARG]] : $ActorAddress
+// CHECK: store [[ACTOR_ID_ARG]] to [[ID_PROPERTY]] : $*ActorAddress
+// CHECK: [[SYSTEM_PROPERTY:%[0-9]+]] = ref_element_addr [[ACTOR_INSTANCE]] : $MyDistActor, #MyDistActor.actorSystem
+// CHECK: strong_retain [[SYSTEM_ARG]] : $FakeRoundtripActorSystem
+// CHECK: store [[SYSTEM_ARG]] to [[SYSTEM_PROPERTY]] : $*FakeRoundtripActorSystem
+// CHECK: br bb5([[ACTOR_INSTANCE]] : $MyDistActor)
+}
+

test/Distributed/SIL/distributed_id_system_ownership_verify_sil.swift

@@ -0,0 +1,52 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/../Inputs/FakeDistributedActorSystems.swift
+// RUN: %target-swift-frontend -verify -emit-sil -module-name main -disable-availability-checking -parse-as-library -I %t %s %S/../Inputs/FakeDistributedActorSystems.swift | %FileCheck %s
+
+// REQUIRES: concurrency
+// REQUIRES: distributed
+
+// rdar://76038845
+// UNSUPPORTED: use_os_stdlib
+// UNSUPPORTED: back_deployment_runtime
+
+// FIXME(distributed): Distributed actors currently have some issues on windows, isRemote always returns false. rdar://82593574
+// UNSUPPORTED: OS=windows-msvc
+
+import Distributed
+import FakeDistributedActorSystems
+
+/// This type uses a non loadable (size not known at compile time) ID which forces the actor's init SILGen
+/// to make use of the isAddressOnly handling of the ID assignments.
+///
+/// This test covers rdar://104583893 / https://github.com/apple/swift/issues/62898
+/// And used to fail with:
+///     SIL memory lifetime failure in @$s18DistributedCluster0B16EventStreamActorC11actorSystemAcA0bG0C_tcfc: memory is initialized, but shouldn't be
+typealias DefaultDistributedActorSystem = FakeNotLoadableAddressActorSystem
+
+distributed actor Greeter {
+
+  init(actorSystem: ActorSystem) {
+    self.actorSystem = actorSystem
+  }
+
+  distributed func echo(name: String) -> String {
+    return "Echo: \(name)"
+  }
+}
+
+// CHECK: sil hidden @$s4main7GreeterC7resolve2id5usingAcA23NotLoadableActorAddressV_AA04FakefgiH6SystemVtKFZ : $@convention(method) (@in_guaranteed NotLoadableActorAddress, @guaranteed FakeNotLoadableAddressActorSystem, @thick Greeter.Type) -> (@owned Greeter, @error any Error) {
+// CHECK: bb0([[ADDRESS_ARG:%[0-9]+]] : $*NotLoadableActorAddress, [[SYSTEM_ARG:%[0-9]+]] : $FakeNotLoadableAddressActorSystem, [[TYPE_ARG:%[0-9]+]] : $@thick Greeter.Type):
+// CHECK: [[TYPE:%[0-9]+]] = metatype $@thick Greeter.Type
+
+// CHECK: // makeProxyBB
+// CHECK: [[INSTANCE:%[0-9]+]] = builtin "initializeDistributedRemoteActor"([[TYPE]] : $@thick Greeter.Type) : $Greeter
+// CHECK: [[ID_PROPERTY:%[0-9]+]] = ref_element_addr [[INSTANCE]] : $Greeter, #Greeter.id
+// Note specifically that we don't [take] in the below copy_addr:
+// CHECK: copy_addr [[ADDRESS_ARG]] to [init] [[ID_PROPERTY]] : $*NotLoadableActorAddress
+
+func test() async throws {
+  let system = DefaultDistributedActorSystem()
+
+  let local = Greeter(actorSystem: system)
+  _ = try await local.echo(name: "Caplin")
+}

test/Distributed/distributed_actor_accessor_section_elf.swift

@@ -1,6 +1,6 @@
 // RUN: %empty-directory(%t)
 // RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/Inputs/FakeDistributedActorSystems.swift
-// RUN: %target-swift-frontend -emit-irgen -module-name distributed_actor_accessors -disable-availability-checking -I %t 2>&1 %s | %IRGenFileCheck %s --dump-input=always
+// RUN: %target-swift-frontend -emit-irgen -module-name distributed_actor_accessors -disable-availability-checking -I %t 2>&1 %s | %IRGenFileCheck %s
 
 // UNSUPPORTED: back_deploy_concurrency
 // REQUIRES: concurrency