Redo getConditionalRequirements as a Request

Author: CodaFi

include/swift/AST/ProtocolConformance.h

@@ -443,19 +443,6 @@ class NormalProtocolConformance : public RootProtocolConformance,
   /// requirement signature of the protocol.
   ArrayRef<ProtocolConformanceRef> SignatureConformances;
 
-  /// Any additional requirements that are required for this conformance to
-  /// apply, e.g. 'Something: Baz' in 'extension Foo: Bar where Something: Baz'.
-  mutable ArrayRef<Requirement> ConditionalRequirements;
-  enum class ConditionalRequirementsState {
-    Uncomputed,
-    Computing,
-    Complete,
-  };
-  /// The state of the ConditionalRequirements field: whether it has been
-  /// computed or not.
-  mutable ConditionalRequirementsState CRState =
-      ConditionalRequirementsState::Uncomputed;
-
   /// The lazy member loader provides callbacks for populating imported and
   /// deserialized conformances.
   ///
@@ -467,8 +454,6 @@ class NormalProtocolConformance : public RootProtocolConformance,
 
   void resolveLazyInfo() const;
 
-  void differenceAndStoreConditionalRequirements() const;
-
 public:
   NormalProtocolConformance(Type conformingType, ProtocolDecl *protocol,
                             SourceLoc loc, DeclContext *dc,
@@ -493,39 +478,12 @@ class NormalProtocolConformance : public RootProtocolConformance,
     return ContextAndInvalid.getPointer();
   }
 
-  /// Get any additional requirements that are required for this conformance to
-  /// be satisfied if they can be computed.
-  ///
-  /// If \c computeIfPossible is false, this will not do the lazy computation of
-  /// the conditional requirements and will just query the current state. This
-  /// should almost certainly only be used for debugging purposes, prefer \c
-  /// getConditionalRequirementsIfAvailable (these are separate because
-  /// CONFORMANCE_SUBCLASS_DISPATCH does some type checks and a defaulted
-  /// parameter gets in the way of that).
-  Optional<ArrayRef<Requirement>>
-  getConditionalRequirementsIfAvailableOrCached(bool computeIfPossible) const {
-    if (computeIfPossible)
-      differenceAndStoreConditionalRequirements();
-
-    if (CRState == ConditionalRequirementsState::Complete)
-      return ConditionalRequirements;
-
-    return None;
-  }
-  /// Get any additional requirements that are required for this conformance to
-  /// be satisfied if they can be computed.
-  Optional<ArrayRef<Requirement>>
-  getConditionalRequirementsIfAvailable() const {
-    return getConditionalRequirementsIfAvailableOrCached(
-        /*computeIfPossible=*/true);
-  }
-
   /// Get any additional requirements that are required for this conformance to
   /// be satisfied, e.g. for Array<T>: Equatable, T: Equatable also needs
   /// to be satisfied.
-  ArrayRef<Requirement> getConditionalRequirements() const {
-    return *getConditionalRequirementsIfAvailable();
-  }
+  ArrayRef<Requirement> getConditionalRequirements() const;
+
+  Optional<ArrayRef<Requirement>> getConditionalRequirementsIfAvailable() const;
 
   /// Retrieve the state of this conformance.
   ProtocolConformanceState getState() const {

include/swift/AST/TypeCheckRequests.h

@@ -2917,6 +2917,25 @@ class GetImplicitConcurrentValueRequest :
   bool isCached() const { return true; }
 };
 
+class ConditionalRequirementsRequest
+    : public SimpleRequest<ConditionalRequirementsRequest,
+                           llvm::ArrayRef<Requirement>(
+                               NormalProtocolConformance *),
+                           RequestFlags::Cached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  llvm::ArrayRef<Requirement> evaluate(Evaluator &evaluator,
+                                       NormalProtocolConformance *decl) const;
+
+public:
+  bool isCached() const { return true; }
+};
+
 void simple_display(llvm::raw_ostream &out, Type value);
 void simple_display(llvm::raw_ostream &out, const TypeRepr *TyR);
 void simple_display(llvm::raw_ostream &out, ImplicitMemberAction action);

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -43,6 +43,9 @@ SWIFT_REQUEST(TypeChecker, CodeCompletionFileRequest,
 SWIFT_REQUEST(TypeChecker, CompareDeclSpecializationRequest,
               bool (DeclContext *, ValueDecl *, ValueDecl *, bool), Cached,
               NoLocationInfo)
+SWIFT_REQUEST(TypeChecker, ConditionalRequirementsRequest,
+              ArrayRef<Requirement> (NormalProtocolConformance *),
+              Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, IsDeclRefinementOfRequest,
               bool (ValueDecl *, ValueDecl *),
               Cached, NoLocationInfo)

lib/AST/ASTDumper.cpp

@@ -3279,8 +3279,7 @@ static void dumpProtocolConformanceRec(
       }
     }
 
-    if (auto condReqs = normal->getConditionalRequirementsIfAvailableOrCached(
-            /*computeIfPossible=*/false)) {
+    if (auto condReqs = normal->getConditionalRequirementsIfAvailable()) {
       for (auto requirement : *condReqs) {
         out << '\n';
         out.indent(indent + 2);

lib/AST/ProtocolConformance.cpp

@@ -482,56 +482,47 @@ ProtocolConformanceRef::getConditionalRequirements() const {
     return {};
 }
 
-void NormalProtocolConformance::differenceAndStoreConditionalRequirements()
-    const {
-  switch (CRState) {
-  case ConditionalRequirementsState::Complete:
-    // already done!
-    return;
-  case ConditionalRequirementsState::Computing:
-    // recursive
-    return;
-  case ConditionalRequirementsState::Uncomputed:
-    // try to compute it!
-    break;
-  };
+Optional<ArrayRef<Requirement>>
+NormalProtocolConformance::getConditionalRequirementsIfAvailable() const {
+  const auto &eval = getDeclContext()->getASTContext().evaluator;
+  if (eval.hasActiveRequest(ConditionalRequirementsRequest{
+          const_cast<NormalProtocolConformance *>(this)})) {
+    return None;
+  }
+  return getConditionalRequirements();
+}
 
-  CRState = ConditionalRequirementsState::Computing;
-  auto success = [this](ArrayRef<Requirement> reqs = {}) {
-    ConditionalRequirements = reqs;
-    assert(CRState == ConditionalRequirementsState::Computing);
-    CRState = ConditionalRequirementsState::Complete;
-  };
-  auto failure = [this] {
-    assert(CRState == ConditionalRequirementsState::Computing);
-    CRState = ConditionalRequirementsState::Uncomputed;
-  };
+llvm::ArrayRef<Requirement>
+NormalProtocolConformance::getConditionalRequirements() const {
+  const auto ext = dyn_cast<ExtensionDecl>(getDeclContext());
+  if (ext && ext->isComputingGenericSignature()) {
+    return {};
+  }
+  return evaluateOrDefault(getProtocol()->getASTContext().evaluator,
+                           ConditionalRequirementsRequest{
+                               const_cast<NormalProtocolConformance *>(this)},
+                           {});
+}
 
+llvm::ArrayRef<Requirement>
+ConditionalRequirementsRequest::evaluate(Evaluator &evaluator,
+                                         NormalProtocolConformance *NPC) const {
   // A non-extension conformance won't have conditional requirements.
-  const auto ext = dyn_cast<ExtensionDecl>(getDeclContext());
+  const auto ext = dyn_cast<ExtensionDecl>(NPC->getDeclContext());
   if (!ext) {
-    return success();
+    return {};
   }
 
   // If the extension is invalid, it won't ever get a signature, so we
   // "succeed" with an empty result instead.
   if (ext->isInvalid()) {
-    return success();
+    return {};
   }
 
   // A non-generic type won't have conditional requirements.
   const auto typeSig = ext->getExtendedNominal()->getGenericSignature();
   if (!typeSig) {
-    return success();
-  }
-
-  // Recursively validating the signature comes up frequently as expanding
-  // conformance requirements might re-enter this method.  We can at least catch
-  // this and come back to these requirements later.
-  //
-  // FIXME: In the long run, break this cycle in a more principled way.
-  if (ext->isComputingGenericSignature()) {
-    return failure();
+    return {};
   }
 
   const auto extensionSig = ext->getGenericSignature();
@@ -548,9 +539,9 @@ void NormalProtocolConformance::differenceAndStoreConditionalRequirements()
   // type, these are the ones that make the conformance conditional.
   const auto unsatReqs = extensionSig->requirementsNotSatisfiedBy(typeSig);
   if (unsatReqs.empty())
-    return success();
+    return {};
 
-  return success(getProtocol()->getASTContext().AllocateCopy(unsatReqs));
+  return NPC->getProtocol()->getASTContext().AllocateCopy(unsatReqs);
 }
 
 void NormalProtocolConformance::setSignatureConformances(

validation-test/compiler_crashers_2_fixed/0161-sr6569.swift

@@ -1,10 +1,19 @@
 // RUN: %target-typecheck-verify-swift
 
 protocol P {
-    associatedtype A: P
+    associatedtype A: P // expected-note {{protocol requires nested type 'A'; do you want to add it?}}
 }
 
 struct Type<Param> {}
-extension Type: P where Param: P, Param.A == Type<Param> { // expected-error {{requirement involves recursion that is not currently supported}}
-    typealias A = Param
+extension Type: P where Param: P, Param.A == Type<Param> {
+  // expected-error@-1 {{circular reference}}
+  // expected-note@-2 {{through reference here}}
+  // expected-error@-3 {{circular reference}}
+  // expected-note@-4 {{through reference here}}
+  // expected-error@-5 {{circular reference}}
+  // expected-note@-6 {{through reference here}}
+  // expected-error@-7 {{type 'Type<Param>' does not conform to protocol 'P'}}
+  typealias A = Param
+  // expected-note@-1 {{through reference here}}
+  // expected-note@-2 {{through reference here}}
 }

validation-test/compiler_crashers_2_fixed/0162-sr8019.swift

@@ -2,4 +2,4 @@
 
 protocol P {}
 struct A<C> {}
-extension A: P where A: P {} // expected-error {{requirement involves recursion that is not currently supported}}
+extension A: P where A: P {} // expected-error {{type 'A<C>' in conformance requirement does not refer to a generic parameter or associated type}}

validation-test/compiler_crashers_2_fixed/0163-sr8033.swift

@@ -5,5 +5,7 @@ struct Foo<T> {}
 protocol P1 {
     associatedtype A // expected-note {{protocol requires nested type 'A'; do you want to add it?}}
 }
-extension Foo: P1 where A : P1 {} // expected-error {{unsupported recursion for reference to associated type 'A' of type 'Foo<T>'}}
-// expected-error@-1 {{type 'Foo<T>' does not conform to protocol 'P1'}}
+extension Foo: P1 where A : P1 {} // expected-error {{circular reference}}
+// expected-note@-1 {{while resolving type 'A'}}
+// expected-note@-2 {{through reference here}}
+// expected-error@-3 {{type 'Foo<T>' does not conform to protocol 'P1'}}