Skip to content

Commit 856be9f

Browse files
authored
Merge pull request #73602 from gottesmm/pr-22e1094e1247a2340184e3b4795c4037ae10bc3f
[region-isolation] When computing isolation for isolated parameters, use getAnyActor instead of getNominalOrBoundGenericNominal().
2 parents 3dffcc6 + fe2dc11 commit 856be9f

File tree

5 files changed

+64
-35
lines changed

5 files changed

+64
-35
lines changed

include/swift/SILOptimizer/Utils/PartitionUtils.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,10 @@ class SILIsolationInfo {
192192
actorInstance(ActorInstance::getForValue(actorInstance)) {
193193
assert((!actorInstance ||
194194
(actorIsolation.getKind() == ActorIsolation::ActorInstance &&
195-
actorInstance->getType().isAnyActor())) &&
195+
actorInstance->getType()
196+
.getASTType()
197+
->lookThroughAllOptionalTypes()
198+
->getAnyActor())) &&
196199
"actorInstance must be an actor if it is non-empty");
197200
}
198201

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6574,36 +6574,6 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
65746574
})),
65756575
"transferring result means all results are transferring");
65766576

6577-
// We should only ever have a single sil_isolated parameter.
6578-
bool foundIsolatedParameter = false;
6579-
for (const auto &parameterInfo : FTy->getParameters()) {
6580-
if (parameterInfo.hasOption(SILParameterInfo::Isolated)) {
6581-
auto argType = parameterInfo.getArgumentType(F.getModule(),
6582-
FTy,
6583-
F.getTypeExpansionContext());
6584-
if (argType->isOptional())
6585-
argType = argType->lookThroughAllOptionalTypes()->getCanonicalType();
6586-
6587-
auto genericSig = FTy->getInvocationGenericSignature();
6588-
auto &ctx = F.getASTContext();
6589-
auto *actorProtocol = ctx.getProtocol(KnownProtocolKind::Actor);
6590-
auto *anyActorProtocol = ctx.getProtocol(KnownProtocolKind::AnyActor);
6591-
bool genericTypeWithActorRequirement = llvm::any_of(
6592-
genericSig.getRequirements(), [&](const Requirement &other) {
6593-
if (other.getKind() != RequirementKind::Conformance)
6594-
return false;
6595-
if (other.getFirstType()->getCanonicalType() != argType)
6596-
return false;
6597-
auto *otherProtocol = other.getProtocolDecl();
6598-
return otherProtocol->inheritsFrom(actorProtocol) ||
6599-
otherProtocol->inheritsFrom(anyActorProtocol);
6600-
});
6601-
require(argType->isAnyActorType() || genericTypeWithActorRequirement,
6602-
"Only any actor types can be isolated");
6603-
require(!foundIsolatedParameter, "Two isolated parameters");
6604-
foundIsolatedParameter = true;
6605-
}
6606-
}
66076577
require(1 >= std::count_if(FTy->getParameters().begin(), FTy->getParameters().end(),
66086578
[](const SILParameterInfo &parameterInfo) {
66096579
return parameterInfo.hasOption(SILParameterInfo::Isolated);
@@ -7014,11 +6984,37 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
70146984
}
70156985
}
70166986

6987+
void verifyParentFunctionSILFunctionType(CanSILFunctionType FTy) {
6988+
bool foundIsolatedParameter = false;
6989+
for (const auto &parameterInfo : FTy->getParameters()) {
6990+
if (parameterInfo.hasOption(SILParameterInfo::Isolated)) {
6991+
auto argType = parameterInfo.getArgumentType(F.getModule(),
6992+
FTy,
6993+
F.getTypeExpansionContext());
6994+
6995+
if (argType->isOptional())
6996+
argType = argType->lookThroughAllOptionalTypes()->getCanonicalType();
6997+
6998+
auto genericSig = FTy->getInvocationGenericSignature();
6999+
auto &ctx = F.getASTContext();
7000+
auto *actorProtocol = ctx.getProtocol(KnownProtocolKind::Actor);
7001+
auto *anyActorProtocol = ctx.getProtocol(KnownProtocolKind::AnyActor);
7002+
require(argType->isAnyActorType() ||
7003+
genericSig->requiresProtocol(argType, actorProtocol) ||
7004+
genericSig->requiresProtocol(argType, anyActorProtocol),
7005+
"Only any actor types can be isolated");
7006+
require(!foundIsolatedParameter, "Two isolated parameters");
7007+
foundIsolatedParameter = true;
7008+
}
7009+
}
7010+
}
7011+
70177012
void visitSILFunction(SILFunction *F) {
70187013
PrettyStackTraceSILFunction stackTrace("verifying", F);
70197014

70207015
CanSILFunctionType FTy = F->getLoweredFunctionType();
70217016
verifySILFunctionType(FTy);
7017+
verifyParentFunctionSILFunctionType(FTy);
70227018

70237019
SILModule &mod = F->getModule();
70247020
bool embedded = mod.getASTContext().LangOpts.hasFeature(Feature::Embedded);

lib/SILOptimizer/Utils/PartitionUtils.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,9 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
113113
auto &self = fas.getSelfArgumentOperand();
114114
if (fas.getArgumentParameterInfo(self).hasOption(
115115
SILParameterInfo::Isolated)) {
116+
CanType astType = self.get()->getType().getASTType();
116117
if (auto *nomDecl =
117-
self.get()->getType().getNominalOrBoundGenericNominal()) {
118+
astType->lookThroughAllOptionalTypes()->getAnyActor()) {
118119
// TODO: We really should be doing this based off of an Operand. Then
119120
// we would get the SILValue() for the first element. Today this can
120121
// only mess up isolation history.
@@ -416,8 +417,8 @@ SILIsolationInfo SILIsolationInfo::get(SILArgument *arg) {
416417
// Before we do anything further, see if we have an isolated parameter. This
417418
// handles isolated self and specifically marked isolated.
418419
if (auto *isolatedArg = fArg->getFunction()->maybeGetIsolatedArgument()) {
419-
if (auto *nomDecl =
420-
isolatedArg->getType().getNominalOrBoundGenericNominal()) {
420+
auto astType = isolatedArg->getType().getASTType();
421+
if (auto *nomDecl = astType->lookThroughAllOptionalTypes()->getAnyActor()) {
421422
return SILIsolationInfo::getActorInstanceIsolated(fArg, isolatedArg,
422423
nomDecl);
423424
}

test/Concurrency/transfernonsendable.swift

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
////////////////////////
1616

1717
/// Classes are always non-sendable, so this is non-sendable
18-
class NonSendableKlass { // expected-complete-note 48{{}}
18+
class NonSendableKlass { // expected-complete-note 49{{}}
1919
// expected-typechecker-only-note @-1 4{{}}
2020
// expected-tns-note @-2 2{{}}
2121
var field: NonSendableKlass? = nil
@@ -1698,3 +1698,16 @@ func differentInstanceTest(_ a: MyActor, _ b: MyActor) async {
16981698
await b.useKlass(x) // expected-tns-note {{access can happen concurrently}}
16991699
// expected-complete-warning @-1 {{passing argument of non-sendable type 'NonSendableKlass' into actor-isolated context may introduce data races}}
17001700
}
1701+
1702+
protocol AssociatedTypeTestProtocol {
1703+
associatedtype A: Actor
1704+
}
1705+
1706+
func associatedTypeTestBasic<T: AssociatedTypeTestProtocol>(_: T, _: isolated T.A) {
1707+
}
1708+
1709+
func associatedTypeTestBasic2<T: AssociatedTypeTestProtocol>(_: T, iso: isolated T.A, x: NonSendableKlass) async {
1710+
await transferToMain(x) // expected-tns-warning {{sending 'x' risks causing data races}}
1711+
// expected-tns-note @-1 {{sending 'iso'-isolated 'x' to main actor-isolated global function 'transferToMain' risks causing data races between main actor-isolated and 'iso'-isolated uses}}
1712+
// expected-complete-warning @-2 {{passing argument of non-sendable type 'NonSendableKlass' into main actor-isolated context may introduce data races}}
1713+
}

test/Distributed/distributed_actor_transfernonsendable.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,3 +66,19 @@ distributed actor MyDistributedActor {
6666
}
6767
}
6868
}
69+
70+
/////////////////////////////////
71+
// MARK: Associated Type Tests //
72+
/////////////////////////////////
73+
74+
protocol AssociatedTypeTestProtocol {
75+
associatedtype A: DistributedActor
76+
}
77+
78+
func associatedTypeTestBasic<T: AssociatedTypeTestProtocol>(_: T, _: isolated T.A) {
79+
}
80+
81+
func associatedTypeTestBasic2<T: AssociatedTypeTestProtocol>(_: T, iso: isolated T.A, x: NonSendableKlass) async {
82+
await transferToMain(x) // expected-error {{sending 'x' risks causing data races}}
83+
// expected-note @-1 {{sending 'iso'-isolated 'x' to main actor-isolated global function 'transferToMain' risks causing data races between main actor-isolated and 'iso'-isolated uses}}
84+
}

0 commit comments

Comments
 (0)