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 <qiuco...@cn.ibm.com> Date: Thu, 9 Nov 2023 00:00:26 +0800 Subject: [PATCH 1/3] [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 <qiuco...@cn.ibm.com> Date: Thu, 9 Nov 2023 17:49:33 +0800 Subject: [PATCH 2/3] 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 *')}} }; } >From 5c7de8c5259d0738664a25ac48c36f9354ac953e Mon Sep 17 00:00:00 2001 From: Qiu Chaofan <qiuco...@cn.ibm.com> Date: Thu, 9 Nov 2023 17:50:31 +0800 Subject: [PATCH 3/3] 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 7a131cb520aa600..4e8ef236792847b 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 755a71b0d90e6fc..f53f2697d1e152e 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 b2c1f1fff9d7e7b..9791b10cf3782c4 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) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits