Skip to content

Commit a3dd168

Browse files
committed
Split conformance isolation request to eliminate a reference cycle
Inference of conformance isolation needs to check whether all of the witnesses are nonisolated. However, witness checking looks at conformance isolation. To break this reference cycle, split the conformance isolation request into two requests: a "raw" request that looks at explicitly-specified isolation, and the existing one that also performs inference. The existing one builds on the "raw" one, as does a separate path for the conformance checker. Fixes rdar://152461344.
1 parent 9b62aed commit a3dd168

File tree

8 files changed

+155
-7
lines changed

8 files changed

+155
-7
lines changed

include/swift/AST/ProtocolConformance.h

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance
132132
Type ConformingType;
133133

134134
friend class ConformanceIsolationRequest;
135+
friend class RawConformanceIsolationRequest;
135136

136137
protected:
137138
// clang-format off
@@ -141,11 +142,14 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance
141142
union { uint64_t OpaqueBits;
142143

143144
SWIFT_INLINE_BITFIELD_BASE(ProtocolConformance,
144-
1+
145+
1+1+
145146
bitmax(NumProtocolConformanceKindBits, 8),
146147
/// The kind of protocol conformance.
147148
Kind : bitmax(NumProtocolConformanceKindBits, 8),
148149

150+
/// Whether the "raw" conformance isolation is "inferred", which applies to most conformances.
151+
IsRawConformanceInferred : 1,
152+
149153
/// Whether the computed actor isolation is nonisolated.
150154
IsComputedNonisolated : 1
151155
);
@@ -201,9 +205,18 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance
201205
ProtocolConformance(ProtocolConformanceKind kind, Type conformingType)
202206
: ConformingType(conformingType) {
203207
Bits.ProtocolConformance.Kind = unsigned(kind);
208+
Bits.ProtocolConformance.IsRawConformanceInferred = false;
204209
Bits.ProtocolConformance.IsComputedNonisolated = false;
205210
}
206211

212+
bool isRawConformanceInferred() const {
213+
return Bits.ProtocolConformance.IsRawConformanceInferred;
214+
}
215+
216+
void setRawConformanceInferred(bool value = true) {
217+
Bits.ProtocolConformance.IsRawConformanceInferred = value;
218+
}
219+
207220
bool isComputedNonisolated() const {
208221
return Bits.ProtocolConformance.IsComputedNonisolated;
209222
}
@@ -258,6 +271,14 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance
258271
/// Otherwise a new conformance will be created.
259272
ProtocolConformance *getCanonicalConformance();
260273

274+
/// Determine the "raw" actor isolation of this conformance, before applying any inference rules.
275+
///
276+
/// Most clients should use `getIsolation()`, unless they are part of isolation inference
277+
/// themselves (e.g., conformance checking).
278+
///
279+
/// - Returns std::nullopt if the isolation will be inferred.
280+
std::optional<ActorIsolation> getRawIsolation() const;
281+
261282
/// Determine the actor isolation of this conformance.
262283
ActorIsolation getIsolation() const;
263284

@@ -556,6 +577,7 @@ class NormalProtocolConformance : public RootProtocolConformance,
556577
friend class ValueWitnessRequest;
557578
friend class TypeWitnessRequest;
558579
friend class ConformanceIsolationRequest;
580+
friend class RawConformanceIsolationRequest;
559581

560582
/// The protocol being conformed to.
561583
ProtocolDecl *Protocol;

include/swift/AST/TypeCheckRequests.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,28 @@ class ConformanceHasEffectRequest :
475475
bool isCached() const { return true; }
476476
};
477477

478+
class RawConformanceIsolationRequest :
479+
public SimpleRequest<RawConformanceIsolationRequest,
480+
std::optional<ActorIsolation>(ProtocolConformance *),
481+
RequestFlags::SeparatelyCached |
482+
RequestFlags::SplitCached> {
483+
public:
484+
using SimpleRequest::SimpleRequest;
485+
486+
private:
487+
friend SimpleRequest;
488+
489+
// Evaluation.
490+
std::optional<ActorIsolation>
491+
evaluate(Evaluator &evaluator, ProtocolConformance *conformance) const;
492+
493+
public:
494+
// Separate caching.
495+
bool isCached() const { return true; }
496+
std::optional<std::optional<ActorIsolation>> getCachedResult() const;
497+
void cacheResult(std::optional<ActorIsolation> result) const;
498+
};
499+
478500
class ConformanceIsolationRequest :
479501
public SimpleRequest<ConformanceIsolationRequest,
480502
ActorIsolation(ProtocolConformance *),

include/swift/AST/TypeCheckerTypeIDZone.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,10 @@ SWIFT_REQUEST(TypeChecker, PolymorphicEffectRequirementsRequest,
406406
SWIFT_REQUEST(TypeChecker, ConformanceHasEffectRequest,
407407
bool(EffectKind, ProtocolConformanceRef),
408408
Cached, NoLocationInfo)
409+
SWIFT_REQUEST(TypeChecker, RawConformanceIsolationRequest,
410+
std::optional<ActorIsolation>(ProtocolConformance *),
411+
SeparatelyCached | SplitCached,
412+
NoLocationInfo)
409413
SWIFT_REQUEST(TypeChecker, ConformanceIsolationRequest,
410414
ActorIsolation(ProtocolConformance *),
411415
SeparatelyCached | SplitCached,

lib/AST/TypeCheckRequests.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,6 +1369,38 @@ void AssociatedConformanceRequest::cacheResult(
13691369
conformance->setAssociatedConformance(index, assocConf);
13701370
}
13711371

1372+
//----------------------------------------------------------------------------//
1373+
// RawConformanceIsolationRequest computation.
1374+
//----------------------------------------------------------------------------//
1375+
std::optional<std::optional<ActorIsolation>>
1376+
RawConformanceIsolationRequest::getCachedResult() const {
1377+
// We only want to cache for global-actor-isolated conformances. For
1378+
// everything else, which is nearly every conformance, this request quickly
1379+
// returns "nonisolated" so there is no point in caching it.
1380+
auto conformance = std::get<0>(getStorage());
1381+
1382+
// Was actor isolation non-isolated?
1383+
if (conformance->isRawConformanceInferred())
1384+
return std::optional<ActorIsolation>();
1385+
1386+
ASTContext &ctx = conformance->getDeclContext()->getASTContext();
1387+
return ctx.evaluator.getCachedNonEmptyOutput(*this);
1388+
}
1389+
1390+
void RawConformanceIsolationRequest::cacheResult(
1391+
std::optional<ActorIsolation> result) const {
1392+
auto conformance = std::get<0>(getStorage());
1393+
1394+
// Common case: conformance is inferred, so there's no result.
1395+
if (!result) {
1396+
conformance->setRawConformanceInferred();
1397+
return;
1398+
}
1399+
1400+
ASTContext &ctx = conformance->getDeclContext()->getASTContext();
1401+
ctx.evaluator.cacheNonEmptyOutput(*this, std::move(result));
1402+
}
1403+
13721404
//----------------------------------------------------------------------------//
13731405
// ConformanceIsolationRequest computation.
13741406
//----------------------------------------------------------------------------//

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7954,6 +7954,14 @@ bool swift::diagnoseNonSendableFromDeinit(
79547954
var->getDescriptiveKind(), var->getName());
79557955
}
79567956

7957+
std::optional<ActorIsolation> ProtocolConformance::getRawIsolation() const {
7958+
ASTContext &ctx = getDeclContext()->getASTContext();
7959+
auto conformance = const_cast<ProtocolConformance *>(this);
7960+
return evaluateOrDefault(
7961+
ctx.evaluator, RawConformanceIsolationRequest{conformance},
7962+
ActorIsolation());
7963+
}
7964+
79577965
ActorIsolation ProtocolConformance::getIsolation() const {
79587966
ASTContext &ctx = getDeclContext()->getASTContext();
79597967
auto conformance = const_cast<ProtocolConformance *>(this);
@@ -7962,16 +7970,18 @@ ActorIsolation ProtocolConformance::getIsolation() const {
79627970
ActorIsolation());
79637971
}
79647972

7965-
ActorIsolation
7966-
ConformanceIsolationRequest::evaluate(Evaluator &evaluator, ProtocolConformance *conformance) const {
7973+
std::optional<ActorIsolation>
7974+
RawConformanceIsolationRequest::evaluate(
7975+
Evaluator &evaluator, ProtocolConformance *conformance
7976+
) const {
79677977
// Only normal protocol conformances can be isolated.
79687978
auto rootNormal =
79697979
dyn_cast<NormalProtocolConformance>(conformance->getRootConformance());
79707980
if (!rootNormal)
79717981
return ActorIsolation::forNonisolated(false);
79727982

79737983
if (conformance != rootNormal)
7974-
return rootNormal->getIsolation();
7984+
return rootNormal->getRawIsolation();
79757985

79767986
// If the conformance is explicitly non-isolated, report that.
79777987
if (rootNormal->getOptions().contains(ProtocolConformanceFlags::Nonisolated))
@@ -8021,9 +8031,15 @@ ConformanceIsolationRequest::evaluate(Evaluator &evaluator, ProtocolConformance
80218031
if (rootNormal->isPreconcurrency())
80228032
return ActorIsolation::forNonisolated(false);
80238033

8034+
return std::nullopt;
8035+
}
80248036

8025-
// Isolation inference rules follow. If we aren't inferring isolated conformances,
8026-
// we're done.
8037+
ActorIsolation swift::inferConformanceIsolation(
8038+
NormalProtocolConformance *conformance, bool hasKnownIsolatedWitness) {
8039+
auto dc = conformance->getDeclContext();
8040+
ASTContext &ctx = dc->getASTContext();
8041+
8042+
// If we aren't inferring isolated conformances, we're done.
80278043
if (!ctx.LangOpts.hasFeature(Feature::InferIsolatedConformances))
80288044
return ActorIsolation::forNonisolated(false);
80298045

@@ -8041,6 +8057,12 @@ ConformanceIsolationRequest::evaluate(Evaluator &evaluator, ProtocolConformance
80418057

80428058
// If all of the value witnesses are nonisolated, then we should not infer
80438059
// global actor isolation.
8060+
if (hasKnownIsolatedWitness) {
8061+
// The caller told us we have an isolated value witness, so infer
8062+
// the nominal isolation.
8063+
return nominalIsolation;
8064+
}
8065+
80448066
bool anyIsolatedWitness = false;
80458067
auto protocol = conformance->getProtocol();
80468068
for (auto requirement : protocol->getMembers()) {
@@ -8069,6 +8091,29 @@ ConformanceIsolationRequest::evaluate(Evaluator &evaluator, ProtocolConformance
80698091
return nominalIsolation;
80708092
}
80718093

8094+
ActorIsolation
8095+
ConformanceIsolationRequest::evaluate(
8096+
Evaluator &evaluator, ProtocolConformance *conformance
8097+
) const {
8098+
// If there is raw isolation, use that.
8099+
if (auto rawIsolation = conformance->getRawIsolation())
8100+
return *rawIsolation;
8101+
8102+
// Otherwise, we may infer isolation.
8103+
8104+
// Only normal protocol conformances can be isolated.
8105+
auto rootNormal =
8106+
dyn_cast<NormalProtocolConformance>(conformance->getRootConformance());
8107+
if (!rootNormal)
8108+
return ActorIsolation::forNonisolated(false);
8109+
8110+
if (conformance != rootNormal)
8111+
return rootNormal->getIsolation();
8112+
8113+
return inferConformanceIsolation(
8114+
rootNormal, /*hasKnownIsolatedWitness=*/false);
8115+
}
8116+
80728117
namespace {
80738118
/// Identifies isolated conformances whose isolation differs from the
80748119
/// context's isolation.

lib/Sema/TypeCheckConcurrency.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,13 @@ bool checkIsolatedConformancesInContext(
746746
Type type, SourceLoc loc, const DeclContext *dc,
747747
HandleConformanceIsolationFn handleBad = doNotDiagnoseConformanceIsolation);
748748

749+
/// For a protocol conformance that does not have a "raw" isolation, infer its isolation.
750+
///
751+
/// - hasKnownIsolatedWitness: indicates when it is known that there is an actor-isolated witness, meaning
752+
/// that this operation will not look at other witnesses to determine if they are all nonisolated.
753+
ActorIsolation inferConformanceIsolation(
754+
NormalProtocolConformance *conformance, bool hasKnownIsolatedWitness);
755+
749756
} // end namespace swift
750757

751758
namespace llvm {

lib/Sema/TypeCheckProtocol.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3357,6 +3357,16 @@ static bool hasExplicitGlobalActorAttr(ValueDecl *decl) {
33573357
return !globalActorAttr->first->isImplicit();
33583358
}
33593359

3360+
/// Determine the isolation of a conformance with a known-isolated value witness.
3361+
static ActorIsolation getConformanceIsolationForIsolatedWitness(
3362+
NormalProtocolConformance *conformance) {
3363+
if (auto rawIsolation = conformance->getRawIsolation())
3364+
return *rawIsolation;
3365+
3366+
return inferConformanceIsolation(
3367+
conformance, /*hasKnownIsolatedWitness=*/true);
3368+
}
3369+
33603370
std::optional<ActorIsolation>
33613371
ConformanceChecker::checkActorIsolation(ValueDecl *requirement,
33623372
ValueDecl *witness,
@@ -3421,7 +3431,8 @@ ConformanceChecker::checkActorIsolation(ValueDecl *requirement,
34213431
case ActorReferenceResult::EntersActor: {
34223432
// If the conformance itself is isolated to the same isolation domain as
34233433
// the witness, treat this as being in the same concurrency domain.
3424-
auto conformanceIsolation = Conformance->getIsolation();
3434+
auto conformanceIsolation =
3435+
getConformanceIsolationForIsolatedWitness(Conformance);
34253436
if (conformanceIsolation.isGlobalActor() &&
34263437
refResult.isolation == conformanceIsolation) {
34273438
sameConcurrencyDomain = true;

test/Concurrency/isolated_conformance_default_actor.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ class OtherClass {
6464
var otherState: any Encodable.Type = CodableClass.self
6565
}
6666

67+
struct Landmark: Codable {
68+
var name: String
69+
var foundingYear: Int
70+
}
71+
6772
func acceptSendablePMeta<T: Sendable & P>(_: T.Type) { }
6873
func acceptSendableQMeta<T: Sendable & Q>(_: T.Type) { }
6974

0 commit comments

Comments
 (0)