Skip to content

Commit 8016bf2

Browse files
committed
[sil] Change SILIsolationInfo inference for classmethods to use SILDeclRef instead of using the AST directly.
We are creating/relying on a contract between the AST and SIL... that SILDeclRef should accurately describe the method/accessor that a class_method is from. By doing this we eliminate pattern matching on the AST which ties this code too tightly to the AST and makes it brittle in the face of AST changes. This also fixes an issue where we were not handling setters correctly. I am doing this now since it is natural to fix it along side fixing the ref_element_addr issue in the previous commit since they are effectively doing the same thing. rdar://153207557
1 parent dfbe7c1 commit 8016bf2

File tree

5 files changed

+807
-96
lines changed

5 files changed

+807
-96
lines changed

include/swift/SIL/SILArgument.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,12 @@ class SILFunctionArgument : public SILArgument {
429429
/// Returns true if this SILFunctionArgument is an 'inout sending' parameter.
430430
bool isInOutSending() const;
431431

432+
bool isIsolated() const {
433+
return !isIndirectResult() && !isIndirectErrorResult() &&
434+
getKnownParameterInfo().getOptions().contains(
435+
SILParameterInfo::Isolated);
436+
}
437+
432438
Lifetime getLifetime() const {
433439
return getType()
434440
.getLifetime(*getFunction())

include/swift/SILOptimizer/Utils/SILIsolationInfo.h

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,16 @@ class ActorInstance {
5353
/// function)... so we just use an artificial ActorInstance to represent
5454
/// self in this case.
5555
CapturedActorSelf = 0x2,
56+
57+
/// An actor instance in an async allocating init where we are going to
58+
/// allocate the actual actor internally. This is considered to be isolated
59+
/// to the actor instance.
60+
ActorAsyncAllocatingInit = 0x3,
5661
};
5762

58-
/// Set to (SILValue(), $KIND) if we have an ActorAccessorInit|CapturedSelf.
59-
/// Is null if we have (SILValue(), Kind::Value).
63+
/// Set to (SILValue(), $KIND) if we have an
64+
/// ActorAccessorInit|CapturedSelf|ActorAsyncAllocatingInit. Is null if we
65+
/// have (SILValue(), Kind::Value).
6066
llvm::PointerIntPair<SILValue, 2> value;
6167

6268
ActorInstance(SILValue value, Kind kind)
@@ -94,6 +100,12 @@ class ActorInstance {
94100
return ActorInstance(SILValue(), Kind::CapturedActorSelf);
95101
}
96102

103+
/// See Kind::ActorAsyncAllocatingInit for explanation on what a
104+
/// ActorAsyncAllocatingInit is.
105+
static ActorInstance getForActorAsyncAllocatingInit() {
106+
return ActorInstance(SILValue(), Kind::ActorAsyncAllocatingInit);
107+
}
108+
97109
explicit operator bool() const { return bool(value.getOpaqueValue()); }
98110

99111
Kind getKind() const { return Kind(value.getInt()); }
@@ -117,6 +129,10 @@ class ActorInstance {
117129
return getKind() == Kind::CapturedActorSelf;
118130
}
119131

132+
bool isActorAsyncAllocatingInit() const {
133+
return getKind() == Kind::ActorAsyncAllocatingInit;
134+
}
135+
120136
bool operator==(const ActorInstance &other) const {
121137
// If both are null, return true.
122138
if (!bool(*this) && !bool(other))
@@ -132,6 +148,7 @@ class ActorInstance {
132148
return getValue() == other.getValue();
133149
case Kind::ActorAccessorInit:
134150
case Kind::CapturedActorSelf:
151+
case Kind::ActorAsyncAllocatingInit:
135152
return true;
136153
}
137154
}

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

Lines changed: 93 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -426,27 +426,31 @@ static bool isPartialApplyNonisolatedUnsafe(PartialApplyInst *pai) {
426426
return foundOneNonIsolatedUnsafe;
427427
}
428428

429-
/// Return the SILIsolationInfo for a class field.
429+
/// Return the SILIsolationInfo for a class field for a ref_element_addr or
430+
/// class_method. Methods that are direct should get their isolation information
431+
/// from the static function rather than from this function.
430432
///
431433
/// \arg queriedValue the actual value that SILIsolationInfo::get was called
432434
/// upon. This is used for IsolationHistory.
433435
///
434436
/// \arg classValue this is the actual underlying class value that we are
435437
/// extracting a field out of. As an example this is the base passed to
436-
/// ref_element_addr or class_method.
438+
/// ref_element_addr or class_method. This /can/ be a metatype potentially in
439+
/// the case of class 'class' methods and computed properties.
437440
///
438441
/// \arg field the actual AST field that we discovered we are querying. This
439442
/// could be the field of the ref_element_addr or an accessor decl extracted
440443
/// from a SILDeclRef of a class_method.
441444
static SILIsolationInfo computeIsolationForClassField(SILValue queriedValue,
442445
SILValue classValue,
443446
ValueDecl *field) {
447+
// First look for explicit isolation on the field itself. These always
448+
// override what is on the class.
444449
auto varIsolation = swift::getActorIsolation(field);
445450

446-
// If we have a global actor isolated field, then we know that we override
447-
// the actual isolation of the actor or global actor isolated class with
448-
// some other form of isolation. In such a case, we need to use that
449-
// isolation instead.
451+
// If we have an explicitly global actor isolated field, then we must prefer
452+
// that isolation since it takes precedence over any isolation directly on the
453+
// underlying class type.
450454
if (varIsolation.isGlobalActor()) {
451455
assert(!varIsolation.isNonisolatedUnsafe() &&
452456
"Cannot apply both nonisolated(unsafe) and a global actor attribute "
@@ -455,27 +459,74 @@ static SILIsolationInfo computeIsolationForClassField(SILValue queriedValue,
455459
queriedValue, varIsolation.getGlobalActor());
456460
}
457461

462+
// Then check if our field is explicitly nonisolated (not
463+
// nonisolated(unsafe)). In such a case, we want to return disconnected since
464+
// we are overriding the isolation of the actual nominal type. If we have
465+
// nonisolated(unsafe), we want to respect the isolation of the nominal type
466+
// since we just want to squelch the error but still have it be isolated in
467+
// its normal way. This provides more information since we know what the
468+
// underlying isolation /would/ have been.
469+
if (varIsolation.isNonisolated() && !varIsolation.isNonisolatedUnsafe())
470+
return SILIsolationInfo::getDisconnected(false /*nonisolated unsafe*/);
471+
472+
// Ok, we know that we do not have any overriding isolation from the var
473+
// itself... so now we should use isolation from the underlying nominal type.
474+
475+
// First see if we have an actor instance value from an isolated
476+
// SILFunctionArgument.
458477
if (auto instance = ActorInstance::getForValue(classValue)) {
459478
if (auto *fArg = llvm::dyn_cast_or_null<SILFunctionArgument>(
460479
instance.maybeGetValue())) {
461480
if (auto info =
462-
SILIsolationInfo::getActorInstanceIsolated(queriedValue, fArg))
481+
SILIsolationInfo::getActorInstanceIsolated(queriedValue, fArg)) {
463482
return info.withUnsafeNonIsolated(varIsolation.isNonisolatedUnsafe());
483+
}
464484
}
465485
}
466486

467-
auto *nomDecl = classValue->getType().getNominalOrBoundGenericNominal();
487+
// First find out if our classValue is a nominal type and exit early...
488+
if (auto *nomDecl = classValue->getType().getNominalOrBoundGenericNominal()) {
489+
if (auto isolation = swift::getActorIsolation(nomDecl);
490+
isolation && isolation.isGlobalActor()) {
491+
return SILIsolationInfo::getGlobalActorIsolated(
492+
queriedValue, isolation.getGlobalActor())
493+
.withUnsafeNonIsolated(varIsolation.isNonisolatedUnsafe());
494+
}
468495

469-
if (nomDecl->isAnyActor())
470-
return SILIsolationInfo::getActorInstanceIsolated(queriedValue, classValue,
471-
nomDecl)
472-
.withUnsafeNonIsolated(varIsolation.isNonisolatedUnsafe());
496+
if (nomDecl->isAnyActor())
497+
return SILIsolationInfo::getActorInstanceIsolated(queriedValue,
498+
classValue, nomDecl)
499+
.withUnsafeNonIsolated(varIsolation.isNonisolatedUnsafe());
500+
}
501+
502+
// If we have a metatype...
503+
if (classValue->getType().isMetatype()) {
504+
// And we can a class nominal decl...
505+
if (auto *nomDecl =
506+
classValue->getType()
507+
.getLoweredInstanceTypeOfMetatype(classValue->getFunction())
508+
.getNominalOrBoundGenericNominal()) {
473509

474-
if (auto isolation = swift::getActorIsolation(nomDecl);
475-
isolation && isolation.isGlobalActor()) {
476-
return SILIsolationInfo::getGlobalActorIsolated(queriedValue,
477-
isolation.getGlobalActor())
478-
.withUnsafeNonIsolated(varIsolation.isNonisolatedUnsafe());
510+
// See if the nominal decl is global actor isolated. In such a case, we
511+
// know that the metatype is also actor isolated.
512+
if (auto isolation = swift::getActorIsolation(nomDecl);
513+
isolation && isolation.isGlobalActor()) {
514+
return SILIsolationInfo::getGlobalActorIsolated(
515+
queriedValue, isolation.getGlobalActor())
516+
.withUnsafeNonIsolated(varIsolation.isNonisolatedUnsafe());
517+
}
518+
519+
// Then finally check if we have an actor instance and we are getting an
520+
// async allocating initializer for it.
521+
if (nomDecl->isAnyActor()) {
522+
if (auto *constructorDecl = dyn_cast<ConstructorDecl>(field);
523+
constructorDecl && constructorDecl->isAsync()) {
524+
return SILIsolationInfo::getActorInstanceIsolated(
525+
classValue, ActorInstance::getForActorAsyncAllocatingInit(),
526+
nomDecl);
527+
}
528+
}
529+
}
479530
}
480531

481532
return SILIsolationInfo::getDisconnected(varIsolation.isNonisolatedUnsafe());
@@ -759,81 +810,19 @@ SILIsolationInfo SILIsolationInfo::get(SILInstruction *inst) {
759810
}
760811

761812
if (auto *cmi = dyn_cast<ClassMethodInst>(inst)) {
762-
// Ok, we know that we do not have an actor... but we might have a global
763-
// actor isolated method. Use the AST to compute the actor isolation and
764-
// check if we are self. If we are not self, we want this to be
765-
// disconnected.
766-
if (auto *expr = cmi->getLoc().getAsASTNode<Expr>()) {
767-
DeclRefExprAnalysis exprAnalysis;
768-
if (exprAnalysis.compute(expr)) {
769-
auto *dre = exprAnalysis.getResult();
770-
771-
// First see if we can get any information from the actual var decl of
772-
// the class_method. We could find isolation or if our value is marked
773-
// as nonisolated(unsafe), we could find that as well. If we have
774-
// nonisolated(unsafe), we just propagate the value. Otherwise, we
775-
// return the isolation.
776-
bool isNonIsolatedUnsafe = exprAnalysis.hasNonisolatedUnsafe();
777-
{
778-
auto isolation = swift::getActorIsolation(dre->getDecl());
779-
780-
if (isolation.isActorIsolated()) {
781-
// Check if we have a global actor and handle it appropriately.
782-
if (isolation.getKind() == ActorIsolation::GlobalActor) {
783-
bool localNonIsolatedUnsafe =
784-
isNonIsolatedUnsafe | isolation.isNonisolatedUnsafe();
785-
return SILIsolationInfo::getGlobalActorIsolated(
786-
cmi, isolation.getGlobalActor())
787-
.withUnsafeNonIsolated(localNonIsolatedUnsafe);
788-
}
789-
790-
// In this case, we have an actor instance that is self.
791-
if (isolation.getKind() != ActorIsolation::ActorInstance &&
792-
isolation.isActorInstanceForSelfParameter()) {
793-
bool localNonIsolatedUnsafe =
794-
isNonIsolatedUnsafe | isolation.isNonisolatedUnsafe();
795-
return SILIsolationInfo::getActorInstanceIsolated(
796-
cmi, cmi->getOperand(),
797-
cmi->getOperand()
798-
->getType()
799-
.getNominalOrBoundGenericNominal())
800-
.withUnsafeNonIsolated(localNonIsolatedUnsafe);
801-
}
802-
}
803-
}
804-
805-
if (auto type = dre->getType()->getNominalOrBoundGenericNominal()) {
806-
if (auto isolation = swift::getActorIsolation(type)) {
807-
if (isolation.isActorIsolated()) {
808-
// Check if we have a global actor and handle it appropriately.
809-
if (isolation.getKind() == ActorIsolation::GlobalActor) {
810-
bool localNonIsolatedUnsafe =
811-
isNonIsolatedUnsafe | isolation.isNonisolatedUnsafe();
812-
return SILIsolationInfo::getGlobalActorIsolated(
813-
cmi, isolation.getGlobalActor())
814-
.withUnsafeNonIsolated(localNonIsolatedUnsafe);
815-
}
813+
auto base = cmi->getOperand();
814+
auto member = cmi->getMember();
816815

817-
// In this case, we have an actor instance that is self.
818-
if (isolation.getKind() != ActorIsolation::ActorInstance &&
819-
isolation.isActorInstanceForSelfParameter()) {
820-
bool localNonIsolatedUnsafe =
821-
isNonIsolatedUnsafe | isolation.isNonisolatedUnsafe();
822-
return SILIsolationInfo::getActorInstanceIsolated(
823-
cmi, cmi->getOperand(),
824-
cmi->getOperand()
825-
->getType()
826-
.getNominalOrBoundGenericNominal())
827-
.withUnsafeNonIsolated(localNonIsolatedUnsafe);
828-
}
829-
}
830-
}
831-
}
816+
// First see if we can use our SILDeclRef to infer isolation.
817+
if (auto *accessor = member.getAccessorDecl()) {
818+
return computeIsolationForClassField(cmi, base, accessor);
819+
}
832820

833-
if (isNonIsolatedUnsafe)
834-
return SILIsolationInfo::getDisconnected(isNonIsolatedUnsafe);
835-
}
821+
if (auto *funcDecl = member.getAbstractFunctionDecl()) {
822+
return computeIsolationForClassField(cmi, base, funcDecl);
836823
}
824+
825+
llvm_unreachable("Unsupported?!");
837826
}
838827

839828
// See if we have a struct_extract from a global-actor-isolated type.
@@ -1392,6 +1381,12 @@ void SILIsolationInfo::print(SILFunction *fn, llvm::raw_ostream &os) const {
13921381
os << '\n';
13931382
os << "instance: captured actor instance self\n";
13941383
return;
1384+
case ActorInstance::Kind::ActorAsyncAllocatingInit:
1385+
os << "'self'-isolated";
1386+
printOptions(os);
1387+
os << '\n';
1388+
os << "instance: actor async allocating init\n";
1389+
return;
13951390
}
13961391
}
13971392

@@ -1522,9 +1517,8 @@ void SILIsolationInfo::printForDiagnostics(SILFunction *fn,
15221517
break;
15231518
}
15241519
case ActorInstance::Kind::ActorAccessorInit:
1525-
os << "'self'-isolated";
1526-
return;
15271520
case ActorInstance::Kind::CapturedActorSelf:
1521+
case ActorInstance::Kind::ActorAsyncAllocatingInit:
15281522
os << "'self'-isolated";
15291523
return;
15301524
}
@@ -1591,9 +1585,8 @@ void SILIsolationInfo::printForCodeDiagnostic(SILFunction *fn,
15911585
break;
15921586
}
15931587
case ActorInstance::Kind::ActorAccessorInit:
1594-
os << "'self'-isolated code";
1595-
return;
15961588
case ActorInstance::Kind::CapturedActorSelf:
1589+
case ActorInstance::Kind::ActorAsyncAllocatingInit:
15971590
os << "'self'-isolated code";
15981591
return;
15991592
}
@@ -1645,6 +1638,10 @@ void SILIsolationInfo::printForOneLineLogging(SILFunction *fn,
16451638
os << "'self'-isolated (captured-actor-self)";
16461639
printOptions(os);
16471640
return;
1641+
case ActorInstance::Kind::ActorAsyncAllocatingInit:
1642+
os << "'self'-isolated (actor-async-allocating-init)";
1643+
printOptions(os);
1644+
return;
16481645
}
16491646
}
16501647

@@ -1876,6 +1873,9 @@ void ActorInstance::print(llvm::raw_ostream &os) const {
18761873
case Kind::CapturedActorSelf:
18771874
os << "CapturedActorSelf.";
18781875
break;
1876+
case Kind::ActorAsyncAllocatingInit:
1877+
os << "ActorAsyncAllocatingInit.";
1878+
break;
18791879
}
18801880

18811881
if (auto value = maybeGetValue()) {

0 commit comments

Comments
 (0)