[Distributed] Force the order of properties in AST

fix messed up rebase

Author: ktoso

lib/Sema/CodeSynthesisDistributedActor.cpp

@@ -117,9 +117,17 @@ static VarDecl *addImplicitDistributedActorIDProperty(
   propDecl->getAttrs().add(
       new (C) CompilerInitializedAttr(/*IsImplicit=*/true));
 
-  nominal->addMember(propDecl);
-  nominal->addMember(pbDecl);
-
+  // IMPORTANT: The `id` MUST be the first field of any distributed actor,
+  // because when we allocate remote proxy instances, we don't allocate memory
+  // for anything except the first two fields: id and actorSystem, so they
+  // MUST be those fields.
+  //
+  // Their specific order also matters, because it is enforced this way in IRGen
+  // and how we emit them in AST MUST match what IRGen expects or cross-module
+  // things could be using wrong offsets and manifest as reading trash memory on
+  // id or system accesses.
+  nominal->addMember(propDecl, /*hint=*/nullptr, /*insertAtHead=*/true);
+  nominal->addMember(pbDecl, /*hint=*/nullptr, /*insertAtHead=*/true);
   return propDecl;
 }
 

lib/Sema/DerivedConformanceDistributedActor.cpp

@@ -426,32 +426,6 @@ static FuncDecl *deriveDistributedActorSystem_invokeHandlerOnReturn(
 /******************************* PROPERTIES ***********************************/
 /******************************************************************************/
 
-// TODO(distributed): make use of this after all, but FORCE it?
-static ValueDecl *deriveDistributedActor_id(DerivedConformance &derived) {
-  assert(derived.Nominal->isDistributedActor());
-  auto &C = derived.Context;
-
-  // ```
-  // nonisolated
-  // let id: Self.ID // Self.ActorSystem.ActorID
-  // ```
-  auto propertyType = getDistributedActorIDType(derived.Nominal);
-
-  VarDecl *propDecl;
-  PatternBindingDecl *pbDecl;
-  std::tie(propDecl, pbDecl) = derived.declareDerivedProperty(
-      DerivedConformance::SynthesizedIntroducer::Let, C.Id_id, propertyType,
-      propertyType,
-      /*isStatic=*/false, /*isFinal=*/true);
-
-  // mark as nonisolated, allowing access to it from everywhere
-  propDecl->getAttrs().add(
-      new (C) NonisolatedAttr(/*IsImplicit=*/true));
-
-  derived.addMembersToConformanceContext({ propDecl, pbDecl });
-  return propDecl;
-}
-
 static ValueDecl *deriveDistributedActor_actorSystem(
     DerivedConformance &derived) {
   auto &C = derived.Context;
@@ -460,8 +434,7 @@ static ValueDecl *deriveDistributedActor_actorSystem(
   assert(classDecl && derived.Nominal->isDistributedActor());
 
   // ```
-  // nonisolated
-  // let actorSystem: ActorSystem
+  // nonisolated let actorSystem: ActorSystem
   // ```
   // (no need for @actorIndependent because it is an immutable let)
   auto propertyType = getDistributedActorSystemType(classDecl);
@@ -477,7 +450,14 @@ static ValueDecl *deriveDistributedActor_actorSystem(
   propDecl->getAttrs().add(
       new (C) NonisolatedAttr(/*IsImplicit=*/true));
 
-  derived.addMembersToConformanceContext({ propDecl, pbDecl });
+  // IMPORTANT: `id` MUST be the first field of a distributed actor, and
+  // `actorSystem` MUST be the second field, because for a remote instance
+  // we don't allocate memory after those two fields, so their order is very
+  // important. The `hint` below makes sure the system is inserted right after.
+  auto id = derived.Nominal->getDistributedActorIDProperty();
+  derived.addMemberToConformanceContext(pbDecl, /*hint=*/id);
+  derived.addMemberToConformanceContext(propDecl, /*hint=*/id);
+
   return propDecl;
 }
 
@@ -571,11 +551,14 @@ deriveDistributedActorType_SerializationRequirement(
 
 ValueDecl *DerivedConformance::deriveDistributedActor(ValueDecl *requirement) {
   if (auto var = dyn_cast<VarDecl>(requirement)) {
-    if (var->getName() == Context.Id_id)
-      return deriveDistributedActor_id(*this);
-
     if (var->getName() == Context.Id_actorSystem)
       return deriveDistributedActor_actorSystem(*this);
+
+    if (var->getName() == Context.Id_id)
+      llvm_unreachable("DistributedActor.id MUST be synthesized earlier, "
+                       "because it is forced by the Identifiable conformance. "
+                       "If we attempted to do synthesis here, the earlier phase "
+                       "failed and something is wrong: please report a bug.");
   }
 
   if (auto func = dyn_cast<FuncDecl>(requirement)) {

lib/Sema/DerivedConformances.cpp

@@ -51,6 +51,18 @@ void DerivedConformance::addMembersToConformanceContext(
     IDC->addMember(child);
 }
 
+void DerivedConformance::addMemberToConformanceContext(
+    Decl *member, Decl *hint) {
+  auto IDC = cast<IterableDeclContext>(ConformanceDecl);
+  IDC->addMember(member, hint, /*insertAtHead=*/false);
+}
+
+void DerivedConformance::addMemberToConformanceContext(
+    Decl *member, bool insertAtHead) {
+  auto IDC = cast<IterableDeclContext>(ConformanceDecl);
+  IDC->addMember(member, /*hint=*/nullptr, insertAtHead);
+}
+
 Type DerivedConformance::getProtocolType() const {
   return Protocol->getDeclaredInterfaceType();
 }
@@ -325,10 +337,6 @@ ValueDecl *DerivedConformance::getDerivableRequirement(NominalTypeDecl *nominal,
     if (name.isSimpleName(ctx.Id_unownedExecutor))
       return getRequirement(KnownProtocolKind::Actor);
 
-    // DistributedActor.id
-    if(name.isSimpleName(ctx.Id_id))
-      return getRequirement(KnownProtocolKind::DistributedActor);
-
     // DistributedActor.actorSystem
     if(name.isSimpleName(ctx.Id_actorSystem))
       return getRequirement(KnownProtocolKind::DistributedActor);

lib/Sema/DerivedConformances.h

@@ -70,6 +70,10 @@ class DerivedConformance {
 
   /// Add \c children as members of the context that declares the conformance.
   void addMembersToConformanceContext(ArrayRef<Decl *> children);
+  /// Add \c member right after the \c hint member which may be the tail
+  void addMemberToConformanceContext(Decl *member, Decl* hint);
+  /// Add \c member in front of any other existing members
+  void addMemberToConformanceContext(Decl *member, bool insertAtHead);
 
   /// Get the declared type of the protocol that this is conformance is for.
   Type getProtocolType() const;

stdlib/public/Distributed/LocalTestingDistributedActorSystem.swift

@@ -125,22 +125,34 @@ public final class LocalTestingDistributedActorSystem: DistributedActorSystem, @
   }
 }
 
+@available(*, deprecated, renamed: "LocalTestingActorID")
+public typealias LocalTestingActorAddress = LocalTestingActorID
+
 @available(SwiftStdlib 5.7, *)
-public struct LocalTestingActorAddress: Hashable, Sendable, Codable {
-  public let address: String
+public struct LocalTestingActorID: Hashable, Sendable, Codable {
+  @available(*, deprecated, renamed: "id")
+  public var address: String {
+    self.id
+  }
+  public let id: String
+
+  @available(*, deprecated, renamed: "init(id:)")
+  public init(parse id: String) {
+    self.id = id
+  }
 
-  public init(parse address: String) {
-    self.address = address
+  public init(id: String) {
+    self.id = id
   }
 
   public init(from decoder: Decoder) throws {
     let container = try decoder.singleValueContainer()
-    self.address = try container.decode(String.self)
+    self.id = try container.decode(String.self)
   }
 
   public func encode(to encoder: Encoder) throws {
     var container = encoder.singleValueContainer()
-    try container.encode(self.address)
+    try container.encode(self.id)
   }
 }
 

test/Distributed/Runtime/distributed_actor_init_local.swift

@@ -90,6 +90,15 @@ distributed actor MaybeAfterAssign {
   }
 }
 
+distributed actor LocalTestingDA_Int {
+  typealias ActorSystem = LocalTestingDistributedActorSystem
+  var int: Int
+  init() {
+    actorSystem = .init()
+    int = 12
+  }
+}
+
 // ==== Fake Transport ---------------------------------------------------------
 
 struct ActorAddress: Sendable, Hashable, Codable {
@@ -262,6 +271,10 @@ func test() async {
   // CHECK:      assign type:MaybeAfterAssign, id:ActorAddress(address: "[[ID10:.*]]")
   // CHECK-NEXT: ready actor:main.MaybeAfterAssign, id:ActorAddress(address: "[[ID10]]")
 
+  let localDA = LocalTestingDA_Int()
+  print("localDA = \(localDA.id)")
+  // CHECK: localDA = LocalTestingActorID(id: "1")
+
   // the following tests fail to initialize the actor's identity.
   print("-- start of no-assign tests --")
   test.append(MaybeSystem(nil))

test/IRGen/distributed_actor.swift

@@ -6,7 +6,7 @@
 import Distributed
 
 // Type descriptor.
-// CHECK-LABEL: @"$s17distributed_actor7MyActorC2id11Distributed012LocalTestingD7AddressVvpWvd"
+// CHECK-LABEL: @"$s17distributed_actor7MyActorC2id11Distributed012LocalTestingD2IDVvpWvd"
 @available(SwiftStdlib 5.6, *)
 public distributed actor MyActor {
   public typealias ActorSystem = LocalTestingDistributedActorSystem