[Sema] Improve template argument deduction diagnostic for ambiguous base classes

[erik.pilkington]

Summary

Previously, clang would give a terrible diagnostic for the following:

template <int N> struct B {};
struct D : B<0>, B<1> {};
template <int N> int f(B<N>) { return 0; } // error, ambiguous bases of 'D' could be deduced.
int main() { f(D()); }

This patch just introduces a new deduction failure kind: TDK_AmbiguousBaseClasses, and uses it to emit a better diagnostic.

Thanks!

[rsmith]

include/clang/Basic/DiagnosticSemaKinds.td

3227

ambiguous -> multiple, perhaps? (An "ambiguous base class" is something else.)

However, even that's not quite right. The rule is (from [temp.deduct.call]/5):

"If <deducing against all base classes> yields more than one possible deduced A, type deduction fails"

... so deduction failure only happens if the base classes deduce different template arguments, not just if multiple deductions succeed. In particular, if you multiply-inherit from the same base class, then deduction can succeed (even though the subsequent initialization of the parameter will fail due to ambiguity):

template<int> struct A {};
struct B : A<0> {};
struct C : A<1> {};
struct D : B, C {};
template<int N> void f(A<N>);
void f(...);
void g(D d) { f(d); }  // deduction should succeed, should choose f<0>, result is hard error due to ambiguity

So maybe we should word this more like the note_ovl_candidate_deduced_mismatch case, since the problem really is that we deduced two different values for the same template parameter.

include/clang/Sema/Sema.h

6450

TDK_InconsistentBaseClassDeduction maybe?

lib/Sema/SemaOverload.cpp

599–603

Please update the comments in include/clang/Sema/TemplateDeduction.h to say what Info.FirstArg and Info.SecondArg mean for TDK_AmbiguousBaseClasses.

include/clang/Basic/DiagnosticSemaKinds.td

     "|of %ordinal0 parameter does not match adjusted type of argument}1,2%3">;
 def note_ovl_candidate_non_deduced_mismatch : Note<
     "candidate template ignored: could not match %diff{$ against $|types}0,1">;
 // This note is needed because the above note would sometimes print two
 // different types with the same name.  Remove this note when the above note
 // can handle that case properly.
 def note_ovl_candidate_non_deduced_mismatch_qualified : Note<
     "candidate template ignored: could not match %q0 against %q1">;
+def note_ovl_candidate_ambiguous_base_classes : Note<
+    "candidate template ignored: ambiguous base classes of %0 match %1">;
 
 // Note that we don't treat templates differently for this diagnostic.
 def note_ovl_candidate_arity : Note<"candidate "
     "%select{function|function|constructor|function|function|constructor|"
     "constructor (the implicit default constructor)|"
     "constructor (the implicit copy constructor)|"
     "constructor (the implicit move constructor)|"
     "function (the implicit copy assignment operator)|"

include/clang/Sema/Sema.h

     /// template, there were too few call arguments.
     TDK_TooFewArguments,
     /// \brief The explicitly-specified template arguments were not valid
     /// template arguments for the given template.
     TDK_InvalidExplicitArguments,
     /// \brief The arguments included an overloaded function name that could
     /// not be resolved to a suitable function.
     TDK_FailedOverloadResolution,
+    /// \brief Multiple base classes could have been successfully deduced.
+    TDK_AmbiguousBaseClasses,
     /// \brief Deduction failed; that's all we know.
     TDK_MiscellaneousDeductionFailure
   };
 
   TemplateDeductionResult
   DeduceTemplateArguments(ClassTemplatePartialSpecializationDecl *Partial,
                           const TemplateArgumentList &TemplateArgs,
                           sema::TemplateDeductionInfo &Info);

lib/Sema/SemaOverload.cpp

     Saved->FirstArg = Info.FirstArg;
     Saved->SecondArg = Info.SecondArg;
     Saved->TemplateArgs = Info.take();
     Saved->CallArgIndex = Info.CallArgIndex;
     Result.Data = Saved;
     break;
   }
 
+  case Sema::TDK_AmbiguousBaseClasses:
   case Sema::TDK_NonDeducedMismatch: {
     // FIXME: Should allocate from normal heap so that we can free this later.
     DFIArguments *Saved = new (Context) DFIArguments;
     Saved->FirstArg = Info.FirstArg;
     Saved->SecondArg = Info.SecondArg;
     Result.Data = Saved;
     break;
   }
 
   case Sema::TDK_Inconsistent:
   case Sema::TDK_Underqualified: {
     // FIXME: Should allocate from normal heap so that we can free this later.
     DFIParamWithArguments *Saved = new (Context) DFIParamWithArguments;
     Saved->Param = Info.Param;

   case Sema::TDK_InvalidExplicitArguments:
   case Sema::TDK_FailedOverloadResolution:
     break;
 
   case Sema::TDK_Inconsistent:
   case Sema::TDK_Underqualified:
   case Sema::TDK_DeducedMismatch:
   case Sema::TDK_NonDeducedMismatch:
+  case Sema::TDK_AmbiguousBaseClasses:
     // FIXME: Destroy the data?
     Data = nullptr;
     break;
 
   case Sema::TDK_SubstitutionFailure:
     // FIXME: Destroy the template argument list?
     Data = nullptr;
     if (PartialDiagnosticAt *Diag = getSFINAEDiagnostic()) {

   case Sema::TDK_Invalid:
   case Sema::TDK_InstantiationDepth:
   case Sema::TDK_TooManyArguments:
   case Sema::TDK_TooFewArguments:
   case Sema::TDK_SubstitutionFailure:
   case Sema::TDK_DeducedMismatch:
   case Sema::TDK_NonDeducedMismatch:
   case Sema::TDK_FailedOverloadResolution:
+  case Sema::TDK_AmbiguousBaseClasses:
     return TemplateParameter();
 
   case Sema::TDK_Incomplete:
   case Sema::TDK_InvalidExplicitArguments:
     return TemplateParameter::getFromOpaqueValue(Data);
 
   case Sema::TDK_Inconsistent:
   case Sema::TDK_Underqualified:

   case Sema::TDK_TooManyArguments:
   case Sema::TDK_TooFewArguments:
   case Sema::TDK_Incomplete:
   case Sema::TDK_InvalidExplicitArguments:
   case Sema::TDK_Inconsistent:
   case Sema::TDK_Underqualified:
   case Sema::TDK_NonDeducedMismatch:
   case Sema::TDK_FailedOverloadResolution:
+  case Sema::TDK_AmbiguousBaseClasses:
     return nullptr;
 
   case Sema::TDK_DeducedMismatch:
     return static_cast<DFIDeducedMismatchArgs*>(Data)->TemplateArgs;
 
   case Sema::TDK_SubstitutionFailure:
     return static_cast<TemplateArgumentList*>(Data);
 

   case Sema::TDK_SubstitutionFailure:
   case Sema::TDK_FailedOverloadResolution:
     return nullptr;
 
   case Sema::TDK_Inconsistent:
   case Sema::TDK_Underqualified:
   case Sema::TDK_DeducedMismatch:
   case Sema::TDK_NonDeducedMismatch:
+  case Sema::TDK_AmbiguousBaseClasses:
     return &static_cast<DFIArguments*>(Data)->FirstArg;
 
   // Unhandled
   case Sema::TDK_MiscellaneousDeductionFailure:
     break;
   }
 
   return nullptr;

   case Sema::TDK_SubstitutionFailure:
   case Sema::TDK_FailedOverloadResolution:
     return nullptr;
 
   case Sema::TDK_Inconsistent:
   case Sema::TDK_Underqualified:
   case Sema::TDK_DeducedMismatch:
   case Sema::TDK_NonDeducedMismatch:
+  case Sema::TDK_AmbiguousBaseClasses:
     return &static_cast<DFIArguments*>(Data)->SecondArg;
 
   // Unhandled
   case Sema::TDK_MiscellaneousDeductionFailure:
     break;
   }
 
   return nullptr;

     // call operator, and if so, emit a prettier and more informative 
     // diagnostic that mentions 'auto' and lambda in addition to 
     // (or instead of?) the canonical template type parameters.
     S.Diag(Templated->getLocation(),
            diag::note_ovl_candidate_non_deduced_mismatch)
         << FirstTA << SecondTA;
     return;
   }
+
+  case Sema::TDK_AmbiguousBaseClasses:
+    S.Diag(Templated->getLocation(),
+           diag::note_ovl_candidate_ambiguous_base_classes)
+        << *DeductionFailure.getSecondArg() << *DeductionFailure.getFirstArg();
+    return;
+
   // TODO: diagnose these individually, then kill off
   // note_ovl_candidate_bad_deduction, which is uselessly vague.
   case Sema::TDK_MiscellaneousDeductionFailure:
     S.Diag(Templated->getLocation(), diag::note_ovl_candidate_bad_deduction);
     MaybeEmitInheritedConstructorNote(S, Found);
     return;
   }
 }

 
   case Sema::TDK_Underqualified:
   case Sema::TDK_Inconsistent:
     return 2;
 
   case Sema::TDK_SubstitutionFailure:
   case Sema::TDK_DeducedMismatch:
   case Sema::TDK_NonDeducedMismatch:
+  case Sema::TDK_AmbiguousBaseClasses:
   case Sema::TDK_MiscellaneousDeductionFailure:
     return 3;
 
   case Sema::TDK_InstantiationDepth:
   case Sema::TDK_FailedOverloadResolution:
     return 4;
 
   case Sema::TDK_InvalidExplicitArguments:

lib/Sema/SemaTemplateDeduction.cpp

                                       QualType(NextT, 0), BaseInfo, Deduced);
 
           // If template argument deduction for this base was successful,
           // note that we had some success. Otherwise, ignore any deductions
           // from this base class.
           if (BaseResult == Sema::TDK_Success) {
             // If we've already seen some success, then deduction fails due to
             // an ambiguity (temp.deduct.call p5).
-            if (Successful)
-              return Sema::TDK_MiscellaneousDeductionFailure;
+            if (Successful) {
+              Info.FirstArg = TemplateArgument(ParamIn);
+              Info.SecondArg = TemplateArgument(ArgIn);
+              return Sema::TDK_AmbiguousBaseClasses;
+            }
 
             Successful = true;
             std::swap(SuccessfulDeduced, Deduced);
 
             Info.Param = BaseInfo.Param;
             Info.FirstArg = BaseInfo.FirstArg;
             Info.SecondArg = BaseInfo.SecondArg;
           }

test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p3.cpp

 
 }
 
 namespace PR28195 {
 
 template<int N> struct B {};
 struct D : B<0>, B<1> {};
 
-template<int N> int callee(B<N>); // expected-note{{failed template argument deduction}}
+template<int N> int callee(B<N>); // expected-note{{candidate template ignored: ambiguous base classes of 'PR28195::D' match 'B<N>'}}
 
 int caller() {
   callee(D()); // expected-error{{no matching function}}
 }
 
 }