Skip to content

Commit c5320c0

Browse files
committed
express resignIdentity in SILGen as an unwind-only clean-up
turning it into a clean-up covers both failable and throwing initializers. resolves rdar://84535066
1 parent 25a7327 commit c5320c0

File tree

3 files changed

+98
-13
lines changed

3 files changed

+98
-13
lines changed

lib/SILGen/SILGenConstructor.cpp

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -681,6 +681,8 @@ void SILGenFunction::emitConstructorPrologActorHop(
681681
}
682682
}
683683

684+
// MARK: class constructor
685+
684686
void SILGenFunction::emitClassConstructorInitializer(ConstructorDecl *ctor) {
685687
MagicFunctionName = SILGenModule::getMagicFunctionName(ctor);
686688

@@ -784,9 +786,8 @@ void SILGenFunction::emitClassConstructorInitializer(ConstructorDecl *ctor) {
784786
}
785787

786788
// Distributed actor initializers implicitly initialize their transport and id
787-
if (isDesignatedDistActorInit) {
789+
if (isDesignatedDistActorInit)
788790
emitDistActorImplicitPropertyInits(ctor, selfArg);
789-
}
790791

791792
// Prepare the end of initializer location.
792793
SILLocation endOfInitLoc = RegularLocation(ctor);
@@ -818,12 +819,6 @@ void SILGenFunction::emitClassConstructorInitializer(ConstructorDecl *ctor) {
818819
failureExitBB = createBasicBlock();
819820
failureExitArg = failureExitBB->createPhiArgument(
820821
resultLowering.getLoweredType(), OwnershipKind::Owned);
821-
822-
// For an async distributed actor init, we must `resignIdentity` before
823-
// the clean-ups are triggered in this failure block.
824-
if (isDesignatedDistActorInit && ctor->hasAsync()) {
825-
emitResignIdentityCall(loc, selfClassDecl, selfArg);
826-
}
827822

828823
Cleanups.emitCleanupsForReturn(ctor, IsForUnwind);
829824
SILValue nilResult =
@@ -878,9 +873,6 @@ void SILGenFunction::emitClassConstructorInitializer(ConstructorDecl *ctor) {
878873

879874
// For distributed actors, their synchronous initializers invoke "actor ready"
880875
// at the very end, just before returning on a successful initialization.
881-
// There is no need for injecting `resignIdentity` calls in such inits,
882-
// even if they are failable or throwing, because we will never call
883-
// `actorReady` at a point before the init can exit abnormally.
884876
if (isDesignatedDistActorInit && !ctor->hasAsync()) {
885877
RegularLocation loc(ctor);
886878
loc.markAutoGenerated();

lib/SILGen/SILGenDistributed.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,35 @@ static void emitIdentityInit(SILGenFunction &SGF, ConstructorDecl *ctor,
178178
initializeProperty(SGF, loc, borrowedSelfArg.getValue(), var, temp);
179179
}
180180

181+
namespace {
182+
/// Cleanup to resign the identity of a distributed actor if an abnormal exit happens.
183+
class ResignIdentity : public Cleanup {
184+
ClassDecl *actorDecl;
185+
SILValue self;
186+
public:
187+
ResignIdentity(ClassDecl *actorDecl, SILValue self)
188+
: actorDecl(actorDecl), self(self) {
189+
assert(actorDecl->isDistributedActor());
190+
}
191+
192+
void emit(SILGenFunction &SGF, CleanupLocation l, ForUnwind_t forUnwind) override {
193+
if (forUnwind == IsForUnwind) {
194+
l.markAutoGenerated();
195+
SGF.emitResignIdentityCall(l, actorDecl,
196+
ManagedValue::forUnmanaged(self));
197+
}
198+
}
199+
200+
void dump(SILGenFunction &SGF) const override {
201+
#ifndef NDEBUG
202+
llvm::errs() << "ResignIdentity "
203+
<< "State:" << getState() << " "
204+
<< "Self: " << self << "\n";
205+
#endif
206+
}
207+
};
208+
} // end anonymous namespace
209+
181210
void SILGenFunction::emitDistActorImplicitPropertyInits(
182211
ConstructorDecl *ctor, ManagedValue selfArg) {
183212
// Only designated initializers should perform this initialization.
@@ -189,6 +218,10 @@ void SILGenFunction::emitDistActorImplicitPropertyInits(
189218
selfArg = selfArg.borrow(*this, loc);
190219
emitTransportInit(*this, ctor, loc, selfArg);
191220
emitIdentityInit(*this, ctor, loc, selfArg);
221+
222+
// register a clean-up to resign the identity upon abnormal exit
223+
auto *actorDecl = cast<ClassDecl>(ctor->getParent()->getAsDecl());
224+
Cleanups.pushCleanup<ResignIdentity>(actorDecl, selfArg.getValue());
192225
}
193226

194227
void SILGenFunction::emitDistributedActorReady(

test/Distributed/SIL/distributed_actor_default_init_sil.swift

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,19 @@
22
// REQUIRES: concurrency
33
// REQUIRES: distributed
44

5+
/// The convention in this test is that the Swift declaration comes before its FileCheck lines.
6+
57
import _Distributed
68

79
/// Use the existential wrapper as the default actor transport.
810
typealias DefaultActorTransport = AnyActorTransport
911

1012
class SomeClass {}
1113

14+
enum Err : Error {
15+
case blah
16+
}
17+
1218
distributed actor MyDistActor {
1319
var localOnlyField: SomeClass
1420

@@ -64,6 +70,8 @@ distributed actor MyDistActor {
6470
// CHECK: br [[RET_BB:bb[0-9]+]]
6571

6672
// CHECK: [[FAIL_BB]]:
73+
// CHECK: [[RESIGN_FN:%[0-9]+]] = witness_method $AnyActorTransport, #ActorTransport.resignIdentity
74+
// CHECK: = apply [[RESIGN_FN]]
6775
// CHECK: builtin "destroyDefaultActor"
6876
// CHECK: br [[RET_BB]]
6977

@@ -73,6 +81,60 @@ distributed actor MyDistActor {
7381

7482

7583

84+
init?(transport_async_fail: AnyActorTransport, cond: Bool) async {
85+
guard cond else { return nil }
86+
self.localOnlyField = SomeClass()
87+
}
88+
89+
// CHECK-LABEL: sil hidden{{.*}} @$s4test11MyDistActorC20transport_async_fail4condACSg12_Distributed03AnyD9TransportV_SbtYacfc : $@convention(method) @async (@in AnyActorTransport, Bool, @owned MyDistActor) -> @owned Optional<MyDistActor> {
90+
// CHECK: bb0([[TPORT:%[0-9]+]] : $*AnyActorTransport, [[COND:%[0-9]+]] : $Bool, [[SELF:%[0-9]+]] : $MyDistActor):
91+
// CHECK: cond_br {{%[0-9]+}}, [[SUCCESS_BB:bb[0-9]+]], [[FAIL_BB:bb[0-9]+]]
92+
93+
// CHECK: [[SUCCESS_BB]]:
94+
// CHECK: hop_to_executor {{%[0-9]+}}
95+
// CHECK: [[READY_FN:%[0-9]+]] = witness_method $AnyActorTransport, #ActorTransport.actorReady
96+
// CHECK: = apply [[READY_FN]]
97+
// CHECK: br [[RET_BB:bb[0-9]+]]
98+
99+
// CHECK: [[FAIL_BB]]:
100+
// CHECK: [[RESIGN_FN:%[0-9]+]] = witness_method $AnyActorTransport, #ActorTransport.resignIdentity
101+
// CHECK: = apply [[RESIGN_FN]]
102+
// CHECK: builtin "destroyDefaultActor"
103+
// CHECK: br [[RET_BB]]
104+
105+
// CHECK: [[RET_BB]]({{%[0-9]+}} : $Optional<MyDistActor>):
106+
// CHECK: return
107+
// CHECK: } // end sil function '$s4test11MyDistActorC20transport_async_fail4condACSg12_Distributed03AnyD9TransportV_SbtYacfc'
108+
109+
110+
111+
init?(transport_async_fail_throws: AnyActorTransport, cond: Bool) async throws {
112+
guard cond else { throw Err.blah }
113+
self.localOnlyField = SomeClass()
114+
}
115+
116+
// CHECK-LABEL: sil hidden @$s4test11MyDistActorC27transport_async_fail_throws4condACSg12_Distributed03AnyD9TransportV_SbtYaKcfc : $@convention(method) @async (@in AnyActorTransport, Bool, @owned MyDistActor) -> (@owned Optional<MyDistActor>, @error Error) {
117+
// CHECK: bb0([[TPORT:%[0-9]+]] : $*AnyActorTransport, [[COND:%[0-9]+]] : $Bool, [[SELF:%[0-9]+]] : $MyDistActor):
118+
// CHECK: cond_br {{%[0-9]+}}, [[SUCCESS_BB:bb[0-9]+]], [[FAIL_BB:bb[0-9]+]]
119+
120+
// CHECK: [[SUCCESS_BB]]:
121+
// CHECK: hop_to_executor {{%[0-9]+}}
122+
// CHECK: [[READY_FN:%[0-9]+]] = witness_method $AnyActorTransport, #ActorTransport.actorReady
123+
// CHECK: = apply [[READY_FN]]
124+
// CHECK: br [[RET_BB:bb[0-9]+]]
125+
126+
// CHECK: [[FAIL_BB]]:
127+
// CHECK: [[RESIGN_FN:%[0-9]+]] = witness_method $AnyActorTransport, #ActorTransport.resignIdentity
128+
// CHECK: = apply [[RESIGN_FN]]
129+
// CHECK: builtin "destroyDefaultActor"
130+
// CHECK: throw {{%[0-9]+}} : $Error
131+
132+
// CHECK: [[RET_BB]]:
133+
// CHECK: return
134+
// CHECK: } // end sil function '$s4test11MyDistActorC27transport_async_fail_throws4condACSg12_Distributed03AnyD9TransportV_SbtYaKcfc'
135+
136+
137+
76138
init(transport_async: AnyActorTransport, cond: Bool) async {
77139
if cond {
78140
self.localOnlyField = SomeClass()
@@ -125,5 +187,3 @@ distributed actor MyDistActor {
125187

126188

127189
}
128-
129-

0 commit comments

Comments
 (0)