From: Richard Smith Date: Mon, 7 May 2012 09:03:25 +0000 (+0000) Subject: When we suppress an error due to SFINAE, stash the diagnostic away with the X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b8590f3572158bde97f14037c4cc8f4a57c8d810;p=clang When we suppress an error due to SFINAE, stash the diagnostic away with the overload candidate, and include its message in any subsequent 'candidate not viable due to substitution failure' note we may produce. To keep the note small (since the 'overload resolution failed' diagnostics are often already very verbose), the text of the SFINAE diagnostic is included as part of the text of the note, and any notes which were attached to it are discarded. There happened to be spare space in OverloadCandidate into which a PartialDiagnosticAt could be squeezed, and this patch goes to lengths to avoid unnecessary PartialDiagnostic copies, resulting in no slowdown that I could measure. (Removal in passing of some PartialDiagnostic copies has resulted in a slightly smaller clang binary overall.) Even on a torture test, I was unable to measure a memory increase of above 0.2%. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@156297 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index e7960e9fe4..ffe4939155 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -2023,7 +2023,7 @@ def note_ovl_candidate_underqualified : Note< "candidate template ignored: can't deduce a type for %0 which would " "make %2 equal %1">; def note_ovl_candidate_substitution_failure : Note< - "candidate template ignored: substitution failure %0">; + "candidate template ignored: substitution failure%0%1">; // Note that we don't treat templates differently for this diagnostic. def note_ovl_candidate_arity : Note<"candidate " diff --git a/include/clang/Basic/PartialDiagnostic.h b/include/clang/Basic/PartialDiagnostic.h index 007e6a4760..7dce172786 100644 --- a/include/clang/Basic/PartialDiagnostic.h +++ b/include/clang/Basic/PartialDiagnostic.h @@ -179,6 +179,12 @@ private: } public: + struct NullDiagnostic {}; + /// \brief Create a null partial diagnostic, which cannot carry a payload, + /// and only exists to be swapped with a real partial diagnostic. + PartialDiagnostic(NullDiagnostic) + : DiagID(0), DiagStorage(0), Allocator(0) { } + PartialDiagnostic(unsigned DiagID, StorageAllocator &Allocator) : DiagID(DiagID), DiagStorage(0), Allocator(&Allocator) { } @@ -237,6 +243,12 @@ public: freeStorage(); } + void swap(PartialDiagnostic &PD) { + std::swap(DiagID, PD.DiagID); + std::swap(DiagStorage, PD.DiagStorage); + std::swap(Allocator, PD.Allocator); + } + unsigned getDiagID() const { return DiagID; } void AddTaggedVal(intptr_t V, DiagnosticsEngine::ArgumentKind Kind) const { @@ -283,6 +295,18 @@ public: DB.AddFixItHint(DiagStorage->FixItHints[i]); } + void EmitToString(DiagnosticsEngine &Diags, + llvm::SmallVectorImpl &Buf) const { + // FIXME: It should be possible to render a diagnostic to a string without + // messing with the state of the diagnostics engine. + DiagnosticBuilder DB(Diags.Report(getDiagID())); + Emit(DB); + DB.FlushCounts(); + Diagnostic(&Diags).FormatDiagnostic(Buf); + DB.Clear(); + Diags.Clear(); + } + /// \brief Clear out this partial diagnostic, giving it a new diagnostic ID /// and removing all of its arguments, ranges, and fix-it hints. void Reset(unsigned DiagID = 0) { diff --git a/include/clang/Sema/Overload.h b/include/clang/Sema/Overload.h index d334447edc..4e39678fa6 100644 --- a/include/clang/Sema/Overload.h +++ b/include/clang/Sema/Overload.h @@ -659,12 +659,25 @@ namespace clang { /// A structure used to record information about a failed /// template argument deduction. struct DeductionFailureInfo { - // A Sema::TemplateDeductionResult. - unsigned Result; + /// A Sema::TemplateDeductionResult. + unsigned Result : 8; + + /// \brief Indicates whether a diagnostic is stored in Diagnostic. + unsigned HasDiagnostic : 1; /// \brief Opaque pointer containing additional data about /// this deduction failure. void *Data; + + /// \brief A diagnostic indicating why deduction failed. + union { + void *Align; + char Diagnostic[sizeof(PartialDiagnosticAt)]; + }; + + /// \brief Retrieve the diagnostic which caused this deduction failure, + /// if any. + PartialDiagnosticAt *getSFINAEDiagnostic(); /// \brief Retrieve the template parameter this deduction failure /// refers to, if any. @@ -741,9 +754,12 @@ namespace clang { public: OverloadCandidateSet(SourceLocation Loc) : Loc(Loc), NumInlineSequences(0){} ~OverloadCandidateSet() { - for (iterator i = begin(), e = end(); i != e; ++i) + for (iterator i = begin(), e = end(); i != e; ++i) { for (unsigned ii = 0, ie = i->NumConversions; ii != ie; ++ii) i->Conversions[ii].~ImplicitConversionSequence(); + if (i->FailureKind == ovl_fail_bad_deduction) + i->DeductionFailure.Destroy(); + } } SourceLocation getLocation() const { return Loc; } diff --git a/include/clang/Sema/TemplateDeduction.h b/include/clang/Sema/TemplateDeduction.h index 100d56ed33..4c2d876d1f 100644 --- a/include/clang/Sema/TemplateDeduction.h +++ b/include/clang/Sema/TemplateDeduction.h @@ -39,6 +39,9 @@ class TemplateDeductionInfo { /// deduction is occurring. SourceLocation Loc; + /// \brief Have we suppressed an error during deduction? + bool HasSFINAEDiagnostic; + /// \brief Warnings (and follow-on notes) that were suppressed due to /// SFINAE while performing template argument deduction. SmallVector SuppressedDiagnostics; @@ -49,7 +52,7 @@ class TemplateDeductionInfo { public: TemplateDeductionInfo(ASTContext &Context, SourceLocation Loc) - : Context(Context), Deduced(0), Loc(Loc) { } + : Context(Context), Deduced(0), Loc(Loc), HasSFINAEDiagnostic(false) { } ~TemplateDeductionInfo() { // FIXME: if (Deduced) Deduced->Destroy(Context); @@ -68,6 +71,15 @@ public: return Result; } + /// \brief Take ownership of the SFINAE diagnostic. + void takeSFINAEDiagnostic(PartialDiagnosticAt &PD) { + assert(HasSFINAEDiagnostic); + PD.first = SuppressedDiagnostics.front().first; + PD.second.swap(SuppressedDiagnostics.front().second); + SuppressedDiagnostics.clear(); + HasSFINAEDiagnostic = false; + } + /// \brief Provide a new template argument list that contains the /// results of template argument deduction. void reset(TemplateArgumentList *NewDeduced) { @@ -75,10 +87,31 @@ public: Deduced = NewDeduced; } + /// \brief Is a SFINAE diagnostic available? + bool hasSFINAEDiagnostic() const { + return HasSFINAEDiagnostic; + } + + /// \brief Set the diagnostic which caused the SFINAE failure. + void addSFINAEDiagnostic(SourceLocation Loc, PartialDiagnostic PD) { + // Only collect the first diagnostic. + if (HasSFINAEDiagnostic) + return; + SuppressedDiagnostics.clear(); + SuppressedDiagnostics.push_back( + std::make_pair(Loc, PartialDiagnostic::NullDiagnostic())); + SuppressedDiagnostics.back().second.swap(PD); + HasSFINAEDiagnostic = true; + } + /// \brief Add a new diagnostic to the set of diagnostics void addSuppressedDiagnostic(SourceLocation Loc, - const PartialDiagnostic &PD) { - SuppressedDiagnostics.push_back(std::make_pair(Loc, PD)); + PartialDiagnostic PD) { + if (HasSFINAEDiagnostic) + return; + SuppressedDiagnostics.push_back( + std::make_pair(Loc, PartialDiagnostic::NullDiagnostic())); + SuppressedDiagnostics.back().second.swap(PD); } /// \brief Iterator over the set of suppressed diagnostics. diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp index 5017c2cdf5..9f2880fb4e 100644 --- a/lib/Sema/Sema.cpp +++ b/lib/Sema/Sema.cpp @@ -698,6 +698,15 @@ void Sema::EmitCurrentDiagnostic(unsigned DiagID) { // Count this failure so that we know that template argument deduction // has failed. ++NumSFINAEErrors; + + // Make a copy of this suppressed diagnostic and store it with the + // template-deduction information. + if (*Info && !(*Info)->hasSFINAEDiagnostic()) { + Diagnostic DiagInfo(&Diags); + (*Info)->addSFINAEDiagnostic(DiagInfo.getLocation(), + PartialDiagnostic(DiagInfo, Context.getDiagAllocator())); + } + Diags.setLastDiagnosticIgnored(); Diags.Clear(); return; @@ -714,6 +723,15 @@ void Sema::EmitCurrentDiagnostic(unsigned DiagID) { // Suppress this diagnostic. ++NumSFINAEErrors; + + // Make a copy of this suppressed diagnostic and store it with the + // template-deduction information. + if (*Info && !(*Info)->hasSFINAEDiagnostic()) { + Diagnostic DiagInfo(&Diags); + (*Info)->addSFINAEDiagnostic(DiagInfo.getLocation(), + PartialDiagnostic(DiagInfo, Context.getDiagAllocator())); + } + Diags.setLastDiagnosticIgnored(); Diags.Clear(); @@ -730,13 +748,13 @@ void Sema::EmitCurrentDiagnostic(unsigned DiagID) { case DiagnosticIDs::SFINAE_Suppress: // Make a copy of this suppressed diagnostic and store it with the // template-deduction information; - Diagnostic DiagInfo(&Diags); - - if (*Info) + if (*Info) { + Diagnostic DiagInfo(&Diags); (*Info)->addSuppressedDiagnostic(DiagInfo.getLocation(), - PartialDiagnostic(DiagInfo,Context.getDiagAllocator())); - - // Suppress this diagnostic. + PartialDiagnostic(DiagInfo, Context.getDiagAllocator())); + } + + // Suppress this diagnostic. Diags.setLastDiagnosticIgnored(); Diags.Clear(); return; diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp index 007ea0fdcd..f065d38d92 100644 --- a/lib/Sema/SemaOverload.cpp +++ b/lib/Sema/SemaOverload.cpp @@ -28,6 +28,7 @@ #include "clang/Basic/PartialDiagnostic.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/SmallPtrSet.h" +#include "llvm/ADT/SmallString.h" #include "llvm/ADT/STLExtras.h" #include @@ -541,6 +542,7 @@ static MakeDeductionFailureInfo(ASTContext &Context, TemplateDeductionInfo &Info) { OverloadCandidate::DeductionFailureInfo Result; Result.Result = static_cast(TDK); + Result.HasDiagnostic = false; Result.Data = 0; switch (TDK) { case Sema::TDK_Success: @@ -567,6 +569,12 @@ static MakeDeductionFailureInfo(ASTContext &Context, case Sema::TDK_SubstitutionFailure: Result.Data = Info.take(); + if (Info.hasSFINAEDiagnostic()) { + PartialDiagnosticAt *Diag = new (Result.Diagnostic) PartialDiagnosticAt( + SourceLocation(), PartialDiagnostic::NullDiagnostic()); + Info.takeSFINAEDiagnostic(*Diag); + Result.HasDiagnostic = true; + } break; case Sema::TDK_NonDeducedMismatch: @@ -594,8 +602,12 @@ void OverloadCandidate::DeductionFailureInfo::Destroy() { break; case Sema::TDK_SubstitutionFailure: - // FIXME: Destroy the template arugment list? + // FIXME: Destroy the template argument list? Data = 0; + if (PartialDiagnosticAt *Diag = getSFINAEDiagnostic()) { + Diag->~PartialDiagnosticAt(); + HasDiagnostic = false; + } break; // Unhandled @@ -605,6 +617,13 @@ void OverloadCandidate::DeductionFailureInfo::Destroy() { } } +PartialDiagnosticAt * +OverloadCandidate::DeductionFailureInfo::getSFINAEDiagnostic() { + if (HasDiagnostic) + return static_cast(static_cast(Diagnostic)); + return 0; +} + TemplateParameter OverloadCandidate::DeductionFailureInfo::getTemplateParameter() { switch (static_cast(Result)) { @@ -8235,14 +8254,29 @@ void DiagnoseBadDeduction(Sema &S, OverloadCandidate *Cand, return; case Sema::TDK_SubstitutionFailure: { - std::string ArgString; - if (TemplateArgumentList *Args - = Cand->DeductionFailure.getTemplateArgumentList()) - ArgString = S.getTemplateArgumentBindingsText( - Fn->getDescribedFunctionTemplate()->getTemplateParameters(), - *Args); + // Format the template argument list into the argument string. + llvm::SmallString<128> TemplateArgString; + if (TemplateArgumentList *Args = + Cand->DeductionFailure.getTemplateArgumentList()) { + TemplateArgString = " "; + TemplateArgString += S.getTemplateArgumentBindingsText( + Fn->getDescribedFunctionTemplate()->getTemplateParameters(), *Args); + } + + // Format the SFINAE diagnostic into the argument string. + // FIXME: Add a general mechanism to include a PartialDiagnostic *'s + // formatted message in another diagnostic. + llvm::SmallString<128> SFINAEArgString; + SourceRange R; + if (PartialDiagnosticAt *PDiag = + Cand->DeductionFailure.getSFINAEDiagnostic()) { + SFINAEArgString = ": "; + R = SourceRange(PDiag->first, PDiag->first); + PDiag->second.EmitToString(S.getDiagnostics(), SFINAEArgString); + } + S.Diag(Fn->getLocation(), diag::note_ovl_candidate_substitution_failure) - << ArgString; + << TemplateArgString << SFINAEArgString << R; MaybeEmitInheritedConstructorNote(S, Fn); return; } diff --git a/test/SemaTemplate/overload-candidates.cpp b/test/SemaTemplate/overload-candidates.cpp index 8762cc8063..23ca1d47f1 100644 --- a/test/SemaTemplate/overload-candidates.cpp +++ b/test/SemaTemplate/overload-candidates.cpp @@ -24,7 +24,9 @@ void test_get(void *ptr) { } template - typename T::type get_type(const T&); // expected-note{{candidate template ignored: substitution failure [with T = int *]}} + typename T::type get_type(const T&); // expected-note{{candidate template ignored: substitution failure [with T = int *]: type 'int *' cannot be used prior to '::'}} +template + void get_type(T *, int[(int)sizeof(T) - 9] = 0); // expected-note{{candidate template ignored: substitution failure [with T = int]: array size is negative}} void test_get_type(int *ptr) { (void)get_type(ptr); // expected-error{{no matching function for call to 'get_type'}}