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

Qiu Chaofan via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 5 00:08:21 PST 2023


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/4] [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 18c2e861385e4..e60a7513d54e5 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 60786a880b9d3..b2c1f1fff9d7e 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 72abfc3cf51e1..003f4826a3d6c 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/4] 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 003f4826a3d6c..3a10e15a663a5 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 *')}}
   };
 }

>From 5c7de8c5259d0738664a25ac48c36f9354ac953e Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <qiucofan at cn.ibm.com>
Date: Thu, 9 Nov 2023 17:50:31 +0800
Subject: [PATCH 3/4] Add code and release note

---
 clang/docs/ReleaseNotes.rst                      | 5 +++++
 clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 ----
 clang/lib/Sema/SemaDeclCXX.cpp                   | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7a131cb520aa6..4e8ef23679284 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -693,6 +693,11 @@ Bug Fixes to C++ Support
   completes (except deduction guides). Fixes:
   (`#59827 <https://github.com/llvm/llvm-project/issues/59827>`_)
 
+- Clang now reports error when overriden method's non-class return type drops
+  qualifiers, or qualifiers of class return type are not subset of super method's.
+  Fixes:
+  (`#18233 <https://github.com/llvm/llvm-project/issues/18233>`_)
+
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 755a71b0d90e6..f53f2697d1e15 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2112,10 +2112,6 @@ 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<
-  "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)">;
 
 // 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 b2c1f1fff9d7e..9791b10cf3782 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18471,7 +18471,7 @@ bool Sema::CheckOverridingFunctionReturnType(const CXXMethodDecl *New,
   // The new class type must have the same or less qualifiers as the old type.
   if (!OldClassTy.isAtLeastAsQualifiedAs(NewClassTy)) {
     Diag(New->getLocation(),
-         diag::err_covariant_return_type_class_type_more_qualified)
+         diag::err_covariant_return_type_different_qualifications)
         << New->getDeclName() << NewTy << OldTy
         << New->getReturnTypeSourceRange();
     Diag(Old->getLocation(), diag::note_overridden_virtual_function)

>From 1ec82c339ac2c891808d1be4a7208a3b5668fcb2 Mon Sep 17 00:00:00 2001
From: Qiu Chaofan <qiucofan at cn.ibm.com>
Date: Tue, 5 Dec 2023 16:07:57 +0800
Subject: [PATCH 4/4] Add standard reference

---
 clang/lib/Sema/SemaDeclCXX.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 53e5f2f521781..354b6bc7e01df 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18448,7 +18448,9 @@ bool Sema::CheckOverridingFunctionReturnType(const CXXMethodDecl *New,
     return true;
   }
 
-
+  // C++ [class.virtual]p8:
+  //   The return type of an overriding function shall be either identical to
+  //   the return type of the overridden function or [covariant]
   // The new class type must have the same or less qualifiers as the old type.
   if (!OldClassTy.isAtLeastAsQualifiedAs(NewClassTy)) {
     Diag(New->getLocation(),



More information about the cfe-commits mailing list