Improve the reporting of non-viable overload candidates by noting the reason

why the candidate is non-viable.  There's a lot we can do to improve this, but
it's a good start.  Further improvements should probably be integrated with the
bad-initialization reporting routines.

llvm-svn: 93277

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -899,19 +899,29 @@ def err_ovl_deleted_member_call : Error<
   "call to %select{unavailable|deleted}0 member function %1">;
 def note_ovl_candidate : Note<"candidate "
     "%select{function|function|constructor|"
+    "function |function |constructor |"
     "is the implicit default constructor|"
     "is the implicit copy constructor|"
-    "is the implicit copy assignment operator}0">;
-def note_ovl_template_candidate : Note<
-  "candidate function template specialization %0">;
+    "is the implicit copy assignment operator}0%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)|"
+    "function (the implicit copy assignment operator)}0 not viable: requires"
+    "%select{ at least| at most|}2 %3 argument%s3, but %4 %plural{1:was|:were}4 "
+    "provided">;
 def note_ovl_candidate_deleted : Note<
-  "candidate %select{function|function|constructor}0 has been explicitly "
-  "%select{made unavailable|deleted}1">;
-def note_ovl_template_candidate_deleted : Note<
-  "candidate function template specialization %0 has been explicit "
-  "&select{made unavailable|deleted}1">;
-def note_ovl_candidate_not_viable : Note<"function not viable because"
-                          " of ambiguity in conversion of argument %0">;
+  "candidate %select{function|function|constructor|"
+  "function |function |constructor |||}0%1 "
+  "has been explicitly %select{made unavailable|deleted}2">;
+def note_ovl_candidate_bad_conv : Note<"candidate "
+    "%select{function|function|constructor|"
+    "function |function |constructor |"
+    "constructor (the implicit default constructor)|"
+    "constructor (the implicit copy constructor)|"
+    "function (the implicit copy assignment operator)}0%1"
+    " not viable: no known conversion from %2 to %3 for argument %4">;
 def note_ambiguous_type_conversion: Note<
     "because of ambiguity in conversion of %0 to %1">;
 def note_ovl_builtin_binary_candidate : Note<

clang/lib/Sema/SemaOverload.cpp

@@ -4269,19 +4269,28 @@ enum OverloadCandidateKind {
   oc_function,
   oc_method,
   oc_constructor,
+  oc_function_template,
+  oc_method_template,
+  oc_constructor_template,
   oc_implicit_default_constructor,
   oc_implicit_copy_constructor,
-  oc_implicit_copy_assignment,
-  oc_template_specialization // function, constructor, or conversion template
+  oc_implicit_copy_assignment
 };
 
-OverloadCandidateKind ClassifyOverloadCandidate(FunctionDecl *Fn) {
-  if (Fn->getPrimaryTemplate())
-    return oc_template_specialization;
+OverloadCandidateKind ClassifyOverloadCandidate(Sema &S,
+                                                FunctionDecl *Fn,
+                                                std::string &Description) {
+  bool isTemplate = false;
+
+  if (FunctionTemplateDecl *FunTmpl = Fn->getPrimaryTemplate()) {
+    isTemplate = true;
+    Description = S.getTemplateArgumentBindingsText(
+      FunTmpl->getTemplateParameters(), *Fn->getTemplateSpecializationArgs());
+  }
 
   if (CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(Fn)) {
     if (!Ctor->isImplicit())
-      return oc_constructor;
+      return isTemplate ? oc_constructor_template : oc_constructor;
 
     return Ctor->isCopyConstructor() ? oc_implicit_copy_constructor
                                      : oc_implicit_default_constructor;
@@ -4291,34 +4300,24 @@ OverloadCandidateKind ClassifyOverloadCandidate(FunctionDecl *Fn) {
     // This actually gets spelled 'candidate function' for now, but
     // it doesn't hurt to split it out.
     if (!Meth->isImplicit())
-      return oc_method;
+      return isTemplate ? oc_method_template : oc_method;
 
     assert(Meth->isCopyAssignment()
            && "implicit method is not copy assignment operator?");
     return oc_implicit_copy_assignment;
   }
 
-  return oc_function;
-}
-
-std::string DescribeFunctionTemplate(Sema &S, FunctionDecl *Fn) {
-  FunctionTemplateDecl *FunTmpl = Fn->getPrimaryTemplate();
-  return S.getTemplateArgumentBindingsText(FunTmpl->getTemplateParameters(),
-                                       *Fn->getTemplateSpecializationArgs());
+  return isTemplate ? oc_function_template : oc_function;
 }
 
 } // end anonymous namespace
 
 // Notes the location of an overload candidate.
 void Sema::NoteOverloadCandidate(FunctionDecl *Fn) {
-  OverloadCandidateKind K = ClassifyOverloadCandidate(Fn);
-  if (K == oc_template_specialization) {
-    Diag(Fn->getLocation(), diag::note_ovl_template_candidate)
-      << DescribeFunctionTemplate(*this, Fn);
-    return;
-  }
-
-  Diag(Fn->getLocation(), diag::note_ovl_candidate) << (unsigned) K;
+  std::string FnDesc;
+  OverloadCandidateKind K = ClassifyOverloadCandidate(*this, Fn, FnDesc);
+  Diag(Fn->getLocation(), diag::note_ovl_candidate)
+    << (unsigned) K << FnDesc;
 }
 
 /// Diagnoses an ambiguous conversion.  The partial diagnostic is the
@@ -4337,41 +4336,104 @@ void Sema::DiagnoseAmbiguousConversion(const ImplicitConversionSequence &ICS,
 
 namespace {
 
-void NoteFunctionCandidate(Sema &S, OverloadCandidate *Cand) {
+void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand, unsigned I,
+                           Expr **Args, unsigned NumArgs) {
+  assert(Cand->Function && "for now, candidate must be a function");
+  FunctionDecl *Fn = Cand->Function;
+
+  // There's a conversion slot for the object argument if this is a
+  // non-constructor method.  Note that 'I' corresponds the
+  // conversion-slot index.
+  if (isa<CXXMethodDecl>(Fn) && !isa<CXXConstructorDecl>(Fn)) {
+    // FIXME: talk usefully about bad conversions for object arguments.
+    if (I == 0) return S.NoteOverloadCandidate(Fn);
+    else I--;
+  }
+
+  // FIXME: can we have a bad conversion on an ellipsis parameter?
+  assert(I < NumArgs && "index exceeds number of formal arguments");
+  assert(I < Fn->getType()->getAs<FunctionProtoType>()->getNumArgs() &&
+         "index exceeds number of formal parameters");
+
+  std::string FnDesc;
+  OverloadCandidateKind FnKind = ClassifyOverloadCandidate(S, Fn, FnDesc);
+
+  QualType FromTy = Args[I]->getType();
+  QualType ToTy = Fn->getType()->getAs<FunctionProtoType>()->getArgType(I);
+
+  // TODO: specialize based on the kind of mismatch
+  S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_conv)
+    << (unsigned) FnKind << FnDesc
+    << Args[I]->getSourceRange() << FromTy << ToTy
+    << I+1;
+}
+
+void NoteFunctionCandidate(Sema &S, OverloadCandidate *Cand,
+                           Expr **Args, unsigned NumArgs) {
   FunctionDecl *Fn = Cand->Function;
 
   // Note deleted candidates, but only if they're viable.
   if (Cand->Viable && (Fn->isDeleted() || Fn->hasAttr<UnavailableAttr>())) {
-    OverloadCandidateKind FnKind = ClassifyOverloadCandidate(Fn);
-
-    if (FnKind == oc_template_specialization) {
-      S.Diag(Fn->getLocation(), diag::note_ovl_template_candidate_deleted)
-        << DescribeFunctionTemplate(S, Fn) << Fn->isDeleted();
-      return;
-    }
+    std::string FnDesc;
+    OverloadCandidateKind FnKind = ClassifyOverloadCandidate(S, Fn, FnDesc);
 
     S.Diag(Fn->getLocation(), diag::note_ovl_candidate_deleted)
-      << FnKind << Fn->isDeleted();
+      << FnKind << FnDesc << Fn->isDeleted();
     return;
   }
 
-  bool errReported = false;
-  if (!Cand->Viable && Cand->Conversions.size() > 0) {
-    for (int i = Cand->Conversions.size()-1; i >= 0; i--) {
-      const ImplicitConversionSequence &Conversion = 
-        Cand->Conversions[i];
+  // We don't really have anything else to say about viable candidates.
+  if (Cand->Viable) {
+    S.NoteOverloadCandidate(Fn);
+    return;
+  }
 
-      if (!Conversion.isAmbiguous())
+  // Diagnose arity mismatches.
+  // TODO: treat calls to a missing default constructor as a special case
+  unsigned NumFormalArgs = NumArgs;
+  if (isa<CXXMethodDecl>(Fn) && !isa<CXXConstructorDecl>(Fn))
+    NumFormalArgs--;
+  const FunctionProtoType *FnTy = Fn->getType()->getAs<FunctionProtoType>();
+  unsigned MinParams = Fn->getMinRequiredArguments();
+  if (NumFormalArgs < MinParams ||
+      (NumFormalArgs > FnTy->getNumArgs() && !FnTy->isVariadic())) {
+    std::string Description;
+    OverloadCandidateKind FnKind = ClassifyOverloadCandidate(S, Fn, Description);
+
+    // at least / at most / exactly
+    unsigned mode, modeCount;
+    if (NumFormalArgs < MinParams) {
+      if (MinParams != FnTy->getNumArgs())
+        mode = 0; // "at least"
+      else
+        mode = 2; // "exactly"
+      modeCount = MinParams;
+    } else {
+      if (MinParams != FnTy->getNumArgs())
+        mode = 1; // "at most"
+      else
+        mode = 2; // "exactly"
+      modeCount = FnTy->getNumArgs();
+    }
+
+    S.Diag(Fn->getLocation(), diag::note_ovl_candidate_arity)
+      << (unsigned) FnKind << Description << mode << modeCount << NumFormalArgs;
+    return;
+  }
+
+  // Look for bad conversions.
+  if (!Cand->Conversions.empty()) {
+    for (unsigned I = 0, N = Cand->Conversions.size(); I != N; ++I) {
+      if (!Cand->Conversions[I].isBad())
         continue;
 
-      S.DiagnoseAmbiguousConversion(Conversion, Fn->getLocation(),
-                         PDiag(diag::note_ovl_candidate_not_viable) << (i+1));
-      errReported = true;
+      DiagnoseBadConversion(S, Cand, I, Args, NumArgs);
+      return;
     }
   }
 
-  if (!errReported)
-    S.NoteOverloadCandidate(Fn);
+  // Give up and give the generic message.
+  S.NoteOverloadCandidate(Fn);
 }
 
 void NoteSurrogateCandidate(Sema &S, OverloadCandidate *Cand) {
@@ -4506,7 +4568,7 @@ Sema::PrintOverloadCandidates(OverloadCandidateSet& CandidateSet,
     OverloadCandidate *Cand = *I;
 
     if (Cand->Function)
-      NoteFunctionCandidate(*this, Cand);
+      NoteFunctionCandidate(*this, Cand, Args, NumArgs);
     else if (Cand->IsSurrogate)
       NoteSurrogateCandidate(*this, Cand);
 
@@ -4714,7 +4776,8 @@ Sema::ResolveAddressOfOverloadedFunction(Expr *From, QualType ToType,
                            PDiag(),
                            PDiag(diag::err_addr_ovl_ambiguous)
                                << TemplateMatches[0]->getDeclName(),
-                           PDiag(diag::note_ovl_template_candidate));
+                           PDiag(diag::note_ovl_candidate)
+                               << (unsigned) oc_function_template);
     MarkDeclarationReferenced(From->getLocStart(), Result);
     return Result;
   }

clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-var.cpp

@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
 struct Base { }; // expected-note{{candidate is the implicit copy constructor}}
-struct Derived : Base { }; // expected-note{{candidate is the implicit copy constructor}}
+struct Derived : Base { }; // expected-note{{candidate constructor (the implicit copy constructor) not viable}}
 struct Unrelated { };
 struct Derived2 : Base { };
 struct Diamond : Derived, Derived2 { };

clang/test/CXX/temp/temp.decls/temp.class/temp.static/p1-inst.cpp

@@ -14,7 +14,7 @@ struct InitOkay {
   InitOkay(int) { }
 };
 
-struct CannotInit { }; // expected-note{{candidate is the implicit copy constructor}}
+struct CannotInit { }; // expected-note{{candidate constructor (the implicit copy constructor) not viable}}
 
 int &returnInt() { return X<int>::value; }
 float &returnFloat() { return X<float>::value; }

clang/test/CXX/temp/temp.decls/temp.class/temp.static/p1.cpp

@@ -12,7 +12,7 @@ struct X1 {
   X1(int);
 };
 
-struct X2 { }; // expected-note{{candidate is the implicit copy constructor}}
+struct X2 { }; // expected-note{{candidate constructor (the implicit copy constructor) not viable}}
 
 int& get_int() { return X0<int>::value; }
 X1& get_X1() { return X0<X1>::value; }

clang/test/CXX/temp/temp.param/p3.cpp

@@ -15,7 +15,7 @@ template<template<class T> class Y> struct X1 {
 // could be interpreted as either a non-type template-parameter or a
 // type-parameter (because its identifier is the name of an already
 // existing class) is taken as a type-parameter. For example, 
-class T { /* ... */ };  // expected-note{{candidate is the implicit copy constructor}}
+class T { /* ... */ };  // expected-note{{candidate constructor (the implicit copy constructor) not viable}}
 int i; 
 
 template<class T, T i> struct X2 {

clang/test/CXX/temp/temp.spec/temp.expl.spec/p4.cpp

@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
-struct IntHolder { // expected-note{{here}} // expected-note 2{{candidate is the implicit copy constructor}}
+struct IntHolder { // expected-note{{here}} // expected-note 2{{candidate constructor (the implicit copy constructor)}}
   IntHolder(int); // expected-note 2{{candidate constructor}}
 };
 

clang/test/CXX/temp/temp.spec/temp.explicit/p1.cpp

@@ -48,7 +48,7 @@ template void X1<int>::f<>(int&, int*); // expected-note{{instantiation}}
 
 // Explicitly instantiate members of a class template
 struct Incomplete; // expected-note{{forward declaration}}
-struct NonDefaultConstructible { // expected-note{{candidate is the implicit copy constructor}}
+struct NonDefaultConstructible { // expected-note{{candidate constructor (the implicit copy constructor) not viable}}
   NonDefaultConstructible(int); // expected-note{{candidate constructor}}
 };
 

clang/test/SemaCXX/condition.cpp

@@ -16,8 +16,8 @@ void test() {
   for (;s;) ; // expected-error {{value of type 'struct S' is not contextually convertible to 'bool'}}
   switch (s) {} // expected-error {{statement requires expression of integer type ('struct S' invalid)}}
 
-  while (struct S {} x=0) ; // expected-error {{types may not be defined in conditions}} expected-error {{no viable conversion}} expected-error {{value of type 'struct S' is not contextually convertible to 'bool'}} expected-note{{candidate is the implicit copy constructor}}
-  while (struct {} x=0) ; // expected-error {{types may not be defined in conditions}} expected-error {{no viable conversion}} expected-error {{value of type 'struct <anonymous>' is not contextually convertible to 'bool'}} expected-note{{candidate is the implicit copy constructor}}
+  while (struct S {} x=0) ; // expected-error {{types may not be defined in conditions}} expected-error {{no viable conversion}} expected-error {{value of type 'struct S' is not contextually convertible to 'bool'}} expected-note{{candidate constructor (the implicit copy constructor)}}
+  while (struct {} x=0) ; // expected-error {{types may not be defined in conditions}} expected-error {{no viable conversion}} expected-error {{value of type 'struct <anonymous>' is not contextually convertible to 'bool'}} expected-note{{candidate constructor (the implicit copy constructor)}}
   switch (enum {E} x=0) ; // expected-error {{types may not be defined in conditions}} expected-error {{cannot initialize}}
 
   if (int x=0) { // expected-note 2 {{previous definition is here}}

clang/test/SemaCXX/constructor-initializer.cpp

@@ -97,7 +97,7 @@ struct Current : Derived {
                                                   // expected-error {{member initializer 'NonExisting' does not name a non-static data member or}}
 };
 
-struct M {              // expected-note 2 {{candidate is the implicit copy constructor}} \
+struct M {              // expected-note 2 {{candidate constructor (the implicit copy constructor)}} \
                         // expected-note {{declared here}} \
                         // expected-note {{declared here}}
   M(int i, int j);      // expected-note 2 {{candidate constructor}}

clang/test/SemaCXX/converting-constructor.cpp

@@ -27,7 +27,7 @@ public:
   FromShort(short s);
 };
 
-class FromShortExplicitly { // expected-note{{candidate is the implicit copy constructor}}
+class FromShortExplicitly { // expected-note{{candidate constructor (the implicit copy constructor)}}
 public:
   explicit FromShortExplicitly(short s);
 };

clang/test/SemaCXX/dcl_init_aggr.cpp

@@ -40,7 +40,7 @@ char cv[4] = { 'a', 's', 'd', 'f', 0 }; // expected-error{{excess elements in ar
 struct TooFew { int a; char* b; int c; }; 
 TooFew too_few = { 1, "asdf" }; // okay
 
-struct NoDefaultConstructor { // expected-note 3 {{candidate is the implicit copy constructor}} \
+struct NoDefaultConstructor { // expected-note 3 {{candidate constructor (the implicit copy constructor)}} \
                               // expected-note{{declared here}}
   NoDefaultConstructor(int); // expected-note 3 {{candidate constructor}}
 };
@@ -115,7 +115,7 @@ B2 b2_2 = { 4, d2, 0 };
 B2 b2_3 = { c2, a2, a2 };
 
 // C++ [dcl.init.aggr]p15:
-union u { int a; char* b; }; // expected-note{{candidate is the implicit copy constructor}}
+union u { int a; char* b; }; // expected-note{{candidate constructor (the implicit copy constructor)}}
 u u1 = { 1 }; 
 u u2 = u1; 
 u u3 = 1; // expected-error{{no viable conversion}}

clang/test/SemaCXX/direct-initializer.cpp

@@ -13,14 +13,14 @@ class Y {
   explicit Y(float);
 };
 
-class X { // expected-note{{candidate is the implicit copy constructor}}
+class X { // expected-note{{candidate constructor (the implicit copy constructor)}}
 public:
   explicit X(int); // expected-note{{candidate constructor}}
   X(float, float, float); // expected-note{{candidate constructor}}
   X(float, Y); // expected-note{{candidate constructor}}
 };
 
-class Z { // expected-note{{candidate is the implicit copy constructor}}
+class Z { // expected-note{{candidate constructor (the implicit copy constructor)}}
 public:
   Z(int); // expected-note{{candidate constructor}}
 };

clang/test/SemaCXX/functional-cast.cpp

@@ -10,7 +10,7 @@ struct InitViaConstructor {
   InitViaConstructor(int i = 7);
 };
 
-struct NoValueInit { // expected-note 2 {{candidate is the implicit copy constructor}} 
+struct NoValueInit { // expected-note 2 {{candidate constructor (the implicit copy constructor)}} 
   NoValueInit(int i, int j); // expected-note 2 {{candidate constructor}}
 };
 

clang/test/SemaCXX/namespace.cpp

@@ -9,7 +9,7 @@ int A; // expected-error {{redefinition of 'A' as different kind of symbol}}
 class A; // expected-error {{redefinition of 'A' as different kind of symbol}}
 
 class B {}; // expected-note {{previous definition is here}} \
-            // expected-note{{candidate is the implicit copy assignment operator}}
+            // expected-note{{candidate function (the implicit copy assignment operator)}}
 
 void C(); // expected-note {{previous definition is here}}
 namespace C {} // expected-error {{redefinition of 'C' as different kind of symbol}}

clang/test/SemaCXX/nested-name-spec.cpp

@@ -178,7 +178,7 @@ bool (foo_S::value);
 
 
 namespace somens {
-  struct a { }; // expected-note{{candidate is the implicit copy constructor}}
+  struct a { }; // expected-note{{candidate constructor (the implicit copy constructor)}}
 }
 
 template <typename T>

clang/test/SemaCXX/overload-call.cpp

@@ -304,7 +304,7 @@ namespace PR5756 {
 
 // Tests the exact text used to note the candidates
 namespace test1 {
-  template <class T> void foo(T t, unsigned N); // expected-note {{candidate function template specialization [with T = int]}}
+  template <class T> void foo(T t, unsigned N); // expected-note {{candidate function [with T = int]}}
   void foo(int n, char N); // expected-note {{candidate function}} 
 
   void test() {

clang/test/SemaCXX/overloaded-builtin-operators.cpp

@@ -59,7 +59,7 @@ void f(Short s, Long l, Enum1 e1, Enum2 e2, Xpmf pmf) {
   // FIXME: should pass (void)static_cast<no&>(islong(e1 % e2));
 }
 
-struct ShortRef { // expected-note{{candidate is the implicit copy assignment operator}}
+struct ShortRef { // expected-note{{candidate function (the implicit copy assignment operator)}}
   operator short&();
 };
 
@@ -67,7 +67,7 @@ struct LongRef {
   operator volatile long&();
 };
 
-struct XpmfRef { // expected-note{{candidate is the implicit copy assignment operator}}
+struct XpmfRef { // expected-note{{candidate function (the implicit copy assignment operator)}}
   operator pmf&();
 };
 

clang/test/SemaTemplate/ambiguous-ovl-print.cpp

@@ -2,7 +2,7 @@
 
 void f(void*, int); // expected-note{{candidate function}}
 template<typename T>
-  void f(T*, long); // expected-note{{candidate function template}}
+  void f(T*, long); // expected-note{{candidate function}}
 
 void test_f(int *ip, int i) {
   f(ip, i); // expected-error{{ambiguous}}

clang/test/SemaTemplate/default-expr-arguments.cpp

@@ -5,7 +5,7 @@ class C { C(int a0 = 0); };
 template<>
 C<char>::C(int a0);
 
-struct S { }; // expected-note 3 {{candidate is the implicit copy constructor}}
+struct S { }; // expected-note 3 {{candidate constructor (the implicit copy constructor)}}
 
 template<typename T> void f1(T a, T b = 10) { } // expected-error{{no viable conversion}}
 

clang/test/SemaTemplate/explicit-instantiation.cpp

@@ -25,7 +25,7 @@ T X0<T>::value; // expected-error{{no matching constructor}}
 
 template int X0<int>::value;
 
-struct NotDefaultConstructible { // expected-note{{candidate is the implicit copy constructor}}
+struct NotDefaultConstructible { // expected-note{{candidate constructor (the implicit copy constructor)}}
   NotDefaultConstructible(int); // expected-note{{candidate constructor}}
 };
 

clang/test/SemaTemplate/fun-template-def.cpp

@@ -8,7 +8,7 @@
 // Fake typeid, lacking a typeinfo header.
 namespace std { class type_info {}; }
 
-struct dummy {}; // expected-note 3 {{candidate is the implicit copy constructor}}
+struct dummy {}; // expected-note 3 {{candidate constructor (the implicit copy constructor)}}
 
 template<typename T>
 int f0(T x) {

clang/test/SemaTemplate/instantiate-expr-4.cpp

@@ -21,7 +21,7 @@ struct FunctionalCast0 {
 
 template struct FunctionalCast0<5>;
 
-struct X { // expected-note 3 {{candidate is the implicit copy constructor}}
+struct X { // expected-note 3 {{candidate constructor (the implicit copy constructor)}}
   X(int, int); // expected-note 3 {{candidate constructor}}
 };