Merge pull request #81934 from DougGregor/break-isolated-conformance-reference-cycle-6.2

Split conformance isolation request to eliminate a reference cycle

Author: DougGregor

include/swift/AST/ProtocolConformance.h

@@ -132,6 +132,7 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance
   Type ConformingType;
 
   friend class ConformanceIsolationRequest;
+  friend class RawConformanceIsolationRequest;
 
 protected:
   // clang-format off
@@ -141,11 +142,14 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance
   union { uint64_t OpaqueBits;
 
     SWIFT_INLINE_BITFIELD_BASE(ProtocolConformance,
-                               1+
+                               1+1+
                                bitmax(NumProtocolConformanceKindBits, 8),
       /// The kind of protocol conformance.
       Kind : bitmax(NumProtocolConformanceKindBits, 8),
 
+      /// Whether the "raw" conformance isolation is "inferred", which applies to most conformances.
+      IsRawConformanceInferred : 1,
+
       /// Whether the computed actor isolation is nonisolated.
       IsComputedNonisolated : 1
     );
@@ -201,9 +205,18 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance
   ProtocolConformance(ProtocolConformanceKind kind, Type conformingType)
     : ConformingType(conformingType) {
     Bits.ProtocolConformance.Kind = unsigned(kind);
+    Bits.ProtocolConformance.IsRawConformanceInferred = false;
     Bits.ProtocolConformance.IsComputedNonisolated = false;
   }
 
+  bool isRawConformanceInferred() const {
+    return Bits.ProtocolConformance.IsRawConformanceInferred;
+  }
+
+  void setRawConformanceInferred(bool value = true) {
+    Bits.ProtocolConformance.IsRawConformanceInferred = value;
+  }
+
   bool isComputedNonisolated() const {
     return Bits.ProtocolConformance.IsComputedNonisolated;
   }
@@ -258,6 +271,14 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance
   /// Otherwise a new conformance will be created.
   ProtocolConformance *getCanonicalConformance();
 
+  /// Determine the "raw" actor isolation of this conformance, before applying any inference rules.
+  ///
+  /// Most clients should use `getIsolation()`, unless they are part of isolation inference
+  /// themselves (e.g., conformance checking).
+  ///
+  /// - Returns std::nullopt if the isolation will be inferred.
+  std::optional<ActorIsolation> getRawIsolation() const;
+
   /// Determine the actor isolation of this conformance.
   ActorIsolation getIsolation() const;
 
@@ -556,6 +577,7 @@ class NormalProtocolConformance : public RootProtocolConformance,
   friend class ValueWitnessRequest;
   friend class TypeWitnessRequest;
   friend class ConformanceIsolationRequest;
+  friend class RawConformanceIsolationRequest;
 
   /// The protocol being conformed to.
   ProtocolDecl *Protocol;

include/swift/AST/TypeCheckRequests.h

@@ -475,6 +475,28 @@ class ConformanceHasEffectRequest :
   bool isCached() const { return true; }
 };
 
+class RawConformanceIsolationRequest :
+    public SimpleRequest<RawConformanceIsolationRequest,
+                         std::optional<ActorIsolation>(ProtocolConformance *),
+                         RequestFlags::SeparatelyCached |
+                         RequestFlags::SplitCached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  std::optional<ActorIsolation>
+  evaluate(Evaluator &evaluator, ProtocolConformance *conformance) const;
+
+public:
+  // Separate caching.
+  bool isCached() const { return true; }
+  std::optional<std::optional<ActorIsolation>> getCachedResult() const;
+  void cacheResult(std::optional<ActorIsolation> result) const;
+};
+
 class ConformanceIsolationRequest :
     public SimpleRequest<ConformanceIsolationRequest,
                          ActorIsolation(ProtocolConformance *),

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -406,6 +406,10 @@ SWIFT_REQUEST(TypeChecker, PolymorphicEffectRequirementsRequest,
 SWIFT_REQUEST(TypeChecker, ConformanceHasEffectRequest,
               bool(EffectKind, ProtocolConformanceRef),
               Cached, NoLocationInfo)
+SWIFT_REQUEST(TypeChecker, RawConformanceIsolationRequest,
+              std::optional<ActorIsolation>(ProtocolConformance *),
+              SeparatelyCached | SplitCached,
+              NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, ConformanceIsolationRequest,
               ActorIsolation(ProtocolConformance *),
               SeparatelyCached | SplitCached,

lib/AST/TypeCheckRequests.cpp

@@ -1369,6 +1369,38 @@ void AssociatedConformanceRequest::cacheResult(
   conformance->setAssociatedConformance(index, assocConf);
 }
 
+//----------------------------------------------------------------------------//
+// RawConformanceIsolationRequest computation.
+//----------------------------------------------------------------------------//
+std::optional<std::optional<ActorIsolation>>
+RawConformanceIsolationRequest::getCachedResult() const {
+  // We only want to cache for global-actor-isolated conformances. For
+  // everything else, which is nearly every conformance, this request quickly
+  // returns "nonisolated" so there is no point in caching it.
+  auto conformance = std::get<0>(getStorage());
+
+  // Was actor isolation non-isolated?
+  if (conformance->isRawConformanceInferred())
+    return std::optional<ActorIsolation>();
+
+  ASTContext &ctx = conformance->getDeclContext()->getASTContext();
+  return ctx.evaluator.getCachedNonEmptyOutput(*this);
+}
+
+void RawConformanceIsolationRequest::cacheResult(
+    std::optional<ActorIsolation> result) const {
+  auto conformance = std::get<0>(getStorage());
+
+  // Common case: conformance is inferred, so there's no result.
+  if (!result) {
+    conformance->setRawConformanceInferred();
+    return;
+  }
+
+  ASTContext &ctx = conformance->getDeclContext()->getASTContext();
+  ctx.evaluator.cacheNonEmptyOutput(*this, std::move(result));
+}
+
 //----------------------------------------------------------------------------//
 // ConformanceIsolationRequest computation.
 //----------------------------------------------------------------------------//

lib/Sema/TypeCheckConcurrency.cpp

@@ -7954,6 +7954,14 @@ bool swift::diagnoseNonSendableFromDeinit(
                                 var->getDescriptiveKind(), var->getName());
 }
 
+std::optional<ActorIsolation> ProtocolConformance::getRawIsolation() const {
+  ASTContext &ctx = getDeclContext()->getASTContext();
+  auto conformance = const_cast<ProtocolConformance *>(this);
+  return evaluateOrDefault(
+      ctx.evaluator, RawConformanceIsolationRequest{conformance},
+      ActorIsolation());
+}
+
 ActorIsolation ProtocolConformance::getIsolation() const {
   ASTContext &ctx = getDeclContext()->getASTContext();
   auto conformance = const_cast<ProtocolConformance *>(this);
@@ -7962,16 +7970,18 @@ ActorIsolation ProtocolConformance::getIsolation() const {
       ActorIsolation());
 }
 
-ActorIsolation
-ConformanceIsolationRequest::evaluate(Evaluator &evaluator, ProtocolConformance *conformance) const {
+std::optional<ActorIsolation>
+RawConformanceIsolationRequest::evaluate(
+    Evaluator &evaluator, ProtocolConformance *conformance
+) const {
   // Only normal protocol conformances can be isolated.
   auto rootNormal =
       dyn_cast<NormalProtocolConformance>(conformance->getRootConformance());
   if (!rootNormal)
     return ActorIsolation::forNonisolated(false);
 
   if (conformance != rootNormal)
-    return rootNormal->getIsolation();
+    return rootNormal->getRawIsolation();
 
   // If the conformance is explicitly non-isolated, report that.
   if (rootNormal->getOptions().contains(ProtocolConformanceFlags::Nonisolated))
@@ -8021,9 +8031,15 @@ ConformanceIsolationRequest::evaluate(Evaluator &evaluator, ProtocolConformance
   if (rootNormal->isPreconcurrency())
     return ActorIsolation::forNonisolated(false);
 
+  return std::nullopt;
+}
 
-  // Isolation inference rules follow. If we aren't inferring isolated conformances,
-  // we're done.
+ActorIsolation swift::inferConformanceIsolation(
+    NormalProtocolConformance *conformance, bool hasKnownIsolatedWitness) {
+  auto dc = conformance->getDeclContext();
+  ASTContext &ctx = dc->getASTContext();
+
+  // If we aren't inferring isolated conformances, we're done.
   if (!ctx.LangOpts.hasFeature(Feature::InferIsolatedConformances))
     return ActorIsolation::forNonisolated(false);
 
@@ -8041,6 +8057,12 @@ ConformanceIsolationRequest::evaluate(Evaluator &evaluator, ProtocolConformance
 
   // If all of the value witnesses are nonisolated, then we should not infer
   // global actor isolation.
+  if (hasKnownIsolatedWitness) {
+    // The caller told us we have an isolated value witness, so infer
+    // the nominal isolation.
+    return nominalIsolation;
+  }
+
   bool anyIsolatedWitness = false;
   auto protocol = conformance->getProtocol();
   for (auto requirement : protocol->getMembers()) {
@@ -8069,6 +8091,29 @@ ConformanceIsolationRequest::evaluate(Evaluator &evaluator, ProtocolConformance
   return nominalIsolation;
 }
 
+ActorIsolation
+ConformanceIsolationRequest::evaluate(
+    Evaluator &evaluator, ProtocolConformance *conformance
+) const {
+  // If there is raw isolation, use that.
+  if (auto rawIsolation = conformance->getRawIsolation())
+    return *rawIsolation;
+
+  // Otherwise, we may infer isolation.
+
+  // Only normal protocol conformances can be isolated.
+  auto rootNormal =
+      dyn_cast<NormalProtocolConformance>(conformance->getRootConformance());
+  if (!rootNormal)
+    return ActorIsolation::forNonisolated(false);
+
+  if (conformance != rootNormal)
+    return rootNormal->getIsolation();
+
+  return inferConformanceIsolation(
+      rootNormal, /*hasKnownIsolatedWitness=*/false);
+}
+
 namespace {
   /// Identifies isolated conformances whose isolation differs from the
   /// context's isolation.

lib/Sema/TypeCheckConcurrency.h

@@ -746,6 +746,13 @@ bool checkIsolatedConformancesInContext(
     Type type, SourceLoc loc, const DeclContext *dc,
     HandleConformanceIsolationFn handleBad = doNotDiagnoseConformanceIsolation);
 
+/// For a protocol conformance that does not have a "raw" isolation, infer its isolation.
+///
+/// - hasKnownIsolatedWitness: indicates when it is known that there is an actor-isolated witness, meaning
+///   that this operation will not look at other witnesses to determine if they are all nonisolated.
+ActorIsolation inferConformanceIsolation(
+    NormalProtocolConformance *conformance, bool hasKnownIsolatedWitness);
+
 } // end namespace swift
 
 namespace llvm {

lib/Sema/TypeCheckProtocol.cpp

@@ -3357,6 +3357,16 @@ static bool hasExplicitGlobalActorAttr(ValueDecl *decl) {
   return !globalActorAttr->first->isImplicit();
 }
 
+/// Determine the isolation of a conformance with a known-isolated value witness.
+static ActorIsolation getConformanceIsolationForIsolatedWitness(
+    NormalProtocolConformance *conformance) {
+  if (auto rawIsolation = conformance->getRawIsolation())
+    return *rawIsolation;
+
+  return inferConformanceIsolation(
+      conformance, /*hasKnownIsolatedWitness=*/true);
+}
+
 std::optional<ActorIsolation>
 ConformanceChecker::checkActorIsolation(ValueDecl *requirement,
                                         ValueDecl *witness,
@@ -3421,7 +3431,8 @@ ConformanceChecker::checkActorIsolation(ValueDecl *requirement,
   case ActorReferenceResult::EntersActor: {
     // If the conformance itself is isolated to the same isolation domain as
     // the witness, treat this as being in the same concurrency domain.
-    auto conformanceIsolation = Conformance->getIsolation();
+    auto conformanceIsolation =
+        getConformanceIsolationForIsolatedWitness(Conformance);
     if (conformanceIsolation.isGlobalActor() &&
         refResult.isolation == conformanceIsolation) {
       sameConcurrencyDomain = true;

test/Concurrency/isolated_conformance_default_actor.swift

@@ -64,6 +64,11 @@ class OtherClass {
   var otherState: any Encodable.Type = CodableClass.self
 }
 
+struct Landmark: Codable {
+  var name: String
+  var foundingYear: Int
+}
+
 func acceptSendablePMeta<T: Sendable & P>(_: T.Type) { }
 func acceptSendableQMeta<T: Sendable & Q>(_: T.Type) { }