Skip to content

Commit 6cd453e

Browse files
DougGregorDougGregor
committed
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.
1 parent f3ce40f commit 6cd453e

File tree

3 files changed

+97
-26
lines changed

3 files changed

+97
-26
lines changed

‎lib/Sema/TypeCheckStorage.cpp‎

Lines changed: 61 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,55 @@ static void computeLoweredStoredProperties(NominalTypeDecl *decl) {
150150
}
151151
}
152152

153+
/// Enumerate both the stored properties and missing members,
154+
/// in a deterministic order.
155+
static void enumerateStoredPropertiesAndMissing(
156+
NominalTypeDecl *decl,
157+
llvm::function_ref<void(VarDecl *)> addStoredProperty,
158+
llvm::function_ref<void(MissingMemberDecl *)> addMissing) {
159+
// If we have a distributed actor, find the id and actorSystem
160+
// properties. We always want them first, and in a specific
161+
// order.
162+
VarDecl *distributedActorId = nullptr;
163+
VarDecl *distributedActorSystem = nullptr;
164+
if (decl->isDistributedActor()) {
165+
ASTContext &ctx = decl->getASTContext();
166+
for (auto *member : decl->getMembers()) {
167+
if (auto *var = dyn_cast<VarDecl>(member)) {
168+
if (!var->isStatic() && var->hasStorage() &&
169+
var->isSynthesized()) {
170+
if (var->getName() == ctx.Id_id)
171+
distributedActorId = var;
172+
else if (var->getName() == ctx.Id_actorSystem)
173+
distributedActorSystem = var;
174+
}
175+
176+
if (distributedActorId && distributedActorSystem)
177+
break;
178+
}
179+
}
180+
181+
if (distributedActorId)
182+
addStoredProperty(distributedActorId);
183+
if (distributedActorSystem)
184+
addStoredProperty(distributedActorSystem);
185+
}
186+
187+
for (auto *member : decl->getMembers()) {
188+
if (auto *var = dyn_cast<VarDecl>(member)) {
189+
if (!var->isStatic() && var->hasStorage() &&
190+
var != distributedActorId &&
191+
var != distributedActorSystem) {
192+
addStoredProperty(var);
193+
}
194+
}
195+
196+
if (auto missing = dyn_cast<MissingMemberDecl>(member))
197+
if (missing->getNumberOfFieldOffsetVectorEntries() > 0)
198+
addMissing(missing);
199+
}
200+
}
201+
153202
ArrayRef<VarDecl *>
154203
StoredPropertiesRequest::evaluate(Evaluator &evaluator,
155204
NominalTypeDecl *decl) const {
@@ -163,12 +212,11 @@ StoredPropertiesRequest::evaluate(Evaluator &evaluator,
163212
if (isa<SourceFile>(decl->getModuleScopeContext()))
164213
computeLoweredStoredProperties(decl);
165214

166-
for (auto *member : decl->getMembers()) {
167-
if (auto *var = dyn_cast<VarDecl>(member))
168-
if (!var->isStatic() && var->hasStorage()) {
169-
results.push_back(var);
170-
}
171-
}
215+
enumerateStoredPropertiesAndMissing(decl,
216+
[&](VarDecl *var) {
217+
results.push_back(var);
218+
},
219+
[](MissingMemberDecl *missing) { });
172220

173221
return decl->getASTContext().AllocateCopy(results);
174222
}
@@ -186,15 +234,13 @@ StoredPropertiesAndMissingMembersRequest::evaluate(Evaluator &evaluator,
186234
if (isa<SourceFile>(decl->getModuleScopeContext()))
187235
computeLoweredStoredProperties(decl);
188236

189-
for (auto *member : decl->getMembers()) {
190-
if (auto *var = dyn_cast<VarDecl>(member))
191-
if (!var->isStatic() && var->hasStorage())
192-
results.push_back(var);
193-
194-
if (auto missing = dyn_cast<MissingMemberDecl>(member))
195-
if (missing->getNumberOfFieldOffsetVectorEntries() > 0)
196-
results.push_back(missing);
197-
}
237+
enumerateStoredPropertiesAndMissing(decl,
238+
[&](VarDecl *var) {
239+
results.push_back(var);
240+
},
241+
[&](MissingMemberDecl *missing) {
242+
results.push_back(missing);
243+
});
198244

199245
return decl->getASTContext().AllocateCopy(results);
200246
}

‎test/Distributed/SIL/distributed_actor_default_deinit_sil.swift‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -61,33 +61,34 @@ distributed actor MyDistActor {
6161

6262
// *** only destroy the id and system if remote ***
6363
// CHECK: [[REMOTE_BB_DEALLOC]]:
64-
// *** destroy system ***
65-
// SKIP: [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
66-
// SKIP: [[ACCESS:%[0-9]+]] = begin_access [deinit] [static] [[REF]]
67-
// SKIP: destroy_addr [[ACCESS]] : $*FakeActorSystem
68-
// SKIP: end_access [[ACCESS]]
6964
// *** destroy identity ***
7065
// CHECK: [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.id
7166
// CHECK: [[ACCESS:%[0-9]+]] = begin_access [deinit] [static] [[REF]]
7267
// CHECK: destroy_addr [[ACCESS]] : $*ActorAddress
68+
// CHECK: end_access [[ACCESS]]
69+
// *** destroy system ***
70+
// CHECK: [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
71+
// CHECK: [[ACCESS:%[0-9]+]] = begin_access [deinit] [static] [[REF]]
72+
// CHECK: destroy_addr [[ACCESS]] : $*FakeActorSystem
7373
// CHECK: end_access [[ACCESS]]
7474
// CHECK: br [[AFTER_DEALLOC:bb[0-9]+]]
7575

7676
// *** destroy everything if local ***
7777
// CHECK: [[LOCAL_BB_DEALLOC]]:
78-
// *** destroy the user-defined field ***
79-
// CHECK: [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.localOnlyField
78+
// *** destroy identity ***
79+
// CHECK: [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.id
8080
// CHECK: [[ACCESS:%[0-9]+]] = begin_access [deinit] [static] [[REF]]
81-
// CHECK: destroy_addr [[ACCESS]] : $*SomeClass
81+
// CHECK: destroy_addr [[ACCESS]] : $*ActorAddress
8282
// CHECK: end_access [[ACCESS]]
83-
// *** the rest of this part is identical to the remote case ***
83+
// *** destroy system ***
8484
// SKIP: [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.actorSystem
8585
// SKIP: [[ACCESS:%[0-9]+]] = begin_access [deinit] [static] [[REF]]
8686
// SKIP: destroy_addr [[ACCESS]] : $*FakeActorSystem
8787
// SKIP: end_access [[ACCESS]]
88-
// CHECK: [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.id
88+
// *** destroy the user-defined field ***
89+
// CHECK: [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MyDistActor, #MyDistActor.localOnlyField
8990
// CHECK: [[ACCESS:%[0-9]+]] = begin_access [deinit] [static] [[REF]]
90-
// CHECK: destroy_addr [[ACCESS]] : $*ActorAddress
91+
// CHECK: destroy_addr [[ACCESS]] : $*SomeClass
9192
// CHECK: end_access [[ACCESS]]
9293
// CHECK: br [[AFTER_DEALLOC]]
9394

‎test/Distributed/distributed_actor_layout.swift‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/Inputs/FakeDistributedActorSystems.swift
3+
// RUN: %target-swift-frontend -emit-irgen -module-name distributed_actor_accessors -disable-availability-checking -I %t 2>&1 %s | %IRGenFileCheck %s
4+
5+
// UNSUPPORTED: back_deploy_concurrency
6+
// REQUIRES: concurrency
7+
// REQUIRES: distributed
8+
9+
import Distributed
10+
import FakeDistributedActorSystems
11+
12+
@available(SwiftStdlib 5.7, *)
13+
typealias DefaultDistributedActorSystem = FakeActorSystem
14+
15+
class MyClass { }
16+
17+
// Ensure that the actor layout is (metadata pointer, default actor, id, system,
18+
// <user fields>)
19+
20+
// CHECK: %T27distributed_actor_accessors7MyActorC = type <{ %swift.refcounted, %swift.defaultactor, %T27FakeDistributedActorSystems0C7AddressV, %T27FakeDistributedActorSystems0aC6SystemV, %T27distributed_actor_accessors7MyClassC* }>
21+
@available(SwiftStdlib 5.7, *)
22+
public distributed actor MyActor {
23+
var field: MyClass = MyClass()
24+
}

0 commit comments

Comments
 (0)