carlosgalvezp created this revision.
carlosgalvezp added reviewers: mgartmann, aaron.ballman, alexfh, alexfh_.
carlosgalvezp added a project: clang-tools-extra.
Herald added subscribers: shchenz, kbarton, xazax.hun, nemanjai.
carlosgalvezp requested review of this revision.
Incorrectly triggers for template classes that inherit from a base class that
has virtual destructor.
Any class inheriting from a base that has a virtual destructor will have their
destructor also virtual, as per the Standard, so there's no need for them to
explicitly declare it as virtual.
https://timsong-cpp.github.io/cppwp/n4140/class.dtor#9
> If a class has a base class with a virtual destructor, its destructor
> (whether user- or implicitly-declared) is virtual.
Added unit test to prevent regression.
Fixes https://bugs.llvm.org/show_bug.cgi?id=51912
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D110614
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
Index:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
===================================================================
---
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
+++
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -202,3 +202,12 @@
void m();
};
// inherits virtual method
+
+// CHECK-MESSAGES-NOT: :[[@LINE+2]]:8: warning: destructor of
'TemplatedDerived' is public and non-virtual
[cppcoreguidelines-virtual-class-destructor]
+template <typename T>
+struct TemplatedDerived : PublicVirtualBaseStruct {
+};
+
+void test_templates() {
+ TemplatedDerived<int> x;
+}
Index:
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
===================================================================
---
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
+++
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
@@ -23,10 +23,15 @@
ast_matchers::internal::Matcher<CXXRecordDecl> InheritsVirtualMethod =
hasAnyBase(hasType(cxxRecordDecl(has(cxxMethodDecl(isVirtual())))));
+ ast_matchers::internal::Matcher<CXXRecordDecl> BaseHasPublicVirtualDtor =
+ hasAnyBase(hasType(
+ cxxRecordDecl(has(cxxDestructorDecl(isPublic(), isVirtual())))));
+
Finder->addMatcher(
cxxRecordDecl(
anyOf(has(cxxMethodDecl(isVirtual())), InheritsVirtualMethod),
unless(anyOf(
+ BaseHasPublicVirtualDtor,
has(cxxDestructorDecl(isPublic(), isVirtual())),
has(cxxDestructorDecl(isProtected(), unless(isVirtual()))))))
.bind("ProblematicClassOrStruct"),
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -202,3 +202,12 @@
void m();
};
// inherits virtual method
+
+// CHECK-MESSAGES-NOT: :[[@LINE+2]]:8: warning: destructor of 'TemplatedDerived' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+template <typename T>
+struct TemplatedDerived : PublicVirtualBaseStruct {
+};
+
+void test_templates() {
+ TemplatedDerived<int> x;
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
@@ -23,10 +23,15 @@
ast_matchers::internal::Matcher<CXXRecordDecl> InheritsVirtualMethod =
hasAnyBase(hasType(cxxRecordDecl(has(cxxMethodDecl(isVirtual())))));
+ ast_matchers::internal::Matcher<CXXRecordDecl> BaseHasPublicVirtualDtor =
+ hasAnyBase(hasType(
+ cxxRecordDecl(has(cxxDestructorDecl(isPublic(), isVirtual())))));
+
Finder->addMatcher(
cxxRecordDecl(
anyOf(has(cxxMethodDecl(isVirtual())), InheritsVirtualMethod),
unless(anyOf(
+ BaseHasPublicVirtualDtor,
has(cxxDestructorDecl(isPublic(), isVirtual())),
has(cxxDestructorDecl(isProtected(), unless(isVirtual()))))))
.bind("ProblematicClassOrStruct"),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
