Layout distributed actor id and actorSystem fields first in stored properties

The stored properties `id` and `actorSystem` that are synthesized for
a distributed actor would end up getting placed in fairly random
locations within the stored properties lists based on when they were
synthesized. This means that they (1) weren't always in the prefix, and
(2) weren't always the same order in every translation unit. Hilarity ensues.

Fixes rdar://92142457.

Author: DougGregor

lib/Sema/TypeCheckStorage.cpp

@@ -150,6 +150,55 @@ static void computeLoweredStoredProperties(NominalTypeDecl *decl) {
   }
 }
 
+/// Enumerate both the stored properties and missing members,
+/// in a deterministic order.
+static void enumerateStoredPropertiesAndMissing(
+    NominalTypeDecl *decl,
+    llvm::function_ref<void(VarDecl *)> addStoredProperty,
+    llvm::function_ref<void(MissingMemberDecl *)> addMissing) {
+  // If we have a distributed actor, find the id and actorSystem
+  // properties. We always want them first, and in a specific
+  // order.
+  VarDecl *distributedActorId = nullptr;
+  VarDecl *distributedActorSystem = nullptr;
+  if (decl->isDistributedActor()) {
+    ASTContext &ctx = decl->getASTContext();
+    for (auto *member : decl->getMembers()) {
+      if (auto *var = dyn_cast<VarDecl>(member)) {
+        if (!var->isStatic() && var->hasStorage() &&
+            var->isSynthesized()) {
+          if (var->getName() == ctx.Id_id)
+            distributedActorId = var;
+          else if (var->getName() == ctx.Id_actorSystem)
+            distributedActorSystem = var;
+        }
+
+        if (distributedActorId && distributedActorSystem)
+          break;
+      }
+    }
+
+    if (distributedActorId)
+      addStoredProperty(distributedActorId);
+    if (distributedActorSystem)
+      addStoredProperty(distributedActorSystem);
+  }
+
+  for (auto *member : decl->getMembers()) {
+    if (auto *var = dyn_cast<VarDecl>(member)) {
+      if (!var->isStatic() && var->hasStorage() &&
+          var != distributedActorId &&
+          var != distributedActorSystem) {
+        addStoredProperty(var);
+      }
+    }
+
+    if (auto missing = dyn_cast<MissingMemberDecl>(member))
+      if (missing->getNumberOfFieldOffsetVectorEntries() > 0)
+        addMissing(missing);
+  }
+}
+
 ArrayRef<VarDecl *>
 StoredPropertiesRequest::evaluate(Evaluator &evaluator,
                                   NominalTypeDecl *decl) const {
@@ -163,12 +212,11 @@ StoredPropertiesRequest::evaluate(Evaluator &evaluator,
   if (isa<SourceFile>(decl->getModuleScopeContext()))
     computeLoweredStoredProperties(decl);
 
-  for (auto *member : decl->getMembers()) {
-    if (auto *var = dyn_cast<VarDecl>(member))
-      if (!var->isStatic() && var->hasStorage()) {
-        results.push_back(var);
-      }
-  }
+  enumerateStoredPropertiesAndMissing(decl,
+    [&](VarDecl *var) {
+      results.push_back(var);
+    },
+    [](MissingMemberDecl *missing) { });
 
   return decl->getASTContext().AllocateCopy(results);
 }
@@ -186,15 +234,13 @@ StoredPropertiesAndMissingMembersRequest::evaluate(Evaluator &evaluator,
   if (isa<SourceFile>(decl->getModuleScopeContext()))
     computeLoweredStoredProperties(decl);
 
-  for (auto *member : decl->getMembers()) {
-    if (auto *var = dyn_cast<VarDecl>(member))
-      if (!var->isStatic() && var->hasStorage())
-        results.push_back(var);
-
-    if (auto missing = dyn_cast<MissingMemberDecl>(member))
-      if (missing->getNumberOfFieldOffsetVectorEntries() > 0)
-        results.push_back(missing);
-  }
+  enumerateStoredPropertiesAndMissing(decl,
+    [&](VarDecl *var) {
+      results.push_back(var);
+    },
+    [&](MissingMemberDecl *missing) {
+      results.push_back(missing);
+    });
 
   return decl->getASTContext().AllocateCopy(results);
 }

test/Distributed/SIL/distributed_actor_default_deinit_sil.swift

@@ -61,33 +61,34 @@ distributed actor MyDistActor {
 
 // *** only destroy the id and system if remote ***
 // CHECK: [[REMOTE_BB_DEALLOC]]:
-            // *** destroy system ***
-// SKIP:   [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
-// SKIP:   [[ACCESS:%[0-9]+]] = begin_access [deinit] [static] [[REF]]
-// SKIP:   destroy_addr [[ACCESS]] : $*FakeActorSystem
-// SKIP:   end_access [[ACCESS]]
             // *** destroy identity ***
 // CHECK:   [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.id
 // CHECK:   [[ACCESS:%[0-9]+]] = begin_access [deinit] [static] [[REF]]
 // CHECK:   destroy_addr [[ACCESS]] : $*ActorAddress
+// CHECK:   end_access [[ACCESS]]
+            // *** destroy system ***
+// CHECK:   [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
+// CHECK:   [[ACCESS:%[0-9]+]] = begin_access [deinit] [static] [[REF]]
+// CHECK:   destroy_addr [[ACCESS]] : $*FakeActorSystem
 // CHECK:   end_access [[ACCESS]]
 // CHECK:   br [[AFTER_DEALLOC:bb[0-9]+]]
 
 // *** destroy everything if local ***
 // CHECK: [[LOCAL_BB_DEALLOC]]:
-            // *** destroy the user-defined field ***
-// CHECK:   [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.localOnlyField
+            // *** destroy identity ***
+// CHECK:   [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.id
 // CHECK:   [[ACCESS:%[0-9]+]] = begin_access [deinit] [static] [[REF]]
-// CHECK:   destroy_addr [[ACCESS]] : $*SomeClass
+// CHECK:   destroy_addr [[ACCESS]] : $*ActorAddress
 // CHECK:   end_access [[ACCESS]]
-            // *** the rest of this part is identical to the remote case ***
+            // *** destroy system ***
 // SKIP:   [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
 // SKIP:   [[ACCESS:%[0-9]+]] = begin_access [deinit] [static] [[REF]]
 // SKIP:   destroy_addr [[ACCESS]] : $*FakeActorSystem
 // SKIP:   end_access [[ACCESS]]
-// CHECK:   [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.id
+            // *** destroy the user-defined field ***
+// CHECK:   [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.localOnlyField
 // CHECK:   [[ACCESS:%[0-9]+]] = begin_access [deinit] [static] [[REF]]
-// CHECK:   destroy_addr [[ACCESS]] : $*ActorAddress
+// CHECK:   destroy_addr [[ACCESS]] : $*SomeClass
 // CHECK:   end_access [[ACCESS]]
 // CHECK:   br [[AFTER_DEALLOC]]
 

test/Distributed/distributed_actor_layout.swift

@@ -0,0 +1,24 @@
+// 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
+
+// UNSUPPORTED: back_deploy_concurrency
+// REQUIRES: concurrency
+// REQUIRES: distributed
+
+import Distributed
+import FakeDistributedActorSystems
+
+@available(SwiftStdlib 5.7, *)
+typealias DefaultDistributedActorSystem = FakeActorSystem
+
+class MyClass { }
+
+// Ensure that the actor layout is (metadata pointer, default actor, id, system,
+// <user fields>)
+
+// CHECK: %T27distributed_actor_accessors7MyActorC = type <{ %swift.refcounted, %swift.defaultactor, %T27FakeDistributedActorSystems0C7AddressV, %T27FakeDistributedActorSystems0aC6SystemV, %T27distributed_actor_accessors7MyClassC* }>
+@available(SwiftStdlib 5.7, *)
+public distributed actor MyActor {
+  var field: MyClass = MyClass()
+}