[libcxx-commits] [clang] [compiler-rt] [libcxx] [flang] [llvm] [clang-tools-extra] [Clang][Sema] Fix qualifier restriction of overriden methods (PR #71696)

[Qiu Chaofan via libcxx-commits]

https://github.com/ecnelises updated https://github.com/llvm/llvm-project/pull/71696

>From 1d0109b7f370a3689a92e20ab52597b112669e47 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <qiucofan at cn.ibm.com>
Date: Thu, 9 Nov 2023 00:00:26 +0800
Subject: [PATCH 1/2] [Clang][Sema] Fix qualifier restriction of overriden
 methods

If return type of overriden method is pointer or reference to
non-class type, qualifiers cannot be dropped. This also fixes check
when qualifier of overriden method's class return type is not subset
of super method's.
---
 .../clang/Basic/DiagnosticSemaKinds.td        |  2 +-
 clang/lib/Sema/SemaDeclCXX.cpp                | 15 +++++++++-
 clang/test/SemaCXX/virtual-override.cpp       | 28 ++++++++++++++++++-
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 18c2e861385e463..e60a7513d54e552 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2115,7 +2115,7 @@ def err_covariant_return_type_different_qualifications : Error<
 def err_covariant_return_type_class_type_more_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">;
+  "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 60786a880b9d3fd..b2c1f1fff9d7e7b 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18469,7 +18469,7 @@ 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)
         << New->getDeclName() << NewTy << OldTy
@@ -18479,6 +18479,19 @@ bool Sema::CheckOverridingFunctionReturnType(const CXXMethodDecl *New,
     return true;
   }
 
+  // Non-class return types should not drop qualifiers in overriden method.
+  if (!OldClassTy->isStructureOrClassType() &&
+      OldClassTy.getLocalCVRQualifiers() !=
+          NewClassTy.getLocalCVRQualifiers()) {
+    Diag(New->getLocation(),
+         diag::err_covariant_return_type_different_qualifications)
+        << New->getDeclName() << NewTy << OldTy
+        << New->getReturnTypeSourceRange();
+    Diag(Old->getLocation(), diag::note_overridden_virtual_function)
+        << Old->getReturnTypeSourceRange();
+    return true;
+  }
+
   return false;
 }
 
diff --git a/clang/test/SemaCXX/virtual-override.cpp b/clang/test/SemaCXX/virtual-override.cpp
index 72abfc3cf51e1f7..003f4826a3d6c86 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -87,7 +87,7 @@ class A {
 
 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 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 *')}}
 };
 
 }
@@ -289,3 +289,29 @@ namespace PR8168 {
     static void foo() {} // expected-error{{'static' member function 'foo' overrides a virtual function}}
   };
 }
+
+namespace T13 {
+  class A {
+  public:
+    virtual const int* foo(); // expected-note{{overridden virtual function is here}}
+  };
+
+  class B: public A {
+  public:
+    virtual int* foo(); // expected-error{{return type of virtual function 'foo' is not covariant with the return type of the function it overrides ('int *' has different qualifiers than 'const int *')}}
+  };
+}
+
+namespace T14 {
+  struct a {};
+
+  class A {
+  public:
+    virtual const a* foo(); // expected-note{{overridden virtual function is here}}
+  };
+
+  class B: public A {
+  public:
+    virtual volatile a* foo(); // expected-error{{return type of virtual function 'foo' is not covariant with the return type of the function it overrides (class type 'volatile a *' is more qualified than class type 'const a *')}}
+  };
+}

>From 5f64fec64b51542abd72a9a870ae9e5fe357d026 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <qiucofan at cn.ibm.com>
Date: Thu, 9 Nov 2023 17:49:33 +0800
Subject: [PATCH 2/2] Say 'different qualifiers' instead of 'more qualified'

---
 clang/test/SemaCXX/virtual-override.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/test/SemaCXX/virtual-override.cpp b/clang/test/SemaCXX/virtual-override.cpp
index 003f4826a3d6c86..3a10e15a663a50a 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -87,7 +87,7 @@ class A {
 
 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 const a* g(); // expected-error{{return type of virtual function 'g' is not covariant with the return type of the function it overrides ('const a *' has different qualifiers than 'a *')}}
 };
 
 }
@@ -312,6 +312,6 @@ namespace T14 {
 
   class B: public A {
   public:
-    virtual volatile a* foo(); // expected-error{{return type of virtual function 'foo' is not covariant with the return type of the function it overrides (class type 'volatile a *' is more qualified than class type 'const a *')}}
+    virtual volatile a* foo(); // expected-error{{return type of virtual function 'foo' is not covariant with the return type of the function it overrides ('volatile a *' has different qualifiers than 'const a *')}}
   };
 }