[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