Merge pull request #71041 from slavapestov/refactor-conformance-checker-diagnostics

Refactor conformance checker diagnostics

Author: slavapestov

include/swift/AST/ASTContext.h

@@ -1313,9 +1313,8 @@ class ASTContext final {
   /// conformance itself, along with a bit indicating whether this diagnostic
   /// produces an error.
   struct DelayedConformanceDiag {
-    const ValueDecl *Requirement;
-    std::function<void()> Callback;
     bool IsError;
+    std::function<void(NormalProtocolConformance *)> Callback;
   };
 
   /// Check whether current context has any errors associated with
@@ -1330,8 +1329,9 @@ class ASTContext final {
 
   /// Add a delayed diagnostic produced while type-checking a
   /// particular protocol conformance.
-  void addDelayedConformanceDiag(NormalProtocolConformance *conformance,
-                                 DelayedConformanceDiag fn);
+  void addDelayedConformanceDiag(
+      NormalProtocolConformance *conformance, bool isError,
+      std::function<void(NormalProtocolConformance *)> callback);
 
   /// Retrieve the delayed-conformance diagnostic callbacks for the
   /// given normal protocol conformance.

include/swift/AST/AnyRequest.h

@@ -58,6 +58,9 @@ struct AnyRequestVTable {
     static void noteCycleStep(const void *ptr, DiagnosticEngine &diags) {
       static_cast<const Request *>(ptr)->noteCycleStep(diags);
     }
+    static SourceLoc getNearestLoc(const void *ptr) {
+      return static_cast<const Request *>(ptr)->getNearestLoc();
+    }
   };
 
   const uint64_t typeID;
@@ -66,6 +69,7 @@ struct AnyRequestVTable {
   const std::function<void(const void *, llvm::raw_ostream &)> simpleDisplay;
   const std::function<void(const void *, DiagnosticEngine &)> diagnoseCycle;
   const std::function<void(const void *, DiagnosticEngine &)> noteCycleStep;
+  const std::function<SourceLoc(const void *)> getNearestLoc;
 
   template <typename Request>
   static const AnyRequestVTable *get() {
@@ -75,7 +79,8 @@ struct AnyRequestVTable {
         &Impl<Request>::isEqual,
         &Impl<Request>::simpleDisplay,
         &Impl<Request>::diagnoseCycle,
-        &Impl<Request>::noteCycleStep
+        &Impl<Request>::noteCycleStep,
+        &Impl<Request>::getNearestLoc
     };
     return &vtable;
   }
@@ -173,6 +178,11 @@ class AnyRequestBase {
     getVTable()->noteCycleStep(getRawStorage(), diags);
   }
 
+  /// Get the best source location describing the parameters to this request.
+  SourceLoc getNearestLoc() const {
+    return getVTable()->getNearestLoc(getRawStorage());
+  }
+
   /// Compare two instances for equality.
   friend bool operator==(const AnyRequestBase<Derived> &lhs,
                          const AnyRequestBase<Derived> &rhs) {
@@ -224,6 +234,8 @@ class AnyRequestBase {
 ///   - Cycle diagnostics operations:
 ///       void diagnoseCycle(DiagnosticEngine &diags) const;
 ///       void noteCycleStep(DiagnosticEngine &diags) const;
+///   - Source location information:
+///       SourceLoc getNearestLoc() const;
 ///
 class ActiveRequest final : public AnyRequestBase<ActiveRequest> {
   template <typename T>

include/swift/AST/DiagnosticsSema.def

@@ -5342,7 +5342,7 @@ ERROR(actor_protocol_illegal_inheritance,none,
       "non-actor type %0 cannot conform to the %1 protocol",
       (DeclName, DeclName))
 ERROR(distributed_actor_conformance_missing_system_type,none,
-      "distributed actor %0 does not declare ActorSystem it can be used with.",
+      "distributed actor %0 does not declare ActorSystem it can be used with",
       (DeclName))
 NOTE(note_distributed_actor_system_can_be_defined_using_defaultdistributedactorsystem,none,
      "you can provide a module-wide default actor system by declaring:\n"

include/swift/AST/Evaluator.h

@@ -229,6 +229,11 @@ class Evaluator {
   /// diagnostics through the given diagnostics engine.
   Evaluator(DiagnosticEngine &diags, const LangOptions &opts);
 
+  /// For last-ditch diagnostics, get a good approximate source location for
+  /// the thing we're currently type checking by searching for a request whose
+  /// source location matches the predicate.
+  SourceLoc getInnermostSourceLoc(llvm::function_ref<bool(SourceLoc)> fn);
+
   /// Emit GraphViz output visualizing the request graph.
   void emitRequestEvaluatorGraphViz(llvm::StringRef graphVizPath);
 

include/swift/AST/Expr.h

@@ -6403,6 +6403,8 @@ void simple_display(llvm::raw_ostream &out, const DefaultArgumentExpr *expr);
 void simple_display(llvm::raw_ostream &out, const Expr *expr);
 
 SourceLoc extractNearestSourceLoc(const DefaultArgumentExpr *expr);
+SourceLoc extractNearestSourceLoc(const MacroExpansionExpr *expr);
+SourceLoc extractNearestSourceLoc(const ClosureExpr *expr);
 
 } // end namespace swift
 

include/swift/AST/ProtocolConformanceRef.h

@@ -213,11 +213,6 @@ class ProtocolConformanceRef {
   /// Create a canonical conformance from the current one.
   ProtocolConformanceRef getCanonicalConformanceRef() const;
 
-  /// Get any additional requirements that are required for this conformance to
-  /// be satisfied, if they're possible to compute.
-  llvm::Optional<ArrayRef<Requirement>>
-  getConditionalRequirementsIfAvailable() const;
-
   /// Get any additional requirements that are required for this conformance to
   /// be satisfied.
   ArrayRef<Requirement> getConditionalRequirements() const;

lib/AST/ASTContext.cpp

@@ -192,6 +192,28 @@ template <> struct DenseMapInfo<OverrideSignatureKey> {
 };
 } // namespace llvm
 
+namespace {
+
+/// If the conformance is in a primary file, we might diagnose some failures
+/// early via request evaluation, with all remaining failures diagnosed when
+/// we completely force the conformance from typeCheckDecl(). To emit the
+/// diagnostics together, we batch them up in the Diags vector.
+///
+/// If the conformance is in a secondary file, we instead just diagnose a
+/// generic "T does not conform to P" error the first time we hit an error
+/// via request evaluation. The detailed delayed conformance diagnostics
+/// are discarded, since we'll emit them again when we compile the file as
+/// a primary file.
+struct DelayedConformanceDiags {
+  /// We set this if we've ever seen an error diagnostic here.
+  bool HadError = false;
+
+  /// The delayed conformance diagnostics that have not been emitted yet.
+  /// Never actually emitted for a secondary file.
+  std::vector<ASTContext::DelayedConformanceDiag> Diags;
+};
+
+}
 struct ASTContext::Implementation {
   Implementation();
   ~Implementation();
@@ -354,8 +376,7 @@ struct ASTContext::Implementation {
 
   /// Map from normal protocol conformances to diagnostics that have
   /// been delayed until the conformance is fully checked.
-  llvm::DenseMap<NormalProtocolConformance *,
-                 std::vector<ASTContext::DelayedConformanceDiag>>
+  llvm::DenseMap<NormalProtocolConformance *, ::DelayedConformanceDiags>
     DelayedConformanceDiags;
 
   /// Map from normal protocol conformances to missing witnesses that have
@@ -1865,7 +1886,7 @@ void ASTContext::addCleanup(std::function<void(void)> cleanup) {
 }
 
 bool ASTContext::hadError() const {
-  return Diags.hadAnyError();
+  return Diags.hadAnyError() || hasDelayedConformanceErrors();
 }
 
 /// Retrieve the arena from which we should allocate storage for a type.
@@ -2735,25 +2756,18 @@ LazyIterableDeclContextData *ASTContext::getOrCreateLazyIterableContextData(
 bool ASTContext::hasDelayedConformanceErrors(
                           NormalProtocolConformance const* conformance) const {
 
-  auto hasDelayedErrors = [](std::vector<DelayedConformanceDiag> const& diags) {
-    return std::any_of(diags.begin(), diags.end(),
-                    [](ASTContext::DelayedConformanceDiag const& diag) {
-                      return diag.IsError;
-                    });
-  };
-
   if (conformance) {
     auto entry = getImpl().DelayedConformanceDiags.find(conformance);
     if (entry != getImpl().DelayedConformanceDiags.end())
-      return hasDelayedErrors(entry->second);
+      return entry->second.HadError;
 
     return false; // unknown conformance, so no delayed diags either.
   }
 
   // check all conformances for any delayed errors
   for (const auto &entry : getImpl().DelayedConformanceDiags) {
     auto const& diagnostics = entry.getSecond();
-    if (hasDelayedErrors(diagnostics))
+    if (diagnostics.HadError)
       return true;
   }
 
@@ -2763,9 +2777,49 @@ bool ASTContext::hasDelayedConformanceErrors(
 MissingWitnessesBase::~MissingWitnessesBase() { }
 
 void ASTContext::addDelayedConformanceDiag(
-       NormalProtocolConformance *conformance,
-       DelayedConformanceDiag fn) {
-  getImpl().DelayedConformanceDiags[conformance].push_back(std::move(fn));
+       NormalProtocolConformance *conformance, bool isError,
+       std::function<void(NormalProtocolConformance *)> callback) {
+  if (isError)
+    conformance->setInvalid();
+
+  auto &diagnostics = getImpl().DelayedConformanceDiags[conformance];
+
+  if (isError && !diagnostics.HadError) {
+    diagnostics.HadError = true;
+
+    auto *proto = conformance->getProtocol();
+    auto *dc = conformance->getDeclContext();
+    auto *sf = dc->getParentSourceFile();
+    auto *mod = sf->getParentModule();
+    assert(mod->isMainModule());
+
+    // 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 *otherSF = mod->getSourceFileContainingLocation(loc);
+        if (otherSF == nullptr)
+          return false;
+
+        return otherSF->isPrimary();
+      });
+
+      if (complainLoc.isInvalid()) {
+        complainLoc = conformance->getLoc();
+      }
+
+      Diags.diagnose(complainLoc,
+                     diag::type_does_not_conform,
+                     dc->getSelfInterfaceType(),
+                     proto->getDeclaredInterfaceType());
+    }
+  }
+
+  diagnostics.Diags.push_back({isError, callback});
 }
 
 void ASTContext::addDelayedMissingWitnesses(
@@ -2791,8 +2845,7 @@ ASTContext::takeDelayedConformanceDiags(NormalProtocolConformance const* cnfrm){
   std::vector<ASTContext::DelayedConformanceDiag> result;
   auto known = getImpl().DelayedConformanceDiags.find(cnfrm);
   if (known != getImpl().DelayedConformanceDiags.end()) {
-    result = std::move(known->second);
-    getImpl().DelayedConformanceDiags.erase(known);
+    std::swap(result, known->second.Diags);
   }
   return result;
 }

lib/AST/Evaluator.cpp

@@ -60,6 +60,17 @@ Evaluator::Evaluator(DiagnosticEngine &diags, const LangOptions &opts)
       debugDumpCycles(opts.DebugDumpCycles),
       recorder(opts.RecordRequestReferences) {}
 
+SourceLoc Evaluator::getInnermostSourceLoc(
+    llvm::function_ref<bool(SourceLoc)> fn) {
+  for (auto request : llvm::reverse(activeRequests)) {
+    SourceLoc loc = request.getNearestLoc();
+    if (fn(loc))
+      return loc;
+  }
+
+  return SourceLoc();
+}
+
 bool Evaluator::checkDependency(const ActiveRequest &request) {
   // Record this as an active request.
   if (activeRequests.insert(request)) {

lib/AST/Expr.cpp

@@ -2835,10 +2835,18 @@ void swift::simple_display(llvm::raw_ostream &out,
   out << "expression";
 }
 
+SourceLoc swift::extractNearestSourceLoc(const ClosureExpr *expr) {
+  return expr->getLoc();
+}
+
 SourceLoc swift::extractNearestSourceLoc(const DefaultArgumentExpr *expr) {
   return expr->getLoc();
 }
 
+SourceLoc swift::extractNearestSourceLoc(const MacroExpansionExpr *expr) {
+  return expr->getLoc();
+}
+
 // See swift/Basic/Statistic.h for declaration: this enables tracing Exprs, is
 // defined here to avoid too much layering violation / circular linkage
 // dependency.

lib/AST/ProtocolConformance.cpp

@@ -483,12 +483,6 @@ NormalProtocolConformance::getTypeWitnessAndDecl(AssociatedTypeDecl *assocType,
     return { Type(), nullptr };
   }
 
-  // If the conditional requirements aren't known, we can't properly run
-  // inference.
-  if (!getConditionalRequirementsIfAvailable()) {
-    return TypeWitnessAndDecl();
-  }
-
   return evaluateOrDefault(
       assocType->getASTContext().evaluator,
       TypeWitnessRequest{const_cast<NormalProtocolConformance *>(this),

lib/AST/ProtocolConformanceRef.cpp

@@ -189,17 +189,6 @@ ProtocolConformanceRef::getWitnessByName(Type type, DeclName name) const {
   return getConcrete()->getWitnessDeclRef(requirement);
 }
 
-llvm::Optional<ArrayRef<Requirement>>
-ProtocolConformanceRef::getConditionalRequirementsIfAvailable() const {
-  if (isConcrete())
-    return getConcrete()->getConditionalRequirementsIfAvailable();
-  else
-    // An abstract conformance is never conditional: any conditionality in the
-    // concrete types that will eventually pass through this at runtime is
-    // completely pre-checked and packaged up.
-    return ArrayRef<Requirement>();
-}
-
 ArrayRef<Requirement>
 ProtocolConformanceRef::getConditionalRequirements() const {
   if (isConcrete())

lib/Sema/ConstraintSystem.cpp

@@ -7235,7 +7235,7 @@ void ConstraintSystem::maybeProduceFallbackDiagnostic(
   // diagnostics already emitted or waiting to be emitted. Because they are
   // a better indication of the problem.
   ASTContext &ctx = getASTContext();
-  if (ctx.Diags.hadAnyError() || ctx.hasDelayedConformanceErrors())
+  if (ctx.hadError())
     return;
 
   ctx.Diags.diagnose(target.getLoc(), diag::failed_to_produce_diagnostic);