Author: Balazs Benics Date: 2022-06-21T11:02:18+02:00 New Revision: d9afb8c3e8fd01a3c89ab2ddebcd44602a30a975
URL: https://github.com/llvm/llvm-project/commit/d9afb8c3e8fd01a3c89ab2ddebcd44602a30a975 DIFF: https://github.com/llvm/llvm-project/commit/d9afb8c3e8fd01a3c89ab2ddebcd44602a30a975.diff LOG: [clang-tidy] cppcoreguidelines-virtual-class-destructor should ignore final classes The `cppcoreguidelines-virtual-class-destructor` supposed to enforce http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c35-a-base-class-destructor-should-be-either-public-and-virtual-or-protected-and-non-virtual Quote: > A **base** class destructor should be either public and virtual, or > protected and non-virtual [emphasis mine] However, this check still rules the following case: class MostDerived final : public Base { public: MostDerived() = default; ~MostDerived() = default; void func() final; }; Even though `MostDerived` class is marked `final`, thus it should not be considered as a **base** class. Consequently, the rule is satisfied, yet the check still flags this code. In this patch, I'm proposing to ignore `final` classes since they cannot be //base// classes. Reviewed By: whisperity Differential Revision: https://reviews.llvm.org/D126891 Added: Modified: clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp index 26ee7ca6eca57..e730fb6fa01db 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp @@ -41,6 +41,7 @@ void VirtualClassDestructorCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( cxxRecordDecl( anyOf(has(cxxMethodDecl(isVirtual())), InheritsVirtualMethod), + unless(isFinal()), unless(hasPublicVirtualOrProtectedNonVirtualDestructor())) .bind("ProblematicClassOrStruct"), this); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 36cac762f0d31..26d99420d3e4b 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -165,6 +165,12 @@ Changes in existing checks Fixed an issue when there was already an initializer in the constructor and the check would try to create another initializer for the same member. +- Fixed a false positive in :doc:`cppcoreguidelines-virtual-class-destructor + <clang-tidy/checks/cppcoreguidelines-virtual-class-destructor>` involving + ``final`` classes. The check will not diagnose classes marked ``final``, since + those cannot be used as base classes, consequently, they can not violate the + rule. + - Fixed a crash in :doc:`llvmlibc-callee-namespace <clang-tidy/checks/llvmlibc/callee-namespace>` when executing for C++ code that contain calls to advanced constructs, e.g. overloaded operators. diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp index c14a2e68def88..fcf558dcac8e6 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp @@ -320,3 +320,22 @@ class FooBar5 { #undef XMACRO #undef CONCAT } // namespace macro_tests + +namespace FinalClassCannotBeBaseClass { +class Base { +public: + Base() = default; + virtual void func() = 0; + +protected: + ~Base() = default; +}; + +// no-warning: 'MostDerived' cannot be a base class, since it's marked 'final'. +class MostDerived final : public Base { +public: + MostDerived() = default; + ~MostDerived() = default; + void func() final; +}; +} // namespace FinalClassCannotBeBaseClass _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
