Skip to content

Commit 43b9b28

Browse files
rjmccallrjmccall
committed
Fix a pair of bugs with @isolated(any) closure erasure.
First, and I really should've checked this, but global actors do not require `shared` to return `Self`; adjust the logic to propagate the right formal type to the erasure logic. Second, handle attempts to erase the isolation of something isolated to a distributed actor using the magic Actor conformance that Doug added. This only works when the actor is local, but it shouldn't be difficult to enforce that we only attempt to erase isolation what that's true --- we need to prevent partial application of distributed actors, and we need to disallow explicit isolated captures of distributed actors when we're not currently isolated to it. Otherwise, it's not possible to get an @isolated(any) function value with an isolation that isn't the current function's isolation, which means it always has to be local. Fixed rdar://123734019
1 parent e98186b commit 43b9b28

File tree

5 files changed

+207
-78
lines changed

5 files changed

+207
-78
lines changed

‎lib/SILGen/SILGenBuiltin.cpp

Lines changed: 1 addition & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1919,54 +1919,6 @@ static ManagedValue emitBuiltinInjectEnumTag(SILGenFunction &SGF, SILLocation lo
19191919
return ManagedValue::forObjectRValueWithoutOwnership(bi);
19201920
}
19211921

1922-
/// Find the extension on DistributedActor that defines __actorUnownedExecutor.
1923-
static ExtensionDecl *findDistributedActorAsActorExtension(
1924-
ProtocolDecl *distributedActorProto, ModuleDecl *module) {
1925-
ASTContext &ctx = distributedActorProto->getASTContext();
1926-
auto name = ctx.getIdentifier("__actorUnownedExecutor");
1927-
auto results = distributedActorProto->lookupDirect(
1928-
name, SourceLoc(),
1929-
NominalTypeDecl::LookupDirectFlags::IncludeAttrImplements);
1930-
for (auto result : results) {
1931-
if (auto var = dyn_cast<VarDecl>(result)) {
1932-
return dyn_cast<ExtensionDecl>(var->getDeclContext());
1933-
}
1934-
}
1935-
1936-
return nullptr;
1937-
}
1938-
1939-
ProtocolConformanceRef
1940-
SILGenModule::getDistributedActorAsActorConformance(SubstitutionMap subs) {
1941-
ASTContext &ctx = M.getASTContext();
1942-
auto actorProto = ctx.getProtocol(KnownProtocolKind::Actor);
1943-
Type distributedActorType = subs.getReplacementTypes()[0];
1944-
1945-
if (!distributedActorAsActorConformance) {
1946-
auto distributedActorProto = ctx.getProtocol(KnownProtocolKind::DistributedActor);
1947-
if (!distributedActorProto)
1948-
return ProtocolConformanceRef();
1949-
1950-
auto ext = findDistributedActorAsActorExtension(
1951-
distributedActorProto, M.getSwiftModule());
1952-
if (!ext)
1953-
return ProtocolConformanceRef();
1954-
1955-
// Conformance of DistributedActor to Actor.
1956-
auto genericParam = subs.getGenericSignature().getGenericParams()[0];
1957-
distributedActorAsActorConformance = ctx.getNormalConformance(
1958-
Type(genericParam), actorProto, SourceLoc(), ext,
1959-
ProtocolConformanceState::Incomplete, /*isUnchecked=*/false,
1960-
/*isPreconcurrency=*/false);
1961-
}
1962-
1963-
return ProtocolConformanceRef(
1964-
actorProto,
1965-
ctx.getSpecializedConformance(distributedActorType,
1966-
distributedActorAsActorConformance,
1967-
subs));
1968-
}
1969-
19701922
void SILGenModule::noteMemberRefExpr(MemberRefExpr *e) {
19711923
VarDecl *var = cast<VarDecl>(e->getMember().getDecl());
19721924

@@ -1989,25 +1941,7 @@ void SILGenModule::noteMemberRefExpr(MemberRefExpr *e) {
19891941
static ManagedValue emitBuiltinDistributedActorAsAnyActor(
19901942
SILGenFunction &SGF, SILLocation loc, SubstitutionMap subs,
19911943
ArrayRef<ManagedValue> args, SGFContext C) {
1992-
auto &ctx = SGF.getASTContext();
1993-
auto distributedActor = args[0];
1994-
ProtocolConformanceRef conformances[1] = {
1995-
SGF.SGM.getDistributedActorAsActorConformance(subs)
1996-
};
1997-
1998-
// Erase the distributed actor instance into an `any Actor` existential with
1999-
// the special conformance.
2000-
CanType distributedActorType =
2001-
subs.getReplacementTypes()[0]->getCanonicalType();
2002-
auto &distributedActorTL = SGF.getTypeLowering(distributedActorType);
2003-
auto actorProto = ctx.getProtocol(KnownProtocolKind::Actor);
2004-
auto &anyActorTL = SGF.getTypeLowering(actorProto->getDeclaredExistentialType());
2005-
return SGF.emitExistentialErasure(
2006-
loc, distributedActorType, distributedActorTL, anyActorTL,
2007-
ctx.AllocateCopy(conformances),
2008-
C, [&distributedActor](SGFContext) {
2009-
return distributedActor;
2010-
});
1944+
return SGF.emitDistributedActorAsAnyActor(loc, subs, args[0]);
20111945
}
20121946

20131947
std::optional<SpecializedEmitter>

‎lib/SILGen/SILGenConcurrency.cpp

Lines changed: 125 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "RValue.h"
1616
#include "Scope.h"
1717
#include "swift/AST/ASTContext.h"
18+
#include "swift/AST/ProtocolConformance.h"
1819
#include "swift/Basic/Range.h"
1920

2021
using namespace swift;
@@ -242,11 +243,12 @@ ManagedValue SILGenFunction::emitNonIsolatedIsolation(SILLocation loc) {
242243

243244
SILValue SILGenFunction::emitLoadGlobalActorExecutor(Type globalActor) {
244245
auto loc = RegularLocation::getAutoGeneratedLocation(F.getLocation());
245-
auto actor = emitLoadOfGlobalActorShared(loc, globalActor->getCanonicalType());
246-
return emitLoadActorExecutor(loc, actor);
246+
auto actorAndFormalType =
247+
emitLoadOfGlobalActorShared(loc, globalActor->getCanonicalType());
248+
return emitLoadActorExecutor(loc, actorAndFormalType.first);
247249
}
248250

249-
ManagedValue
251+
std::pair<ManagedValue, CanType>
250252
SILGenFunction::emitLoadOfGlobalActorShared(SILLocation loc, CanType actorType) {
251253
NominalTypeDecl *nominal = actorType->getNominalOrBoundGenericNominal();
252254
VarDecl *sharedInstanceDecl = nominal->getGlobalActorInstance();
@@ -270,18 +272,53 @@ SILGenFunction::emitLoadOfGlobalActorShared(SILLocation loc, CanType actorType)
270272
actorMetaType, /*isSuper*/ false, sharedInstanceDecl, PreparedArguments(),
271273
subs, AccessSemantics::Ordinary, instanceType, SGFContext());
272274
ManagedValue actorInstance = std::move(actorInstanceRV).getScalarValue();
273-
return actorInstance;
275+
return {actorInstance, instanceType->getCanonicalType()};
274276
}
275277

276278
ManagedValue
277279
SILGenFunction::emitGlobalActorIsolation(SILLocation loc,
278280
CanType globalActorType) {
279-
// GlobalActor.shared returns Self, so this should be a value of
280-
// GlobalActor type.
281-
auto actor = emitLoadOfGlobalActorShared(loc, globalActorType);
281+
// Load the .shared property. Note that this isn't necessarily a value
282+
// of the global actor type.
283+
auto actorAndFormalType = emitLoadOfGlobalActorShared(loc, globalActorType);
282284

283285
// Since it's just a normal actor instance, we can use the normal path.
284-
return emitActorInstanceIsolation(loc, actor, globalActorType);
286+
return emitActorInstanceIsolation(loc, actorAndFormalType.first,
287+
actorAndFormalType.second);
288+
}
289+
290+
/// Given a value of some non-optional distributed actor type, convert it
291+
/// to the non-optional `any Actor` type.
292+
static ManagedValue
293+
emitDistributedActorIsolation(SILGenFunction &SGF, SILLocation loc,
294+
ManagedValue actor, CanType actorType) {
295+
// First, open the actor type if it's an existential type.
296+
if (actorType->isExistentialType()) {
297+
CanType openedType = OpenedArchetypeType::getAny(actorType,
298+
SGF.F.getGenericSignature());
299+
SILType loweredOpenedType = SGF.getLoweredType(openedType);
300+
301+
actor = SGF.emitOpenExistential(loc, actor, loweredOpenedType,
302+
AccessKind::Read);
303+
actorType = openedType;
304+
}
305+
306+
auto &ctx = SGF.getASTContext();
307+
auto distributedActorProto =
308+
ctx.getProtocol(KnownProtocolKind::DistributedActor);
309+
310+
// Build <T: DistributedActor> and its substitutions for actorType.
311+
// Doing this manually is ill-advised in general, but this is such a
312+
// simple case that it's okay.
313+
auto sig = distributedActorProto->getGenericSignature();
314+
auto distributedActorConf =
315+
SGF.SGM.SwiftModule->lookupConformance(actorType, distributedActorProto);
316+
auto distributedActorSubs = SubstitutionMap::get(sig, {actorType},
317+
{distributedActorConf});
318+
319+
// Use that to build the magical conformance to Actor for the distributed
320+
// actor type.
321+
return SGF.emitDistributedActorAsAnyActor(loc, distributedActorSubs, actor);
285322
}
286323

287324
/// Given a value of some non-optional actor type, convert it to
@@ -295,6 +332,13 @@ emitNonOptionalActorInstanceIsolation(SILGenFunction &SGF, SILLocation loc,
295332
return actor;
296333

297334
CanType anyActorType = anyActorTy.getASTType();
335+
336+
// If the actor is a distributed actor, (1) it had better be local
337+
// and (2) we need to use the special conformance.
338+
if (actorType->isDistributedActor()) {
339+
return emitDistributedActorIsolation(SGF, loc, actor, actorType);
340+
}
341+
298342
return SGF.emitTransformExistential(loc, actor, actorType, anyActorType);
299343
}
300344

@@ -587,3 +631,76 @@ SILValue SILGenFunction::emitGetCurrentExecutor(SILLocation loc) {
587631
assert(ExpectedExecutor && "prolog failed to set up expected executor?");
588632
return ExpectedExecutor;
589633
}
634+
635+
/// Find the extension on DistributedActor that defines __actorUnownedExecutor.
636+
static ExtensionDecl *findDistributedActorAsActorExtension(
637+
ProtocolDecl *distributedActorProto, ModuleDecl *module) {
638+
ASTContext &ctx = distributedActorProto->getASTContext();
639+
auto name = ctx.getIdentifier("__actorUnownedExecutor");
640+
auto results = distributedActorProto->lookupDirect(
641+
name, SourceLoc(),
642+
NominalTypeDecl::LookupDirectFlags::IncludeAttrImplements);
643+
for (auto result : results) {
644+
if (auto var = dyn_cast<VarDecl>(result)) {
645+
return dyn_cast<ExtensionDecl>(var->getDeclContext());
646+
}
647+
}
648+
649+
return nullptr;
650+
}
651+
652+
ProtocolConformanceRef
653+
SILGenModule::getDistributedActorAsActorConformance(SubstitutionMap subs) {
654+
ASTContext &ctx = M.getASTContext();
655+
auto actorProto = ctx.getProtocol(KnownProtocolKind::Actor);
656+
Type distributedActorType = subs.getReplacementTypes()[0];
657+
658+
if (!distributedActorAsActorConformance) {
659+
auto distributedActorProto = ctx.getProtocol(KnownProtocolKind::DistributedActor);
660+
if (!distributedActorProto)
661+
return ProtocolConformanceRef();
662+
663+
auto ext = findDistributedActorAsActorExtension(
664+
distributedActorProto, M.getSwiftModule());
665+
if (!ext)
666+
return ProtocolConformanceRef();
667+
668+
// Conformance of DistributedActor to Actor.
669+
auto genericParam = subs.getGenericSignature().getGenericParams()[0];
670+
distributedActorAsActorConformance = ctx.getNormalConformance(
671+
Type(genericParam), actorProto, SourceLoc(), ext,
672+
ProtocolConformanceState::Incomplete, /*isUnchecked=*/false,
673+
/*isPreconcurrency=*/false);
674+
}
675+
676+
return ProtocolConformanceRef(
677+
actorProto,
678+
ctx.getSpecializedConformance(distributedActorType,
679+
distributedActorAsActorConformance,
680+
subs));
681+
}
682+
683+
ManagedValue
684+
SILGenFunction::emitDistributedActorAsAnyActor(SILLocation loc,
685+
SubstitutionMap distributedActorSubs,
686+
ManagedValue actorValue) {
687+
ProtocolConformanceRef conformances[1] = {
688+
SGM.getDistributedActorAsActorConformance(distributedActorSubs)
689+
};
690+
691+
// Erase the distributed actor instance into an `any Actor` existential with
692+
// the special conformance.
693+
auto &ctx = SGM.getASTContext();
694+
CanType distributedActorType =
695+
distributedActorSubs.getReplacementTypes()[0]->getCanonicalType();
696+
auto &distributedActorTL = getTypeLowering(distributedActorType);
697+
auto actorProto = ctx.getProtocol(KnownProtocolKind::Actor);
698+
auto &anyActorTL = getTypeLowering(actorProto->getDeclaredExistentialType());
699+
return emitExistentialErasure(loc, distributedActorType,
700+
distributedActorTL, anyActorTL,
701+
ctx.AllocateCopy(conformances),
702+
SGFContext(),
703+
[actorValue](SGFContext) {
704+
return actorValue;
705+
});
706+
}

‎lib/SILGen/SILGenFunction.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -932,6 +932,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
932932
ArgumentSource &&fnSource,
933933
SGFContext C);
934934

935+
ManagedValue emitDistributedActorAsAnyActor(SILLocation loc,
936+
SubstitutionMap distributedActorSubs,
937+
ManagedValue actor);
938+
935939
/// Generate a nullary function that returns the given value.
936940
/// If \p emitProfilerIncrement is set, emit a profiler increment for
937941
/// \p value.
@@ -1182,8 +1186,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
11821186
SILValue emitLoadGlobalActorExecutor(Type globalActor);
11831187

11841188
/// Call `.shared` on the given global actor type.
1185-
ManagedValue emitLoadOfGlobalActorShared(SILLocation loc,
1186-
CanType globalActorType);
1189+
///
1190+
/// Returns the value of the property and the formal instance type.
1191+
std::pair<ManagedValue, CanType>
1192+
emitLoadOfGlobalActorShared(SILLocation loc, CanType globalActorType);
11871193

11881194
/// Emit a reference to the given global actor as an opaque isolation.
11891195
ManagedValue emitGlobalActorIsolation(SILLocation loc,

‎test/Distributed/distributed_actor_isolated_any.swift

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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+
4+
// RUN: %target-swift-frontend -emit-silgen -enable-experimental-feature IsolatedAny %s -module-name test -swift-version 5 -disable-availability-checking -I %t | %FileCheck %s
5+
6+
// REQUIRES: concurrency
7+
// REQUIRES: asserts
8+
// REQUIRES: distributed
9+
10+
import Distributed
11+
import FakeDistributedActorSystems
12+
13+
typealias DefaultDistributedActorSystem = FakeActorSystem
14+
15+
func takeInheritingAsyncIsolatedAny(@_inheritActorContext fn: @escaping @isolated(any) () async -> ()) {}
16+
17+
// CHECK-LABEL: sil hidden [distributed] [ossa] @$s4test2DAC0A20DistributedIsolationyyF
18+
// CHECK: // function_ref closure #1
19+
// CHECK-NEXT: [[CLOSURE_FN:%.*]] = function_ref @$s4test2DAC0A20DistributedIsolationyyFyyYacfU_ : $@convention(thin) @async (@guaranteed Optional<any Actor>, @sil_isolated @guaranteed DA) -> ()
20+
// CHECK-NEXT: [[CAPTURE:%.*]] = copy_value %0 : $DA
21+
// CHECK-NEXT: [[CAPTURE_FOR_ISOLATION:%.*]] = copy_value [[CAPTURE]] : $DA
22+
// The conformance here is special, but we don't record that in the printed SIL.
23+
// CHECK-NEXT: [[ISOLATION_OBJECT:%.*]] = init_existential_ref [[CAPTURE_FOR_ISOLATION]] : $DA : $DA, $any Actor
24+
// CHECK-NEXT: [[ISOLATION:%.*]] = enum $Optional<any Actor>, #Optional.some!enumelt, [[ISOLATION_OBJECT]] : $any Actor
25+
// CHECK-NEXT: [[CLOSURE:%.*]] = partial_apply [callee_guaranteed] [isolated_any] [[CLOSURE_FN]]([[ISOLATION]], [[CAPTURE]])
26+
// CHECK-NEXT: // function_ref
27+
// CHECK-NEXT: [[TAKE_FN:%.*]] = function_ref @$s4test30takeInheritingAsyncIsolatedAny2fnyyyYaYAc_tF
28+
// CHECK-NEXT: apply [[TAKE_FN]]([[CLOSURE]])
29+
// CHECK-NEXT: destroy_value [[CLOSURE]]
30+
distributed actor DA {
31+
distributed func testDistributedIsolation() {
32+
takeInheritingAsyncIsolatedAny {
33+
await self.asyncAction()
34+
}
35+
}
36+
37+
func asyncAction() async {}
38+
}

‎test/SILGen/isolated_any.swift

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,41 @@ func testEraseInheritingAsyncNonIsolatedClosure() {
325325
// CHECK-NEXT: return
326326
@MainActor
327327
func testEraseInheritingAsyncMainActorClosure() {
328-
takeInheritingAsyncIsolatedAny { @MainActor in
328+
takeInheritingAsyncIsolatedAny {
329+
await asyncAction()
330+
}
331+
}
332+
333+
// Define a global actor that doesn't use Self as its instance type
334+
actor MyGlobalActorInstance {}
335+
@globalActor struct MyGlobalActor {
336+
// Make sure this doesn't confuse things.
337+
let shared = 0
338+
339+
static let shared = MyGlobalActorInstance()
340+
}
341+
342+
// CHECK-LABEL: sil hidden [ossa] @$s4test0A38EraseInheritingAsyncGlobalActorClosureyyF
343+
// CHECK: // function_ref closure #1
344+
// CHECK-NEXT: [[CLOSURE_FN:%.*]] = function_ref @$s4test0A38EraseInheritingAsyncGlobalActorClosureyyFyyYacfU_ :
345+
// CHECK-NEXT: [[GLOBAL_ACTOR_METATYPE:%.*]] = metatype $@thin MyGlobalActor.Type
346+
// CHECK-NEXT: // function_ref
347+
// CHECK-NEXT: [[GLOBAL_ACTOR_SHARED_FN:%.]] = function_ref @$s4test13MyGlobalActorV6sharedAA0bcD8InstanceCvau :
348+
// CHECK-NEXT: [[GLOBAL_ACTOR_PTR:%.*]] = apply [[GLOBAL_ACTOR_SHARED_FN]]()
349+
// CHECK-NEXT: [[GLOBAL_ACTOR_ADDR:%.*]] = pointer_to_address [[GLOBAL_ACTOR_PTR]] : $Builtin.RawPointer to [strict] $*MyGlobalActorInstance
350+
// CHECK-NEXT: [[GLOBAL_ACTOR:%.*]] = load [copy] [[GLOBAL_ACTOR_ADDR]] : $*MyGlobalActorInstance
351+
// CHECK-NEXT: [[ERASED_GLOBAL_ACTOR:%.*]] = init_existential_ref [[GLOBAL_ACTOR]] :
352+
// CHECK-NEXT: [[ISOLATION:%.*]] = enum $Optional<any Actor>, #Optional.some!enumelt, [[ERASED_GLOBAL_ACTOR]] : $any Actor
353+
// CHECK-NEXT: [[CLOSURE:%.*]] = partial_apply [callee_guaranteed] [isolated_any] [[CLOSURE_FN]]([[ISOLATION]])
354+
// CHECK-NEXT: // function_ref
355+
// CHECK-NEXT: [[TAKE_FN:%.*]] = function_ref @$s4test30takeInheritingAsyncIsolatedAny2fnyyyYaYAc_tF
356+
// CHECK-NEXT: apply [[TAKE_FN]]([[CLOSURE]])
357+
// CHECK-NEXT: destroy_value [[CLOSURE]]
358+
// CHECK-NEXT: tuple ()
359+
// CHECK-NEXT: return
360+
@MyGlobalActor
361+
func testEraseInheritingAsyncGlobalActorClosure() {
362+
takeInheritingAsyncIsolatedAny {
329363
await asyncAction()
330364
}
331365
}

0 commit comments

Comments
 (0)