From f91767cfa06aa60076e1487ccd444daea8957c24 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 18 Sep 2020 15:37:05 -0700 Subject: [PATCH 1/2] [Conformance checker] Capture potential matches for missing witnesses. Rework the data structures we use in the conformance checker to talk about missing witnesses, so they can capture the set of potential matches. This will allow us to delay more diagnostics to later, more uniformly. --- include/swift/AST/ASTContext.h | 17 +++++- lib/AST/ASTContext.cpp | 21 ++++--- lib/Sema/CSDiagnostics.cpp | 4 +- lib/Sema/TypeCheckProtocol.cpp | 74 ++++++++++++++----------- lib/Sema/TypeCheckProtocol.h | 53 +++++++++++++++++- lib/Sema/TypeCheckProtocolInference.cpp | 6 +- 6 files changed, 125 insertions(+), 50 deletions(-) diff --git a/include/swift/AST/ASTContext.h b/include/swift/AST/ASTContext.h index fd020c6eb703b..e366bbef30bd0 100644 --- a/include/swift/AST/ASTContext.h +++ b/include/swift/AST/ASTContext.h @@ -197,6 +197,16 @@ class ConstraintCheckerArenaRAII { class SILLayout; // From SIL +/// A set of missing witnesses for a given conformance. These are temporarily +/// stashed in the ASTContext so the type checker can get at them. +/// +/// The only subclass is owned by the type checker, so it can hide its own +/// data structures. +class MissingWitnessesBase { +public: + virtual ~MissingWitnessesBase(); +}; + /// ASTContext - This object creates and owns the AST objects. /// However, this class does more than just maintain context within an AST. /// It is the closest thing to thread-local or compile-local storage in this @@ -943,12 +953,13 @@ class ASTContext final { takeDelayedConformanceDiags(NormalProtocolConformance *conformance); /// Add delayed missing witnesses for the given normal protocol conformance. - void addDelayedMissingWitnesses(NormalProtocolConformance *conformance, - ArrayRef witnesses); + void addDelayedMissingWitnesses( + NormalProtocolConformance *conformance, + std::unique_ptr missingWitnesses); /// Retrieve the delayed missing witnesses for the given normal protocol /// conformance. - std::vector + std::unique_ptr takeDelayedMissingWitnesses(NormalProtocolConformance *conformance); /// Produce a specialized conformance, which takes a generic diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index 21703f4929ce9..0bcf1792edead 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -298,7 +298,8 @@ struct ASTContext::Implementation { /// 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> + llvm::DenseMap< + NormalProtocolConformance *, std::unique_ptr> DelayedMissingWitnesses; /// Stores information about lazy deserialization of various declarations. @@ -2129,22 +2130,24 @@ bool ASTContext::hasDelayedConformanceErrors() const { return false; } +MissingWitnessesBase::~MissingWitnessesBase() { } + void ASTContext::addDelayedConformanceDiag( NormalProtocolConformance *conformance, DelayedConformanceDiag fn) { getImpl().DelayedConformanceDiags[conformance].push_back(std::move(fn)); } -void ASTContext:: -addDelayedMissingWitnesses(NormalProtocolConformance *conformance, - ArrayRef witnesses) { - auto &bucket = getImpl().DelayedMissingWitnesses[conformance]; - bucket.insert(bucket.end(), witnesses.begin(), witnesses.end()); +void ASTContext::addDelayedMissingWitnesses( + NormalProtocolConformance *conformance, + std::unique_ptr missingWitnesses) { + getImpl().DelayedMissingWitnesses[conformance] = std::move(missingWitnesses); } -std::vector ASTContext:: -takeDelayedMissingWitnesses(NormalProtocolConformance *conformance) { - std::vector result; +std::unique_ptr +ASTContext::takeDelayedMissingWitnesses( + NormalProtocolConformance *conformance) { + std::unique_ptr result; auto known = getImpl().DelayedMissingWitnesses.find(conformance); if (known != getImpl().DelayedMissingWitnesses.end()) { result = std::move(known->second); diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index 822ede889e55a..06931632c330d 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -2864,7 +2864,7 @@ bool ContextualFailure::tryProtocolConformanceFixIt( { llvm::SmallString<128> Text; llvm::raw_svector_ostream SS(Text); - llvm::SetVector missingWitnesses; + llvm::SetVector missingWitnesses; for (auto protocol : missingProtocols) { auto conformance = NormalProtocolConformance( nominal->getDeclaredType(), protocol, SourceLoc(), nominal, @@ -2876,7 +2876,7 @@ bool ContextualFailure::tryProtocolConformanceFixIt( } for (auto decl : missingWitnesses) { - swift::printRequirementStub(decl, nominal, nominal->getDeclaredType(), + swift::printRequirementStub(decl.requirement, nominal, nominal->getDeclaredType(), nominal->getStartLoc(), SS); } diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 60b25436f0c3a..165262cdac770 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -1499,7 +1499,7 @@ class swift::MultiConformanceChecker { llvm::SmallVector UnsatisfiedReqs; llvm::SmallVector AllUsedCheckers; llvm::SmallVector AllConformances; - llvm::SetVector MissingWitnesses; + llvm::SetVector MissingWitnesses; llvm::SmallPtrSet CoveredMembers; /// Check one conformance. @@ -1729,13 +1729,17 @@ checkIndividualConformance(NormalProtocolConformance *conformance, PrettyStackTraceConformance trace(getASTContext(), "type-checking", conformance); - std::vector revivedMissingWitnesses; + std::vector revivedMissingWitnesses; switch (conformance->getState()) { case ProtocolConformanceState::Incomplete: if (conformance->isInvalid()) { // Revive registered missing witnesses to handle it below. - revivedMissingWitnesses = - getASTContext().takeDelayedMissingWitnesses(conformance); + if (auto delayed = getASTContext().takeDelayedMissingWitnesses( + conformance)) { + revivedMissingWitnesses = std::move( + static_cast( + delayed.get())->missingWitnesses); + } // If we have no missing witnesses for this invalid conformance, the // conformance is invalid for other reasons, so emit diagnosis now. @@ -2481,7 +2485,7 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance, ConformanceChecker::ConformanceChecker( ASTContext &ctx, NormalProtocolConformance *conformance, - llvm::SetVector &GlobalMissingWitnesses, + llvm::SetVector &GlobalMissingWitnesses, bool suppressDiagnostics) : WitnessChecker(ctx, conformance->getProtocol(), conformance->getType(), conformance->getDeclContext()), @@ -2986,15 +2990,16 @@ printRequirementStub(ValueDecl *Requirement, DeclContext *Adopter, /// NoStubRequirements. static void printProtocolStubFixitString(SourceLoc TypeLoc, ProtocolConformance *Conf, - ArrayRef MissingWitnesses, + ArrayRef MissingWitnesses, std::string &FixitString, llvm::SetVector &NoStubRequirements) { llvm::raw_string_ostream FixitStream(FixitString); std::for_each(MissingWitnesses.begin(), MissingWitnesses.end(), - [&](ValueDecl* VD) { - if (!printRequirementStub(VD, Conf->getDeclContext(), Conf->getType(), - TypeLoc, FixitStream)) { - NoStubRequirements.insert(VD); + [&](const MissingWitness &Missing) { + if (!printRequirementStub( + Missing.requirement, Conf->getDeclContext(), Conf->getType(), + TypeLoc, FixitStream)) { + NoStubRequirements.insert(Missing.requirement); } }); } @@ -3002,10 +3007,11 @@ 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 -filterProtocolRequirements(ArrayRef Reqs, Type Adoptee) { - llvm::SmallVector Filtered; - if (Reqs.empty()) { +static llvm::SmallVector +filterProtocolRequirements( + ArrayRef MissingWitnesses, Type Adoptee) { + llvm::SmallVector Filtered; + if (MissingWitnesses.empty()) { return Filtered; } @@ -3017,10 +3023,11 @@ filterProtocolRequirements(ArrayRef Reqs, Type Adoptee) { llvm::SmallDenseMap, 4> DeclsByName; - for (auto *const Req : Reqs) { + for (const auto &Missing: MissingWitnesses) { + auto Req = Missing.requirement; if (DeclsByName.find(Req->getName()) == DeclsByName.end()) { DeclsByName[Req->getName()] = {Req}; - Filtered.push_back(Req); + Filtered.push_back(Missing); continue; } @@ -3046,7 +3053,7 @@ filterProtocolRequirements(ArrayRef Reqs, Type Adoptee) { } DeclsByName[Req->getName()].push_back(Req); - Filtered.push_back(Req); + Filtered.push_back(Missing); } return Filtered; @@ -3060,10 +3067,9 @@ diagnoseMissingWitnesses(MissingWitnessDiagnosisKind Kind) { if (LocalMissing.empty()) return; - const auto InsertFixit = [](NormalProtocolConformance *Conf, - SourceLoc ComplainLoc, bool EditorMode, - llvm::SmallVector - MissingWitnesses) { + const auto InsertFixit = []( + NormalProtocolConformance *Conf, SourceLoc ComplainLoc, bool EditorMode, + llvm::SmallVector MissingWitnesses) { DeclContext *DC = Conf->getDeclContext(); // The location where to insert stubs. SourceLoc FixitLocation; @@ -3097,7 +3103,9 @@ diagnoseMissingWitnesses(MissingWitnessDiagnosisKind Kind) { } auto &SM = DC->getASTContext().SourceMgr; auto FixitBufferId = SM.findBufferContainingLoc(FixitLocation); - for (auto VD : MissingWitnesses) { + for (const auto &Missing : MissingWitnesses) { + auto VD = Missing.requirement; + // Don't ever emit a diagnostic for a requirement in the NSObject // protocol. They're not implementable. if (isNSObjectProtocol(VD->getDeclContext()->getSelfProtocolDecl())) @@ -3167,10 +3175,13 @@ diagnoseMissingWitnesses(MissingWitnessDiagnosisKind Kind) { // If the diagnostics are suppressed, we register these missing witnesses // for later revisiting. Conformance->setInvalid(); - getASTContext().addDelayedMissingWitnesses(Conformance, MissingWitnesses); + getASTContext().addDelayedMissingWitnesses( + Conformance, + std::make_unique(MissingWitnesses)); } else { diagnoseOrDefer( - LocalMissing[0], true, [&](NormalProtocolConformance *Conf) { + LocalMissing[0].requirement, true, + [&](NormalProtocolConformance *Conf) { InsertFixit(Conf, Loc, IsEditorMode, std::move(MissingWitnesses)); }); } @@ -3178,7 +3189,8 @@ diagnoseMissingWitnesses(MissingWitnessDiagnosisKind Kind) { return; } case MissingWitnessDiagnosisKind::ErrorOnly: { - diagnoseOrDefer(LocalMissing[0], true, [](NormalProtocolConformance *) {}); + diagnoseOrDefer( + LocalMissing[0].requirement, true, [](NormalProtocolConformance *) {}); return; } case MissingWitnessDiagnosisKind::FixItOnly: @@ -3683,7 +3695,7 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) { return ResolveWitnessResult::Missing; } - // Diagnose the error. + // Diagnose the error. // If there was an invalid witness that might have worked, just // suppress the diagnostic entirely. This stops the diagnostic cascade. @@ -3695,7 +3707,7 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) { if (!numViable) { // Save the missing requirement for later diagnosis. - GlobalMissingWitnesses.insert(requirement); + GlobalMissingWitnesses.insert({requirement, matches}); diagnoseOrDefer(requirement, true, [requirement, matches, nominal](NormalProtocolConformance *conformance) { auto dc = conformance->getDeclContext(); @@ -3789,7 +3801,7 @@ ResolveWitnessResult ConformanceChecker::resolveWitnessViaDefault( return ResolveWitnessResult::Success; } // Save the missing requirement for later diagnosis. - GlobalMissingWitnesses.insert(requirement); + GlobalMissingWitnesses.insert({requirement, {}}); return ResolveWitnessResult::ExplicitFailed; } @@ -4014,7 +4026,7 @@ ResolveWitnessResult ConformanceChecker::resolveTypeWitnessViaLookup( return ResolveWitnessResult::ExplicitFailed; } // Save the missing type witness for later diagnosis. - GlobalMissingWitnesses.insert(assocType); + GlobalMissingWitnesses.insert({assocType, {}}); // None of the candidates were viable. diagnoseOrDefer(assocType, true, @@ -5670,7 +5682,7 @@ TypeWitnessAndDecl TypeWitnessRequest::evaluate(Evaluator &eval, NormalProtocolConformance *conformance, AssociatedTypeDecl *requirement) const { - llvm::SetVector MissingWitnesses; + llvm::SetVector MissingWitnesses; ConformanceChecker checker(requirement->getASTContext(), conformance, MissingWitnesses); checker.resolveSingleTypeWitness(requirement); @@ -5688,7 +5700,7 @@ Witness ValueWitnessRequest::evaluate(Evaluator &eval, NormalProtocolConformance *conformance, ValueDecl *requirement) const { - llvm::SetVector MissingWitnesses; + llvm::SetVector MissingWitnesses; ConformanceChecker checker(requirement->getASTContext(), conformance, MissingWitnesses); checker.resolveSingleWitness(requirement); diff --git a/lib/Sema/TypeCheckProtocol.h b/lib/Sema/TypeCheckProtocol.h index d51341300ac48..e850e5ae222d6 100644 --- a/lib/Sema/TypeCheckProtocol.h +++ b/lib/Sema/TypeCheckProtocol.h @@ -643,6 +643,31 @@ 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 matches; + + MissingWitness(ValueDecl *requirement, + ArrayRef 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 missingWitnesses; + + DelayedMissingWitnesses(ArrayRef missingWitnesses) + : missingWitnesses(missingWitnesses.begin(), missingWitnesses.end()) { } +}; + /// The protocol conformance checker. /// /// This helper class handles most of the details of checking whether a @@ -665,7 +690,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 &GlobalMissingWitnesses; + llvm::SetVector &GlobalMissingWitnesses; /// Keep track of the slice in GlobalMissingWitnesses that is local to /// this protocol under checking. @@ -746,7 +771,7 @@ class ConformanceChecker : public WitnessChecker { ValueDecl *requirement, bool isError, std::function fn); - ArrayRef getLocalMissingWitness() { + ArrayRef getLocalMissingWitness() { return GlobalMissingWitnesses.getArrayRef(). slice(LocalMissingWitnessesStartIndex, GlobalMissingWitnesses.size() - LocalMissingWitnessesStartIndex); @@ -764,7 +789,7 @@ class ConformanceChecker : public WitnessChecker { void emitDelayedDiags(); ConformanceChecker(ASTContext &ctx, NormalProtocolConformance *conformance, - llvm::SetVector &GlobalMissingWitnesses, + llvm::SetVector &GlobalMissingWitnesses, bool suppressDiagnostics = true); /// Resolve all of the type witnesses. @@ -1030,4 +1055,26 @@ void diagnoseConformanceFailure(Type T, } +namespace llvm { + +template<> +struct DenseMapInfo { + using MissingWitness = swift::MissingWitness; + using RequirementPointerTraits = DenseMapInfo; + + static inline MissingWitness getEmptyKey() { + return MissingWitness(RequirementPointerTraits::getEmptyKey(), {}); + } + static inline MissingWitness getTombstoneKey() { + return MissingWitness(RequirementPointerTraits::getTombstoneKey(), {}); + } + static inline unsigned getHashValue(MissingWitness missing) { + return RequirementPointerTraits::getHashValue(missing.requirement); + } + static bool isEqual(MissingWitness a, MissingWitness b) { + return a.requirement == b.requirement; + } +}; + +} #endif // SWIFT_SEMA_PROTOCOL_H diff --git a/lib/Sema/TypeCheckProtocolInference.cpp b/lib/Sema/TypeCheckProtocolInference.cpp index 23a6957b930ef..6fdb2ffbeb3aa 100644 --- a/lib/Sema/TypeCheckProtocolInference.cpp +++ b/lib/Sema/TypeCheckProtocolInference.cpp @@ -2010,8 +2010,10 @@ auto AssociatedTypeInference::solve(ConformanceChecker &checker) return None; // Save the missing type witnesses for later diagnosis. - checker.GlobalMissingWitnesses.insert(unresolvedAssocTypes.begin(), - unresolvedAssocTypes.end()); + for (auto assocType : unresolvedAssocTypes) { + checker.GlobalMissingWitnesses.insert({assocType, {}}); + } + return None; } From f956e4f7041d48529eee12a87cbfc9b187338099 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 18 Sep 2020 17:40:44 -0700 Subject: [PATCH 2/2] [Conformance checker] Eliminate redundant recording of missing witnesses. --- lib/Sema/TypeCheckProtocol.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 165262cdac770..488bc0b8fb675 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -3800,8 +3800,7 @@ ResolveWitnessResult ConformanceChecker::resolveWitnessViaDefault( recordOptionalWitness(requirement); return ResolveWitnessResult::Success; } - // Save the missing requirement for later diagnosis. - GlobalMissingWitnesses.insert({requirement, {}}); + return ResolveWitnessResult::ExplicitFailed; }