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

[via cfe-commits]

Author: Boaz Brickner
Date: 2024-10-17T16:50:47+02:00
New Revision: 8f25c0bc7d59a65f27faa88d7debc47275a3a3da

URL: https://github.com/llvm/llvm-project/commit/8f25c0bc7d59a65f27faa88d7debc47275a3a3da
DIFF: https://github.com/llvm/llvm-project/commit/8f25c0bc7d59a65f27faa88d7debc47275a3a3da.diff

LOG: [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

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/lib/Sema/SemaDeclCXX.cpp
    clang/test/SemaCXX/virtual-override.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9977e8bd3ca672..1da8c82d52e618 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/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

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c458a62d9be48c..487dd8990d88e9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2182,10 +2182,10 @@ def err_covariant_return_incomplete : Error<
 def err_covariant_return_type_
diff erent_qualifications : Error<
   "return type of virtual function %0 is not covariant with the return type of "
   "the function it overrides (%1 has 
diff erent 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<

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 38f808a470aa87..43ec25b23d972a 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/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)

diff  --git a/clang/test/SemaCXX/virtual-override.cpp b/clang/test/SemaCXX/virtual-override.cpp
index d37c275d46baeb..ce6dd35e0b56fa 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/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 
diff erent 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 
diff erent 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 
diff erent 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 
diff erent return type ('const int *') than the function it overrides (which has return type 'int *')}}
 };
 
 }