Sema: Replace ASTContext::addDelayedMissingWitnesses() with ASTContext::addDelayedMissingWitness()

Instead of passing a unique_ptr of an opaque type back and forth,
let's just push elements onto an std::vector. For now this change
is completely NFC, but further simplifications will become
possible shortly.

Author: slavapestov

include/swift/AST/ASTContext.h

@@ -123,6 +123,7 @@ namespace swift {
   class Pattern;
   enum PointerTypeKind : unsigned;
   class PrecedenceGroupDecl;
+  struct RequirementMatch;
   class TupleTypeElt;
   class EnumElementDecl;
   class ProtocolDecl;
@@ -1317,6 +1318,19 @@ class ASTContext final {
     std::function<void(NormalProtocolConformance *)> Callback;
   };
 
+  /// Describes a missing witness during conformance checking.
+  class MissingWitness {
+  public:
+    /// The requirement that is missing a witness.
+    ValueDecl *requirement;
+
+    /// The set of potential matching witnesses.
+    std::vector<RequirementMatch> matches;
+
+    MissingWitness(ValueDecl *requirement,
+                   ArrayRef<RequirementMatch> matches);
+  };
+
   /// Check whether current context has any errors associated with
   /// ill-formed protocol conformances which haven't been produced yet.
   ///
@@ -1339,13 +1353,13 @@ class ASTContext final {
   takeDelayedConformanceDiags(NormalProtocolConformance const* conformance);
 
   /// Add delayed missing witnesses for the given normal protocol conformance.
-  void addDelayedMissingWitnesses(
+  void addDelayedMissingWitness(
       NormalProtocolConformance *conformance,
-      std::unique_ptr<MissingWitnessesBase> missingWitnesses);
+      MissingWitness missingWitness);
 
   /// Retrieve the delayed missing witnesses for the given normal protocol
   /// conformance.
-  std::unique_ptr<MissingWitnessesBase>
+  std::vector<MissingWitness>
   takeDelayedMissingWitnesses(NormalProtocolConformance *conformance);
 
   /// Produce a specialized conformance, which takes a generic

lib/AST/ASTContext.cpp

@@ -46,6 +46,7 @@
 #include "swift/AST/PropertyWrappers.h"
 #include "swift/AST/ProtocolConformance.h"
 #include "swift/AST/RawComment.h"
+#include "swift/AST/RequirementMatch.h"
 #include "swift/AST/SILLayout.h"
 #include "swift/AST/SearchPathOptions.h"
 #include "swift/AST/SemanticAttrs.h"
@@ -205,12 +206,19 @@ namespace {
 /// 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;
+
+  /// Any missing witnesses that need to be diagnosed.
+  std::vector<ASTContext::MissingWitness> MissingWitnesses;
+
+  /// We set this if we've ever seen an error diagnostic here.
+  unsigned HadError : 1;
+
+  DelayedConformanceDiags() {
+    HadError = false;
+  }
 };
 
 }
@@ -379,13 +387,6 @@ struct ASTContext::Implementation {
   llvm::DenseMap<NormalProtocolConformance *, ::DelayedConformanceDiags>
     DelayedConformanceDiags;
 
-  /// Map from normal protocol conformances to missing witnesses that have
-  /// been delayed until the conformance is fully checked, so that we can
-  /// issue a fixit that fills the entire protocol stub.
-  llvm::DenseMap<
-      NormalProtocolConformance *, std::unique_ptr<MissingWitnessesBase>>
-    DelayedMissingWitnesses;
-
   /// Stores information about lazy deserialization of various declarations.
   llvm::DenseMap<const Decl *, LazyContextData *> LazyContexts;
 
@@ -2774,7 +2775,10 @@ bool ASTContext::hasDelayedConformanceErrors(
   return false;
 }
 
-MissingWitnessesBase::~MissingWitnessesBase() { }
+ASTContext::MissingWitness::MissingWitness(ValueDecl *requirement,
+                                           ArrayRef<RequirementMatch> matches)
+  : requirement(requirement),
+    matches(matches.begin(), matches.end()) { }
 
 void ASTContext::addDelayedConformanceDiag(
        NormalProtocolConformance *conformance, bool isError,
@@ -2822,20 +2826,21 @@ void ASTContext::addDelayedConformanceDiag(
   diagnostics.Diags.push_back({isError, callback});
 }
 
-void ASTContext::addDelayedMissingWitnesses(
+void ASTContext::addDelayedMissingWitness(
     NormalProtocolConformance *conformance,
-    std::unique_ptr<MissingWitnessesBase> missingWitnesses) {
-  getImpl().DelayedMissingWitnesses[conformance] = std::move(missingWitnesses);
+    ASTContext::MissingWitness missingWitness) {
+  auto &diagnostics = getImpl().DelayedConformanceDiags[conformance];
+  diagnostics.MissingWitnesses.push_back(missingWitness);
 }
 
-std::unique_ptr<MissingWitnessesBase>
+std::vector<ASTContext::MissingWitness>
 ASTContext::takeDelayedMissingWitnesses(
     NormalProtocolConformance *conformance) {
-  std::unique_ptr<MissingWitnessesBase> result;
-  auto known = getImpl().DelayedMissingWitnesses.find(conformance);
-  if (known != getImpl().DelayedMissingWitnesses.end()) {
-    result = std::move(known->second);
-    getImpl().DelayedMissingWitnesses.erase(known);
+  std::vector<ASTContext::MissingWitness> result;
+  auto known = getImpl().DelayedConformanceDiags.find(conformance);
+  if (known != getImpl().DelayedConformanceDiags.end()) {
+    auto &diagnostics = known->second;
+    std::swap(result, diagnostics.MissingWitnesses);
   }
   return result;
 }
@@ -2845,7 +2850,8 @@ ASTContext::takeDelayedConformanceDiags(NormalProtocolConformance const* cnfrm){
   std::vector<ASTContext::DelayedConformanceDiag> result;
   auto known = getImpl().DelayedConformanceDiags.find(cnfrm);
   if (known != getImpl().DelayedConformanceDiags.end()) {
-    std::swap(result, known->second.Diags);
+    auto &diagnostics = known->second;
+    std::swap(result, diagnostics.Diags);
   }
   return result;
 }

lib/Sema/CSDiagnostics.cpp

@@ -3453,7 +3453,7 @@ bool ContextualFailure::tryProtocolConformanceFixIt(
   {
     llvm::SmallString<128> Text;
     llvm::raw_svector_ostream SS(Text);
-    llvm::SetVector<MissingWitness> missingWitnesses;
+    llvm::SetVector<ASTContext::MissingWitness> missingWitnesses;
     for (auto protocol : missingProtocols) {
       auto conformance = NormalProtocolConformance(
           nominal->getDeclaredType(), protocol, SourceLoc(), nominal,

lib/Sema/TypeCheckProtocol.cpp

@@ -1802,7 +1802,7 @@ class swift::MultiConformanceChecker {
   llvm::SmallVector<ValueDecl*, 16> UnsatisfiedReqs;
   llvm::SmallVector<ConformanceChecker, 4> AllUsedCheckers;
   llvm::SmallVector<NormalProtocolConformance*, 4> AllConformances;
-  llvm::SetVector<MissingWitness> MissingWitnesses;
+  llvm::SetVector<ASTContext::MissingWitness> MissingWitnesses;
   llvm::SmallPtrSet<ValueDecl *, 8> CoveredMembers;
 
   /// Check one conformance.
@@ -2068,17 +2068,13 @@ checkIndividualConformance(NormalProtocolConformance *conformance,
                            bool issueFixit) {
   PrettyStackTraceConformance trace("type-checking", conformance);
 
-  std::vector<MissingWitness> revivedMissingWitnesses;
+  std::vector<ASTContext::MissingWitness> revivedMissingWitnesses;
   switch (conformance->getState()) {
     case ProtocolConformanceState::Incomplete:
       if (conformance->isInvalid()) {
         // Revive registered missing witnesses to handle it below.
-        if (auto delayed = getASTContext().takeDelayedMissingWitnesses(
-                conformance)) {
-          revivedMissingWitnesses = std::move(
-              static_cast<DelayedMissingWitnesses *>(
-                delayed.get())->missingWitnesses);
-        }
+        revivedMissingWitnesses = getASTContext().takeDelayedMissingWitnesses(
+            conformance);
 
         // If we have no missing witnesses for this invalid conformance, the
         // conformance is invalid for other reasons, so emit diagnosis now.
@@ -2875,7 +2871,7 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance,
 
 ConformanceChecker::ConformanceChecker(
     ASTContext &ctx, NormalProtocolConformance *conformance,
-    llvm::SetVector<MissingWitness> &GlobalMissingWitnesses)
+    llvm::SetVector<ASTContext::MissingWitness> &GlobalMissingWitnesses)
     : WitnessChecker(ctx, conformance->getProtocol(), conformance->getType(),
                      conformance->getDeclContext()),
       Conformance(conformance), Loc(conformance->getLoc()),
@@ -3662,12 +3658,12 @@ printRequirementStub(ValueDecl *Requirement, DeclContext *Adopter,
 /// NoStubRequirements.
 static void
 printProtocolStubFixitString(SourceLoc TypeLoc, ProtocolConformance *Conf,
-                             ArrayRef<MissingWitness> MissingWitnesses,
+                             ArrayRef<ASTContext::MissingWitness> MissingWitnesses,
                              std::string &FixitString,
                              llvm::SetVector<ValueDecl*> &NoStubRequirements) {
   llvm::raw_string_ostream FixitStream(FixitString);
   std::for_each(MissingWitnesses.begin(), MissingWitnesses.end(),
-    [&](const MissingWitness &Missing) {
+    [&](const ASTContext::MissingWitness &Missing) {
       if (!printRequirementStub(
               Missing.requirement, Conf->getDeclContext(), Conf->getType(),
               TypeLoc, FixitStream)) {
@@ -3679,10 +3675,10 @@ printProtocolStubFixitString(SourceLoc TypeLoc, ProtocolConformance *Conf,
 /// Filter the given array of protocol requirements and produce a new vector
 /// containing the non-conflicting requirements to be implemented by the given
 /// \c Adoptee type.
-static llvm::SmallVector<MissingWitness, 4>
+static llvm::SmallVector<ASTContext::MissingWitness, 4>
 filterProtocolRequirements(
-    ArrayRef<MissingWitness> MissingWitnesses, Type Adoptee) {
-  llvm::SmallVector<MissingWitness, 4> Filtered;
+    ArrayRef<ASTContext::MissingWitness> MissingWitnesses, Type Adoptee) {
+  llvm::SmallVector<ASTContext::MissingWitness, 4> Filtered;
   if (MissingWitnesses.empty()) {
     return Filtered;
   }
@@ -3733,12 +3729,12 @@ filterProtocolRequirements(
 
 /// Prune the set of missing witnesses for the given conformance, eliminating
 /// any requirements that do not actually need to satisfied.
-static ArrayRef<MissingWitness> pruneMissingWitnesses(
+static ArrayRef<ASTContext::MissingWitness> pruneMissingWitnesses(
     ConformanceChecker &checker,
     ProtocolDecl *proto,
     NormalProtocolConformance *conformance,
-    ArrayRef<MissingWitness> missingWitnesses,
-    SmallVectorImpl<MissingWitness> &scratch) {
+    ArrayRef<ASTContext::MissingWitness> missingWitnesses,
+    SmallVectorImpl<ASTContext::MissingWitness> &scratch) {
   if (missingWitnesses.empty()) {
     return missingWitnesses;
   }
@@ -3831,7 +3827,7 @@ bool ConformanceChecker::
 diagnoseMissingWitnesses(MissingWitnessDiagnosisKind Kind, bool Delayed) {
   auto LocalMissing = getLocalMissingWitness();
 
-  SmallVector<MissingWitness, 4> MissingWitnessScratch;
+  SmallVector<ASTContext::MissingWitness, 4> MissingWitnessScratch;
   LocalMissing = pruneMissingWitnesses(
       *this, Proto, Conformance, LocalMissing, MissingWitnessScratch);
 
@@ -3860,7 +3856,7 @@ diagnoseMissingWitnesses(MissingWitnessDiagnosisKind Kind, bool Delayed) {
 
   const auto InsertFixit = [](
       NormalProtocolConformance *Conf, SourceLoc ComplainLoc, bool EditorMode,
-      llvm::SmallVector<MissingWitness, 4> MissingWitnesses) {
+      llvm::SmallVector<ASTContext::MissingWitness, 4> MissingWitnesses) {
     DeclContext *DC = Conf->getDeclContext();
     // The location where to insert stubs.
     SourceLoc FixitLocation;
@@ -3974,9 +3970,9 @@ diagnoseMissingWitnesses(MissingWitnessDiagnosisKind Kind, bool Delayed) {
       // If the diagnostics are suppressed, we register these missing witnesses
       // for later revisiting.
       Conformance->setInvalid();
-      getASTContext().addDelayedMissingWitnesses(
-          Conformance,
-          std::make_unique<DelayedMissingWitnesses>(MissingWitnesses));
+      for (auto Missing : MissingWitnesses) {
+        getASTContext().addDelayedMissingWitness(Conformance, Missing);
+      }
     } else {
       auto Loc = this->Loc;
       getASTContext().addDelayedConformanceDiag(Conformance, true,
@@ -6914,7 +6910,7 @@ TypeWitnessAndDecl
 TypeWitnessRequest::evaluate(Evaluator &eval,
                              NormalProtocolConformance *conformance,
                              AssociatedTypeDecl *requirement) const {
-  llvm::SetVector<MissingWitness> MissingWitnesses;
+  llvm::SetVector<ASTContext::MissingWitness> MissingWitnesses;
   ConformanceChecker checker(requirement->getASTContext(), conformance,
                              MissingWitnesses);
   checker.resolveSingleTypeWitness(requirement);
@@ -6933,7 +6929,7 @@ Witness
 ValueWitnessRequest::evaluate(Evaluator &eval,
                               NormalProtocolConformance *conformance,
                               ValueDecl *requirement) const {
-  llvm::SetVector<MissingWitness> MissingWitnesses;
+  llvm::SetVector<ASTContext::MissingWitness> MissingWitnesses;
   ConformanceChecker checker(requirement->getASTContext(), conformance,
                              MissingWitnesses);
   checker.resolveSingleWitness(requirement);

lib/Sema/TypeCheckProtocol.h

@@ -20,6 +20,7 @@
 
 #include "TypeChecker.h"
 #include "swift/AST/AccessScope.h"
+#include "swift/AST/ASTContext.h"
 #include "swift/AST/RequirementEnvironment.h"
 #include "swift/AST/RequirementMatch.h"
 #include "swift/AST/Type.h"
@@ -323,31 +324,6 @@ enum class MissingWitnessDiagnosisKind {
 class AssociatedTypeInference;
 class MultiConformanceChecker;
 
-/// Describes a missing witness during conformance checking.
-class MissingWitness {
-public:
-  /// The requirement that is missing a witness.
-  ValueDecl *requirement;
-
-  /// The set of potential matching witnesses.
-  std::vector<RequirementMatch> matches;
-
-  MissingWitness(ValueDecl *requirement,
-                 ArrayRef<RequirementMatch> matches)
-    : requirement(requirement),
-      matches(matches.begin(), matches.end()) { }
-};
-
-/// Capture missing witnesses that have been delayed and will be stored
-/// in the ASTContext for later.
-class DelayedMissingWitnesses : public MissingWitnessesBase {
-public:
-  std::vector<MissingWitness> missingWitnesses;
-
-  DelayedMissingWitnesses(ArrayRef<MissingWitness> missingWitnesses)
-      : missingWitnesses(missingWitnesses.begin(), missingWitnesses.end()) { }
-};
-
 /// The protocol conformance checker.
 ///
 /// This helper class handles most of the details of checking whether a
@@ -371,7 +347,7 @@ class ConformanceChecker : public WitnessChecker {
   /// Keep track of missing witnesses, either type or value, for later
   /// diagnosis emits. This may contain witnesses that are external to the
   /// protocol under checking.
-  llvm::SetVector<MissingWitness> &GlobalMissingWitnesses;
+  llvm::SetVector<ASTContext::MissingWitness> &GlobalMissingWitnesses;
 
   /// Keep track of the slice in GlobalMissingWitnesses that is local to
   /// this protocol under checking.
@@ -447,7 +423,7 @@ class ConformanceChecker : public WitnessChecker {
   /// the chosen type witnesses.
   void ensureRequirementsAreSatisfied();
 
-  ArrayRef<MissingWitness> getLocalMissingWitness() {
+  ArrayRef<ASTContext::MissingWitness> getLocalMissingWitness() {
     return GlobalMissingWitnesses.getArrayRef().
       slice(LocalMissingWitnessesStartIndex,
             GlobalMissingWitnesses.size() - LocalMissingWitnessesStartIndex);
@@ -469,7 +445,7 @@ class ConformanceChecker : public WitnessChecker {
   void emitDelayedDiags();
 
   ConformanceChecker(ASTContext &ctx, NormalProtocolConformance *conformance,
-                     llvm::SetVector<MissingWitness> &GlobalMissingWitnesses);
+                     llvm::SetVector<ASTContext::MissingWitness> &GlobalMissingWitnesses);
 
   ~ConformanceChecker();
 
@@ -895,8 +871,8 @@ void diagnoseConformanceFailure(Type T,
 namespace llvm {
 
 template<>
-struct DenseMapInfo<swift::MissingWitness> {
-  using MissingWitness = swift::MissingWitness;
+struct DenseMapInfo<swift::ASTContext::MissingWitness> {
+  using MissingWitness = swift::ASTContext::MissingWitness;
   using RequirementPointerTraits = DenseMapInfo<swift::ValueDecl *>;
 
   static inline MissingWitness getEmptyKey() {