[PATCH] D110614: [clang-tidy] Fix false positive in cppcoreguidelines-virtual-class-destructor

[Carlos Galvez via Phabricator via cfe-commits]

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"),


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D110614.375500.patch
Type: text/x-patch
Size: 1759 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210928/7d677d1e/attachment.bin>