[Sema] Improve notes for value category mismatch in overloading

When an overloaded member function has a ref-qualifier, like:

class X {
    void f() &&;
    void f(int) &;
};

we would print strange notes when the ref-qualifier doesn't fit the value
category:

X x;
x.f();
X().f(0);

would both print a note "no known conversion from 'X' to 'X' for object
argument" on their relevant overload instead of pointing out the
mismatch in value category.

At first I thought the solution is easy: just use the FailureKind member
of the BadConversionSequence struct. But it turns out that we weren't
properly setting this for function arguments. So I went through
TryReferenceInit to make sure we're doing that right, and found a number
of notes in the existing tests that improved as well.

Fixes PR47791.

Reviewed By: rsmith

Differential Revision: https://reviews.llvm.org/D90123

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -4287,9 +4287,9 @@ def note_ovl_candidate_bad_arc_conv : Note<
     "cannot implicitly convert argument "
     "%diff{of type $ to $|type to parameter type}3,4 for "
     "%select{%ordinal6 argument|object argument}5 under ARC">;
-def note_ovl_candidate_bad_lvalue : Note<
+def note_ovl_candidate_bad_value_category : Note<
     "candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
-    "expects an l-value for "
+    "expects an %select{l-value|r-value}5 for "
     "%select{%ordinal4 argument|object argument}3">;
 def note_ovl_candidate_bad_addrspace : Note<
     "candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "

clang/lib/Sema/SemaOverload.cpp

@@ -4832,8 +4832,11 @@ TryReferenceInit(Sema &S, Expr *Init, QualType DeclType,
   //     -- Otherwise, the reference shall be an lvalue reference to a
   //        non-volatile const type (i.e., cv1 shall be const), or the reference
   //        shall be an rvalue reference.
-  if (!isRValRef && (!T1.isConstQualified() || T1.isVolatileQualified()))
+  if (!isRValRef && (!T1.isConstQualified() || T1.isVolatileQualified())) {
+    if (InitCategory.isRValue() && RefRelationship != Sema::Ref_Incompatible)
+      ICS.setBad(BadConversionSequence::lvalue_ref_to_rvalue, Init, DeclType);
     return ICS;
+  }
 
   //       -- If the initializer expression
   //
@@ -4923,9 +4926,11 @@ TryReferenceInit(Sema &S, Expr *Init, QualType DeclType,
 
   // If T1 is reference-related to T2 and the reference is an rvalue
   // reference, the initializer expression shall not be an lvalue.
-  if (RefRelationship >= Sema::Ref_Related &&
+  if (RefRelationship >= Sema::Ref_Related && isRValRef &&
-      isRValRef && Init->Classify(S.Context).isLValue())
+      Init->Classify(S.Context).isLValue()) {
+    ICS.setBad(BadConversionSequence::rvalue_ref_to_lvalue, Init, DeclType);
     return ICS;
+  }
 
   // C++ [over.ics.ref]p2:
   //   When a parameter of reference type is not bound directly to
@@ -4963,11 +4968,8 @@ TryReferenceInit(Sema &S, Expr *Init, QualType DeclType,
     //   binding an rvalue reference to an lvalue other than a function
     //   lvalue.
     // Note that the function case is not possible here.
-    if (DeclType->isRValueReferenceType() && LValRefType) {
+    if (isRValRef && LValRefType) {
-      // FIXME: This is the wrong BadConversionSequence. The problem is binding
+      ICS.setBad(BadConversionSequence::no_conversion, Init, DeclType);
-      // an rvalue reference to a (non-function) lvalue, not binding an lvalue
-      // reference to an rvalue!
-      ICS.setBad(BadConversionSequence::lvalue_ref_to_rvalue, Init, DeclType);
       return ICS;
     }
 
@@ -10458,7 +10460,7 @@ static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
     }
 
     unsigned CVR = FromQs.getCVRQualifiers() & ~ToQs.getCVRQualifiers();
-    assert(CVR && "unexpected qualifiers mismatch");
+    assert(CVR && "expected qualifiers mismatch");
 
     if (isObjectArgument) {
       S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_cvr_this)
@@ -10475,6 +10477,17 @@ static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
     return;
   }
 
+  if (Conv.Bad.Kind == BadConversionSequence::lvalue_ref_to_rvalue ||
+      Conv.Bad.Kind == BadConversionSequence::rvalue_ref_to_lvalue) {
+    S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_value_category)
+        << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
+        << (unsigned)isObjectArgument << I + 1
+        << (Conv.Bad.Kind == BadConversionSequence::rvalue_ref_to_lvalue)
+        << (FromExpr ? FromExpr->getSourceRange() : SourceRange());
+    MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
+    return;
+  }
+
   // Special diagnostic for failure to convert an initializer list, since
   // telling the user that it has type void is not useful.
   if (FromExpr && isa<InitListExpr>(FromExpr)) {
@@ -10532,15 +10545,6 @@ static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
         !ToRefTy->getPointeeType()->isIncompleteType() &&
         S.IsDerivedFrom(SourceLocation(), ToRefTy->getPointeeType(), FromTy)) {
       BaseToDerivedConversion = 3;
-    } else if (ToTy->isLValueReferenceType() && !FromExpr->isLValue() &&
-               ToTy.getNonReferenceType().getCanonicalType() ==
-               FromTy.getNonReferenceType().getCanonicalType()) {
-      S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_lvalue)
-          << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
-          << (unsigned)isObjectArgument << I + 1
-          << (FromExpr ? FromExpr->getSourceRange() : SourceRange());
-      MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
-      return;
     }
   }
 

clang/test/CXX/drs/dr14xx.cpp

@@ -411,17 +411,17 @@ namespace dr1467 {  // dr1467: 3.7 c++11
   // When the array size is 4 the call will attempt to bind an lvalue to an
   // rvalue and fail. Therefore #2 will be called. (rsmith will bring this
   // issue to CWG)
-  void f(const char(&&)[4]);              // expected-note 5 {{no known conversion}}
+  void f(const char(&&)[4]);              // expected-note 2 {{expects an r-value}} expected-note 3 {{no known conversion}}
   void f(const char(&&)[5]) = delete;     // expected-note 2 {{candidate function has been explicitly deleted}} expected-note 3 {{no known conversion}}
-  void f(const wchar_t(&&)[4]);           // expected-note 5 {{no known conversion}}
+  void f(const wchar_t(&&)[4]);           // expected-note {{expects an r-value}} expected-note 4 {{no known conversion}}
   void f(const wchar_t(&&)[5]) = delete;  // expected-note {{candidate function has been explicitly deleted}} expected-note 4 {{no known conversion}}
 #if __cplusplus >= 202002L
-  void f2(const char8_t(&&)[4]);          // expected-note {{no known conversion}}
+  void f2(const char8_t(&&)[4]);          // expected-note {{expects an r-value}}
   void f2(const char8_t(&&)[5]) = delete; // expected-note {{candidate function has been explicitly deleted}}
 #endif
-  void f(const char16_t(&&)[4]);          // expected-note 5 {{no known conversion}}
+  void f(const char16_t(&&)[4]);          // expected-note {{expects an r-value}} expected-note 4 {{no known conversion}}
   void f(const char16_t(&&)[5]) = delete; // expected-note {{candidate function has been explicitly deleted}} expected-note 4 {{no known conversion}}
-  void f(const char32_t(&&)[4]);          // expected-note 5 {{no known conversion}}
+  void f(const char32_t(&&)[4]);          // expected-note {{expects an r-value}} expected-note 4 {{no known conversion}}
   void f(const char32_t(&&)[5]) = delete; // expected-note {{candidate function has been explicitly deleted}} expected-note 4 {{no known conversion}}
   void g() {
     f({"abc"});       // expected-error {{call to deleted function 'f'}}

clang/test/CXX/drs/dr1xx.cpp

@@ -877,7 +877,7 @@ namespace dr177 { // dr177: yes
   // expected-error@-2 {{no viable constructor copying variable}}
 #endif
 
-  struct C { C(C&); }; // expected-note {{not viable: no known conversion from 'dr177::D' to 'dr177::C &'}}
+  struct C { C(C&); }; // expected-note {{not viable: expects an l-value for 1st argument}}
   struct D : C {};
   struct E { operator D(); };
   E e;

clang/test/CXX/drs/dr6xx.cpp

@@ -69,9 +69,9 @@ namespace dr603 { // dr603: yes
 namespace dr606 { // dr606: yes
 #if __cplusplus >= 201103L
   template<typename T> struct S {};
-  template<typename T> void f(S<T> &&); // expected-note {{no known conversion from 'S<int>' to 'S<int> &&'}}
+  template<typename T> void f(S<T> &&); // expected-note {{expects an r-value}}
   template<typename T> void g(T &&);
-  template<typename T> void h(const T &&); // expected-note {{no known conversion from 'S<int>' to 'const dr606::S<int> &&'}}
+  template<typename T> void h(const T &&); // expected-note {{expects an r-value}}
 
   void test(S<int> s) {
     f(s); // expected-error {{no match}}

clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p3-0x.cpp

@@ -7,7 +7,7 @@
 namespace ClassTemplateParamNotForwardingRef {
   // This is not a forwarding reference.
   template<typename T> struct A { // expected-note {{candidate}}
-    A(T&&); // expected-note {{no known conversion from 'int' to 'int &&'}}
+    A(T&&); // expected-note {{expects an r-value}}
   };
   int n;
   A a = n; // expected-error {{no viable constructor or deduction guide}}
@@ -53,8 +53,8 @@ void test_f0() {
   X<Y&> xy2 = f0(lvalue<Y>());
 }
 
-template<typename T> X<T> f1(const T&&); // expected-note{{candidate function [with T = int] not viable: no known conversion from 'int' to 'const int &&' for 1st argument}} \
+template<typename T> X<T> f1(const T&&); // expected-note{{candidate function [with T = int] not viable: expects an r-value for 1st argument}} \
-// expected-note{{candidate function [with T = Y] not viable: no known conversion from 'Y' to 'const Y &&' for 1st argument}}
+// expected-note{{candidate function [with T = Y] not viable: expects an r-value for 1st argument}}
 
 void test_f1() {
   X<int> xi0 = f1(prvalue<int>());
@@ -67,7 +67,7 @@ void test_f1() {
 
 namespace std_example {
   template <class T> int f(T&&); 
-  template <class T> int g(const T&&); // expected-note{{candidate function [with T = int] not viable: no known conversion from 'int' to 'const int &&' for 1st argument}}
+  template <class T> int g(const T&&); // expected-note{{candidate function [with T = int] not viable: expects an r-value for 1st argument}}
 
   int i;
   int n1 = f(i);
@@ -77,7 +77,7 @@ namespace std_example {
 #if __cplusplus > 201402L
   template<class T> struct A { // expected-note {{candidate}}
     template<class U>
-    A(T &&, U &&, int *); // expected-note {{[with T = int, U = int] not viable: no known conversion from 'int' to 'int &&'}}
+    A(T &&, U &&, int *); // expected-note {{[with T = int, U = int] not viable: expects an r-value}}
     A(T &&, int *);       // expected-note {{requires 2}}
   };
   template<class T> A(T &&, int *) -> A<T>; // expected-note {{requires 2}}

clang/test/SemaCXX/overload-member-call.cpp

@@ -83,6 +83,9 @@ namespace test1 {
     void baz(A &d); // expected-note {{candidate function not viable: 1st argument ('const test1::A') would lose const qualifier}}
     void baz(int i); // expected-note {{candidate function not viable: no known conversion from 'const test1::A' to 'int' for 1st argument}} 
 
+    void ref() &&;   // expected-note {{expects an r-value for object argument}} expected-note {{requires 0 arguments, but 1 was provided}}
+    void ref(int) &; // expected-note {{expects an l-value for object argument}} expected-note {{requires 1 argument, but 0 were provided}}
+
     // PR 11857
     void foo(int n); // expected-note {{candidate function not viable: requires single argument 'n', but 2 arguments were provided}}
     void foo(unsigned n = 10); // expected-note {{candidate function not viable: allows at most single argument 'n', but 2 arguments were provided}}
@@ -103,6 +106,9 @@ namespace test1 {
 
     a.rab(); //expected-error {{no matching member function for call to 'rab'}}
     a.zab(3, 4, 5); //expected-error {{no matching member function for call to 'zab'}}
+
+    a.ref();    // expected-error {{no matching member function for call to 'ref'}}
+    A().ref(1); // expected-error {{no matching member function for call to 'ref'}}
   }
 }
 

clang/test/SemaCXX/rval-references-examples.cpp

@@ -13,7 +13,7 @@ public:
 
   ~unique_ptr() { delete ptr; }
 
-  unique_ptr &operator=(unique_ptr &&other) { // expected-note{{candidate function not viable: no known conversion from 'unique_ptr<int>' to 'unique_ptr<int> &&' for 1st argument}}
+  unique_ptr &operator=(unique_ptr &&other) { // expected-note{{candidate function not viable: expects an r-value for 1st argument}}
     if (this == &other)
       return *this;