Merge pull request #42568 from DougGregor/distributed-actor-stored-property-layout-5.7-04182022

[5.7-04182022] Layout distributed actor id and actorSystem fields first in stored properties

Author: DougGregor

lib/Sema/TypeCheckStorage.cpp

@@ -150,6 +150,59 @@ 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()) {
+          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()) {
+        // Skip any properties that we already emitted explicitly
+        if (var == distributedActorId)
+          continue;
+        if (var == distributedActorSystem)
+          continue;
+
+        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 +216,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 +238,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()
+}