[Sema] Improve diagnostics for const- and ref-qualified member functions

Summary:
Adjust wording for const-qualification mismatch to be a little more clear.

Also add another diagnostic for a ref qualifier mismatch, which previously produced a useless error (this error path is simply very old; see rL119336):

Before:
  error: cannot initialize object parameter of type 'X0' with an expression of type 'X0'

After:
  error: 'this' argument to member function 'rvalue' is an lvalue, but function has rvalue ref-qualifier

Reviewers: rsmith, aaron.ballman

Reviewed By: aaron.ballman

Subscribers: lebedev.ri, aaron.ballman, cfe-commits

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

llvm-svn: 321592

4 files changed, 69 insertions, 12 deletions

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 01e819942f6..e4649d3a44d 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1589,12 +1589,20 @@ def err_non_virtual_pure : Error<
 def ext_pure_function_definition : ExtWarn<
   "function definition with pure-specifier is a Microsoft extension">,
   InGroup<MicrosoftPureDefinition>;
-def err_implicit_object_parameter_init : Error<
-  "cannot initialize object parameter of type %0 with an expression "
-  "of type %1">;
 def err_qualified_member_of_unrelated : Error<
   "%q0 is not a member of class %1">;
 
+def err_member_function_call_bad_cvr : Error<
+  "'this' argument to member function %0 has type %1, but function is not marked "
+  "%select{const|restrict|const or restrict|volatile|const or volatile|"
+  "volatile or restrict|const, volatile, or restrict}2">;
+def err_member_function_call_bad_ref : Error<
+  "'this' argument to member function %0 is an %select{lvalue|rvalue}1, "
+  "but function has %select{non-const lvalue|rvalue}2 ref-qualifier">;
+def err_member_function_call_bad_type : Error<
+  "cannot initialize object parameter of type %0 with an expression "
+  "of type %1">;
+
 def warn_call_to_pure_virtual_member_function_from_ctor_dtor : Warning<
   "call to pure virtual member function %0 has undefined behavior; "
   "overrides of %0 in subclasses are not available in the "
@@ -1815,10 +1823,6 @@ def warn_temporary_array_to_pointer_decay : Warning<
 def err_init_list_bad_dest_type : Error<
   "%select{|non-aggregate }0type %1 cannot be initialized with an initializer "
   "list">;
-def err_member_function_call_bad_cvr : Error<"member function %0 not viable: "
-    "'this' argument has type %1, but function is not marked "
-    "%select{const|restrict|const or restrict|volatile|const or volatile|"
-    "volatile or restrict|const, volatile, or restrict}2">;
 
 def err_reference_bind_to_bitfield : Error<
   "%select{non-const|volatile}0 reference cannot bind to "
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 2144845f4dd..07d931552f3 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -5145,7 +5145,8 @@ Sema::PerformObjectArgumentInitialization(Expr *From,
       *this, From->getLocStart(), From->getType(), FromClassification, Method,
       Method->getParent());
   if (ICS.isBad()) {
-    if (ICS.Bad.Kind == BadConversionSequence::bad_qualifiers) {
+    switch (ICS.Bad.Kind) {
+    case BadConversionSequence::bad_qualifiers: {
       Qualifiers FromQs = FromRecordType.getQualifiers();
       Qualifiers ToQs = DestType.getQualifiers();
       unsigned CVR = FromQs.getCVRQualifiers() & ~ToQs.getCVRQualifiers();
@@ -5158,10 +5159,28 @@ Sema::PerformObjectArgumentInitialization(Expr *From,
           << Method->getDeclName();
         return ExprError();
       }
+      break;
+    }
+
+    case BadConversionSequence::lvalue_ref_to_rvalue:
+    case BadConversionSequence::rvalue_ref_to_lvalue: {
+      bool IsRValueQualified =
+        Method->getRefQualifier() == RefQualifierKind::RQ_RValue;
+      Diag(From->getLocStart(), diag::err_member_function_call_bad_ref)
+        << Method->getDeclName() << FromClassification.isRValue()
+        << IsRValueQualified;
+      Diag(Method->getLocation(), diag::note_previous_decl)
+        << Method->getDeclName();
+      return ExprError();
+    }
+
+    case BadConversionSequence::no_conversion:
+    case BadConversionSequence::unrelated_class:
+      break;
     }
 
     return Diag(From->getLocStart(),
-                diag::err_implicit_object_parameter_init)
+                diag::err_member_function_call_bad_type)
        << ImplicitParamRecordType << FromRecordType << From->getSourceRange();
   }
 
diff --git a/clang/test/CXX/over/over.match/over.match.funcs/p4-0x.cpp b/clang/test/CXX/over/over.match/over.match.funcs/p4-0x.cpp
index 68c79900b95..df7762703c0 100644
--- a/clang/test/CXX/over/over.match/over.match.funcs/p4-0x.cpp
+++ b/clang/test/CXX/over/over.match/over.match.funcs/p4-0x.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 template<typename T> T &lvalue();
 template<typename T> T &&xvalue();
@@ -20,6 +19,18 @@ struct X0 {
 
   void g();
 
+  void c() const; // expected-note {{'c' declared here}}
+  void v() volatile; // expected-note {{'v' declared here}}
+  void r() __restrict__; // expected-note {{'r' declared here}}
+  void cr() const __restrict__; // expected-note {{'cr' declared here}}
+  void cv() const volatile;
+  void vr() volatile __restrict__; // expected-note {{'vr' declared here}}
+  void cvr() const volatile __restrict__;
+
+  void lvalue() &; // expected-note 2 {{'lvalue' declared here}}
+  void const_lvalue() const&;
+  void rvalue() &&; // expected-note {{'rvalue' declared here}}
+
   int &operator+(const X0&) &;
   float &operator+(const X0&) &&;
 
@@ -32,7 +43,7 @@ struct X0 {
   float &h2() const&&;
 };
 
-void X0::g() {
+void X0::g() { // expected-note {{'g' declared here}}
   int &ir1 = f();
   int &ir2 = X0::f();
 }
@@ -69,3 +80,26 @@ void test_ref_qualifier_overloading() {
   float &fr3 = xvalue<X0>().h2();
   float &fr4 = prvalue<X0>().h2();
 }
+
+void test_diagnostics(const volatile X0 &__restrict__ cvr) {
+  cvr.g(); // expected-error {{'this' argument to member function 'g' has type 'const volatile X0', but function is not marked const or volatile}}
+  cvr.c(); // expected-error {{not marked volatile}}
+  cvr.v(); // expected-error {{not marked const}}
+  cvr.r(); // expected-error {{not marked const or volatile}}
+  cvr.cr(); // expected-error {{not marked volatile}}
+  cvr.cv();
+  cvr.vr(); // expected-error {{not marked const}}
+  cvr.cvr();
+
+  lvalue<X0>().lvalue();
+  lvalue<X0>().const_lvalue();
+  lvalue<X0>().rvalue(); // expected-error {{'this' argument to member function 'rvalue' is an lvalue, but function has rvalue ref-qualifier}}
+
+  xvalue<X0>().lvalue(); // expected-error {{'this' argument to member function 'lvalue' is an rvalue, but function has non-const lvalue ref-qualifier}}
+  xvalue<X0>().const_lvalue();
+  xvalue<X0>().rvalue();
+
+  prvalue<X0>().lvalue(); // expected-error {{'this' argument to member function 'lvalue' is an rvalue, but function has non-const lvalue ref-qualifier}}
+  prvalue<X0>().const_lvalue();
+  prvalue<X0>().rvalue();
+}
diff --git a/clang/test/SemaCXX/copy-initialization.cpp b/clang/test/SemaCXX/copy-initialization.cpp
index 4f6c65cf550..cd7e5f07e14 100644
--- a/clang/test/SemaCXX/copy-initialization.cpp
+++ b/clang/test/SemaCXX/copy-initialization.cpp
@@ -26,7 +26,7 @@ struct foo {
 };
 
 // PR3600
-void test(const foo *P) { P->bar(); } // expected-error{{'bar' not viable: 'this' argument has type 'const foo', but function is not marked const}}
+void test(const foo *P) { P->bar(); } // expected-error{{'this' argument to member function 'bar' has type 'const foo', but function is not marked const}}
 
 namespace PR6757 {
   struct Foo {