[cfe-commits] r156297 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/PartialDiagnostic.h include/clang/Sema/Overload.h include/clang/Sema/TemplateDeduction.h lib/Sema/Sema.cpp lib/Sema/SemaOverload.cpp test/SemaTemplate/overload-candidates.cpp

Richard Smith richard-llvm at metafoo.co.uk
Mon May 7 02:03:25 PDT 2012


Author: rsmith
Date: Mon May  7 04:03:25 2012
New Revision: 156297

URL: http://llvm.org/viewvc/llvm-project?rev=156297&view=rev
Log:
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%.

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/include/clang/Basic/PartialDiagnostic.h
    cfe/trunk/include/clang/Sema/Overload.h
    cfe/trunk/include/clang/Sema/TemplateDeduction.h
    cfe/trunk/lib/Sema/Sema.cpp
    cfe/trunk/lib/Sema/SemaOverload.cpp
    cfe/trunk/test/SemaTemplate/overload-candidates.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=156297&r1=156296&r2=156297&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon May  7 04:03:25 2012
@@ -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 "

Modified: cfe/trunk/include/clang/Basic/PartialDiagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/PartialDiagnostic.h?rev=156297&r1=156296&r2=156297&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/PartialDiagnostic.h (original)
+++ cfe/trunk/include/clang/Basic/PartialDiagnostic.h Mon May  7 04:03:25 2012
@@ -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) {

Modified: cfe/trunk/include/clang/Sema/Overload.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Overload.h?rev=156297&r1=156296&r2=156297&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Overload.h (original)
+++ cfe/trunk/include/clang/Sema/Overload.h Mon May  7 04:03:25 2012
@@ -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; }

Modified: cfe/trunk/include/clang/Sema/TemplateDeduction.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/TemplateDeduction.h?rev=156297&r1=156296&r2=156297&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/TemplateDeduction.h (original)
+++ cfe/trunk/include/clang/Sema/TemplateDeduction.h Mon May  7 04:03:25 2012
@@ -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.

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=156297&r1=156296&r2=156297&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Mon May  7 04:03:25 2012
@@ -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;

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=156297&r1=156296&r2=156297&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Mon May  7 04:03:25 2012
@@ -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;
   }

Modified: cfe/trunk/test/SemaTemplate/overload-candidates.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/overload-candidates.cpp?rev=156297&r1=156296&r2=156297&view=diff
==============================================================================
--- cfe/trunk/test/SemaTemplate/overload-candidates.cpp (original)
+++ cfe/trunk/test/SemaTemplate/overload-candidates.cpp Mon May  7 04:03:25 2012
@@ -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'}}





More information about the cfe-commits mailing list