Skip to content

Commit d9afb8c

Browse files
author
Balazs Benics
committed
[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
1 parent 3f81841 commit d9afb8c

File tree

3 files changed

+26
-0
lines changed

3 files changed

+26
-0
lines changed

clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ void VirtualClassDestructorCheck::registerMatchers(MatchFinder *Finder) {
4141
Finder->addMatcher(
4242
cxxRecordDecl(
4343
anyOf(has(cxxMethodDecl(isVirtual())), InheritsVirtualMethod),
44+
unless(isFinal()),
4445
unless(hasPublicVirtualOrProtectedNonVirtualDestructor()))
4546
.bind("ProblematicClassOrStruct"),
4647
this);

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,12 @@ Changes in existing checks
165165
Fixed an issue when there was already an initializer in the constructor and
166166
the check would try to create another initializer for the same member.
167167

168+
- Fixed a false positive in :doc:`cppcoreguidelines-virtual-class-destructor
169+
<clang-tidy/checks/cppcoreguidelines-virtual-class-destructor>` involving
170+
``final`` classes. The check will not diagnose classes marked ``final``, since
171+
those cannot be used as base classes, consequently, they can not violate the
172+
rule.
173+
168174
- Fixed a crash in :doc:`llvmlibc-callee-namespace
169175
<clang-tidy/checks/llvmlibc/callee-namespace>` when executing for C++ code
170176
that contain calls to advanced constructs, e.g. overloaded operators.

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,3 +320,22 @@ class FooBar5 {
320320
#undef XMACRO
321321
#undef CONCAT
322322
} // namespace macro_tests
323+
324+
namespace FinalClassCannotBeBaseClass {
325+
class Base {
326+
public:
327+
Base() = default;
328+
virtual void func() = 0;
329+
330+
protected:
331+
~Base() = default;
332+
};
333+
334+
// no-warning: 'MostDerived' cannot be a base class, since it's marked 'final'.
335+
class MostDerived final : public Base {
336+
public:
337+
MostDerived() = default;
338+
~MostDerived() = default;
339+
void func() final;
340+
};
341+
} // namespace FinalClassCannotBeBaseClass

0 commit comments

Comments
 (0)