Merge pull request #71131 from slavapestov/conformance-checker-vs-type-witnesses-untangling

Sema: Decouple type witness resolution from the ConformanceChecker

Author: slavapestov

include/swift/AST/ProtocolConformance.h

@@ -86,8 +86,6 @@ enum class ProtocolConformanceState {
   Complete = 0,
   /// The conformance is known but is not yet complete.
   Incomplete,
-  /// The conformance's type witnesses are currently being resolved.
-  CheckingTypeWitnesses,
   /// The conformance is being checked.
   Checking,
 
@@ -225,7 +223,6 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance
   /// Determine whether this conformance is incomplete.
   bool isIncomplete() const {
     return getState() == ProtocolConformanceState::Incomplete ||
-           getState() == ProtocolConformanceState::CheckingTypeWitnesses ||
            getState() == ProtocolConformanceState::Checking;
   }
 

include/swift/AST/TypeCheckRequests.h

@@ -2870,6 +2870,24 @@ class ReferencedAssociatedTypesRequest
   bool isCached() const { return true; }
 };
 
+class ResolveTypeWitnessesRequest
+    : public SimpleRequest<ResolveTypeWitnessesRequest,
+                           evaluator::SideEffect(NormalProtocolConformance *),
+                           RequestFlags::Cached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  evaluator::SideEffect
+  evaluate(Evaluator &evaluator, NormalProtocolConformance *conformance) const;
+
+public:
+  bool isCached() const { return true; }
+};
+
 class ValueWitnessRequest
     : public SimpleRequest<ValueWitnessRequest,
                            Witness(NormalProtocolConformance *, ValueDecl *),

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -432,6 +432,9 @@ SWIFT_REQUEST(TypeChecker, AssociatedConformanceRequest,
 SWIFT_REQUEST(TypeChecker, ReferencedAssociatedTypesRequest,
               TinyPtrVector<AssociatedTypeDecl *>(ValueDecl *),
               Cached, NoLocationInfo)
+SWIFT_REQUEST(TypeChecker, ResolveTypeWitnessesRequest,
+              evaluator::SideEffect(NormalProtocolConformance *),
+              Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, ValueWitnessRequest,
               Witness(NormalProtocolConformance *, ValueDecl *),
               SeparatelyCached, NoLocationInfo)

lib/AST/ASTContext.cpp

@@ -2780,56 +2780,69 @@ ASTContext::MissingWitness::MissingWitness(ValueDecl *requirement,
   : requirement(requirement),
     matches(matches.begin(), matches.end()) { }
 
-void ASTContext::addDelayedConformanceDiag(
-       NormalProtocolConformance *conformance, bool isError,
-       std::function<void(NormalProtocolConformance *)> callback) {
-  if (isError)
-    conformance->setInvalid();
-
-  auto &diagnostics = getImpl().DelayedConformanceDiags[conformance];
+static void maybeEmitFallbackConformanceDiagnostic(
+    ASTContext &ctx,
+    NormalProtocolConformance *conformance,
+    DelayedConformanceDiags &diagnostics) {
 
-  if (isError && !diagnostics.HadError) {
-    diagnostics.HadError = true;
+  if (diagnostics.HadError)
+    return;
 
-    auto *proto = conformance->getProtocol();
-    auto *dc = conformance->getDeclContext();
-    auto *sf = dc->getParentSourceFile();
-    auto *mod = sf->getParentModule();
-    assert(mod->isMainModule());
+  diagnostics.HadError = true;
 
-    // If we have at least one primary file and the conformance is declared in a
-    // non-primary file, emit a fallback diagnostic.
-    if ((!sf->isPrimary() && !mod->getPrimarySourceFiles().empty()) ||
-        TypeCheckerOpts.EnableLazyTypecheck) {
-      auto complainLoc = evaluator.getInnermostSourceLoc([&](SourceLoc loc) {
-        if (loc.isInvalid())
-          return false;
+  auto *proto = conformance->getProtocol();
+  auto *dc = conformance->getDeclContext();
+  auto *sf = dc->getParentSourceFile();
+  auto *mod = sf->getParentModule();
+  assert(mod->isMainModule());
 
-        auto *otherSF = mod->getSourceFileContainingLocation(loc);
-        if (otherSF == nullptr)
-          return false;
+  // If we have at least one primary file and the conformance is declared in a
+  // non-primary file, emit a fallback diagnostic.
+  if ((!sf->isPrimary() && !mod->getPrimarySourceFiles().empty()) ||
+      ctx.TypeCheckerOpts.EnableLazyTypecheck) {
+    auto complainLoc = ctx.evaluator.getInnermostSourceLoc([&](SourceLoc loc) {
+      if (loc.isInvalid())
+        return false;
 
-        return otherSF->isPrimary();
-      });
+      auto *otherSF = mod->getSourceFileContainingLocation(loc);
+      if (otherSF == nullptr)
+        return false;
 
-      if (complainLoc.isInvalid()) {
-        complainLoc = conformance->getLoc();
-      }
+      return otherSF->isPrimary();
+    });
 
-      Diags.diagnose(complainLoc,
-                     diag::type_does_not_conform,
-                     dc->getSelfInterfaceType(),
-                     proto->getDeclaredInterfaceType());
+    if (complainLoc.isInvalid()) {
+      complainLoc = conformance->getLoc();
     }
+
+    ctx.Diags.diagnose(complainLoc,
+                       diag::type_does_not_conform,
+                       dc->getSelfInterfaceType(),
+                       proto->getDeclaredInterfaceType());
   }
+}
+
+void ASTContext::addDelayedConformanceDiag(
+       NormalProtocolConformance *conformance, bool isError,
+       std::function<void(NormalProtocolConformance *)> callback) {
+  if (isError)
+    conformance->setInvalid();
+
+  auto &diagnostics = getImpl().DelayedConformanceDiags[conformance];
+
+  if (isError)
+    maybeEmitFallbackConformanceDiagnostic(*this, conformance, diagnostics);
 
   diagnostics.Diags.push_back({isError, callback});
 }
 
 void ASTContext::addDelayedMissingWitness(
     NormalProtocolConformance *conformance,
     ASTContext::MissingWitness missingWitness) {
+  conformance->setInvalid();
+
   auto &diagnostics = getImpl().DelayedConformanceDiags[conformance];
+  maybeEmitFallbackConformanceDiagnostic(*this, conformance, diagnostics);
   diagnostics.MissingWitnesses.push_back(missingWitness);
 }
 

lib/AST/ASTVerifier.cpp

@@ -2803,7 +2803,6 @@ class Verifier : public ASTWalker {
         // Ignore incomplete conformances; we didn't need them.
         return;
 
-      case ProtocolConformanceState::CheckingTypeWitnesses:
       case ProtocolConformanceState::Checking:
         dumpRef(decl);
         Out << " has a protocol conformance that is still being checked "

lib/AST/ProtocolConformance.cpp

@@ -493,7 +493,12 @@ NormalProtocolConformance::getTypeWitnessAndDecl(AssociatedTypeDecl *assocType,
 
   // If this conformance is in a state where it is inferring type witnesses but
   // we didn't find anything, fail.
-  if (getState() == ProtocolConformanceState::CheckingTypeWitnesses) {
+  //
+  // FIXME: This is unsound, because we may not have diagnosed anything but
+  // still end up with an ErrorType in the AST.
+  if (getDeclContext()->getASTContext().evaluator.hasActiveRequest(
+         ResolveTypeWitnessesRequest{
+             const_cast<NormalProtocolConformance *>(this)})) {
     return { Type(), nullptr };
   }
 

lib/AST/SubstitutionMap.cpp

@@ -436,10 +436,13 @@ SubstitutionMap::lookupConformance(CanType type, ProtocolDecl *proto) const {
     if (!normal->hasComputedAssociatedConformances()) {
       // If we're in the process of checking the type witnesses, fail
       // gracefully.
-      // FIXME: Seems like we should be able to get at the intermediate state
-      // to use that.
-      if (normal->getState() == ProtocolConformanceState::CheckingTypeWitnesses)
+      //
+      // FIXME: This is unsound, because we may not have diagnosed anything but
+      // still end up with an ErrorType in the AST.
+      if (proto->getASTContext().evaluator.hasActiveRequest(
+            ResolveTypeWitnessesRequest{normal})) {
         return ProtocolConformanceRef::forInvalid();
+      }
     }
 
     // Get the associated conformance.