[clang] Fix covariant cv-qualification check to require the override function return type to have the same or less cv-qualification (#112713)

This prevents changing cv-qualification from const to volatile or vice
versa, for example.

https://eel.is/c++draft/class.virtual#8.3

Previously, we checked that the new type is the same or more qualified
to return an error, but the standard requires the new type to be the
same or less qualified and since the cv-qualification is only partially
ordered, we cannot rely on a check on whether it is more qualified to
return an error. Now, we reversed the condition to check whether the old
is at least as qualified, and return an error if it is not.

Also, adjusted the error name and message to clarify the requirement and
added a missing closing parenthesis.

Added tests to cover different use cases for classes with different
qualifications and also refactored them to make them easier to follow:
1. Use override to make sure the function names actually match.
2. Named the function in a more descriptive way to clarify what each use
case is checking.

Fixes: #111742

Author: bricknerb

clang/docs/ReleaseNotes.rst

@@ -99,17 +99,24 @@ C++ Specific Potentially Breaking Changes
     // Was error, now evaluates to false.
     constexpr bool b = f() == g();
 
-- Clang will now correctly not consider pointers to non classes for covariance.
+- Clang will now correctly not consider pointers to non classes for covariance
+  and disallow changing return type to a type that doesn't have the same or less cv-qualifications.
 
   .. code-block:: c++
 
     struct A {
       virtual const int *f() const;
+      virtual const std::string *g() const;
     };
     struct B : A {
       // Return type has less cv-qualification but doesn't point to a class.
       // Error will be generated.
       int *f() const override;
+
+      // Return type doesn't have more cv-qualification also not the same or
+      // less cv-qualification.
+      // Error will be generated.
+      volatile std::string *g() const override;
     };
 
 - The warning ``-Wdeprecated-literal-operator`` is now on by default, as this is

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -2182,10 +2182,10 @@ def err_covariant_return_incomplete : Error<
 def err_covariant_return_type_different_qualifications : Error<
   "return type of virtual function %0 is not covariant with the return type of "
   "the function it overrides (%1 has different qualifiers than %2)">;
-def err_covariant_return_type_class_type_more_qualified : Error<
+def err_covariant_return_type_class_type_not_same_or_less_qualified : Error<
   "return type of virtual function %0 is not covariant with the return type of "
-  "the function it overrides (class type %1 is more qualified than class "
-  "type %2">;
+  "the function it overrides (class type %1 does not have the same "
+  "cv-qualification as or less cv-qualification than class type %2)">;
 
 // C++ implicit special member functions
 def note_in_declaration_of_implicit_special_member : Note<

clang/lib/Sema/SemaDeclCXX.cpp

@@ -18338,9 +18338,9 @@ bool Sema::CheckOverridingFunctionReturnType(const CXXMethodDecl *New,
 
 
   // The new class type must have the same or less qualifiers as the old type.
-  if (NewClassTy.isMoreQualifiedThan(OldClassTy)) {
+  if (!OldClassTy.isAtLeastAsQualifiedAs(NewClassTy)) {
     Diag(New->getLocation(),
-         diag::err_covariant_return_type_class_type_more_qualified)
+         diag::err_covariant_return_type_class_type_not_same_or_less_qualified)
         << New->getDeclName() << NewTy << OldTy
         << New->getReturnTypeSourceRange();
     Diag(Old->getLocation(), diag::note_overridden_virtual_function)

clang/test/SemaCXX/virtual-override.cpp

@@ -83,17 +83,53 @@ namespace T6 {
 struct a { };
 
 class A {
-  virtual const a* f(); 
-  virtual a* g(); // expected-note{{overridden virtual function is here}}
-  virtual const int* h(); // expected-note{{overridden virtual function is here}}
-  virtual int* i(); // expected-note{{overridden virtual function is here}}
+  // Classes.
+  virtual const a* const_vs_unqualified_class();
+  virtual a* unqualified_vs_const_class(); // expected-note{{overridden virtual function is here}}
+
+  virtual volatile a* volatile_vs_unqualified_class();
+  virtual a* unqualified_vs_volatile_class(); // expected-note{{overridden virtual function is here}}
+
+  virtual const a* const_vs_volatile_class(); // expected-note{{overridden virtual function is here}}
+  virtual volatile a* volatile_vs_const_class(); // expected-note{{overridden virtual function is here}}
+
+  virtual const volatile a* const_volatile_vs_const_class();
+  virtual const a* const_vs_const_volatile_class(); // expected-note{{overridden virtual function is here}}
+
+  virtual const volatile a* const_volatile_vs_volatile_class();
+  virtual volatile a* volatile_vs_const_volatile_class(); // expected-note{{overridden virtual function is here}}
+
+  virtual const volatile a* const_volatile_vs_unualified_class();
+  virtual a* unqualified_vs_const_volatile_class(); // expected-note{{overridden virtual function is here}}
+
+  // Non Classes.
+  virtual const int* const_vs_unqualified_non_class(); // expected-note{{overridden virtual function is here}}
+  virtual int* unqualified_vs_const_non_class(); // expected-note{{overridden virtual function is here}}
 };
 
 class B : A {
-  virtual a* f(); 
-  virtual const a* g(); // expected-error{{return type of virtual function 'g' is not covariant with the return type of the function it overrides (class type 'const a *' is more qualified than class type 'a *'}}
-  virtual int* h();  // expected-error{{virtual function 'h' has a different return type ('int *') than the function it overrides (which has return type 'const int *')}}
-  virtual const int* i(); // expected-error{{virtual function 'i' has a different return type ('const int *') than the function it overrides (which has return type 'int *')}}
+  // Classes.
+  a* const_vs_unqualified_class() override;
+  const a* unqualified_vs_const_class() override; // expected-error{{return type of virtual function 'unqualified_vs_const_class' is not covariant with the return type of the function it overrides (class type 'const a *' does not have the same cv-qualification as or less cv-qualification than class type 'a *')}}
+
+  a* volatile_vs_unqualified_class() override;
+  volatile a* unqualified_vs_volatile_class() override; // expected-error{{return type of virtual function 'unqualified_vs_volatile_class' is not covariant with the return type of the function it overrides (class type 'volatile a *' does not have the same cv-qualification as or less cv-qualification than class type 'a *')}}
+
+  volatile a* const_vs_volatile_class() override; // expected-error{{return type of virtual function 'const_vs_volatile_class' is not covariant with the return type of the function it overrides (class type 'volatile a *' does not have the same cv-qualification as or less cv-qualification than class type 'const a *')}}
+  const a* volatile_vs_const_class() override; // expected-error{{return type of virtual function 'volatile_vs_const_class' is not covariant with the return type of the function it overrides (class type 'const a *' does not have the same cv-qualification as or less cv-qualification than class type 'volatile a *')}}
+
+  const a* const_volatile_vs_const_class() override;
+  const volatile a* const_vs_const_volatile_class() override; // expected-error{{return type of virtual function 'const_vs_const_volatile_class' is not covariant with the return type of the function it overrides (class type 'const volatile a *' does not have the same cv-qualification as or less cv-qualification than class type 'const a *')}}
+
+  volatile a* const_volatile_vs_volatile_class() override;
+  const volatile a* volatile_vs_const_volatile_class() override; // expected-error{{return type of virtual function 'volatile_vs_const_volatile_class' is not covariant with the return type of the function it overrides (class type 'const volatile a *' does not have the same cv-qualification as or less cv-qualification than class type 'volatile a *')}}
+
+  a* const_volatile_vs_unualified_class() override;
+  const volatile a* unqualified_vs_const_volatile_class() override; // expected-error{{return type of virtual function 'unqualified_vs_const_volatile_class' is not covariant with the return type of the function it overrides (class type 'const volatile a *' does not have the same cv-qualification as or less cv-qualification than class type 'a *')}}
+
+  // Non Classes.
+  int* const_vs_unqualified_non_class() override; // expected-error{{virtual function 'const_vs_unqualified_non_class' has a different return type ('int *') than the function it overrides (which has return type 'const int *')}}
+  const int* unqualified_vs_const_non_class() override; // expected-error{{virtual function 'unqualified_vs_const_non_class' has a different return type ('const int *') than the function it overrides (which has return type 'int *')}}
 };
 
 }