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 e7960e9..ffe4939 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2023,7 +2023,7 @@
     "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 007e6a4..7dce172 100644
--- a/include/clang/Basic/PartialDiagnostic.h
+++ b/include/clang/Basic/PartialDiagnostic.h
@@ -179,6 +179,12 @@
   }
 
 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 @@
     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 @@
       DB.AddFixItHint(DiagStorage->FixItHints[i]);
   }
 
+  void EmitToString(DiagnosticsEngine &Diags,
+                    llvm::SmallVectorImpl<char> &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 d334447..4e39678 100644
--- a/include/clang/Sema/Overload.h
+++ b/include/clang/Sema/Overload.h
@@ -659,12 +659,25 @@
     /// 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 @@
   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 100d56e..4c2d876 100644
--- a/include/clang/Sema/TemplateDeduction.h
+++ b/include/clang/Sema/TemplateDeduction.h
@@ -39,6 +39,9 @@
   /// 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<PartialDiagnosticAt, 4> SuppressedDiagnostics;
@@ -49,7 +52,7 @@
 
 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 @@
     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 @@
     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 5017c2c..9f2880f 100644
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -698,6 +698,15 @@
       // 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 @@
 
       // 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 @@
     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 007ea0f..f065d38 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 <algorithm>
 
@@ -541,6 +542,7 @@
                                 TemplateDeductionInfo &Info) {
   OverloadCandidate::DeductionFailureInfo Result;
   Result.Result = static_cast<unsigned>(TDK);
+  Result.HasDiagnostic = false;
   Result.Data = 0;
   switch (TDK) {
   case Sema::TDK_Success:
@@ -567,6 +569,12 @@
 
   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 @@
     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 @@
   }
 }
 
+PartialDiagnosticAt *
+OverloadCandidate::DeductionFailureInfo::getSFINAEDiagnostic() {
+  if (HasDiagnostic)
+    return static_cast<PartialDiagnosticAt*>(static_cast<void*>(Diagnostic));
+  return 0;
+}
+
 TemplateParameter
 OverloadCandidate::DeductionFailureInfo::getTemplateParameter() {
   switch (static_cast<Sema::TemplateDeductionResult>(Result)) {
@@ -8235,14 +8254,29 @@
     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 8762cc8..23ca1d4 100644
--- a/test/SemaTemplate/overload-candidates.cpp
+++ b/test/SemaTemplate/overload-candidates.cpp
@@ -24,7 +24,9 @@
 }
 
 template<typename T>
-  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<typename T>
+  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'}}