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

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 376116.
carlosgalvezp added a comment.

Added additional required tests.

I might take some more time to fix the matcher since I need to get acquainted with AST, clang-query, etc, etc


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110614/new/

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,47 @@
   void m();
 };
 // inherits virtual method
+
+namespace Bugzilla_51912 {
+// Fixes https://bugs.llvm.org/show_bug.cgi?id=51912
+
+// Forward declarations
+// CHECK-MESSAGES-NOT: :[[@LINE+1]]:8: warning: destructor of 'ForwardDeclaredStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+struct ForwardDeclaredStruct;
+
+struct ForwardDeclaredStruct : PublicVirtualBaseStruct {
+};
+
+// Normal Template
+// 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 {
+};
+
+TemplatedDerived<int> InstantiationWithInt;
+
+// Derived from template, base has virtual dtor
+// CHECK-MESSAGES-NOT: :[[@LINE+2]]:8: warning: destructor of 'DerivedFromTemplateVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+template <typename T>
+struct DerivedFromTemplateVirtualBaseStruct : T {
+  virtual void foo();
+};
+
+DerivedFromTemplateVirtualBaseStruct<PublicVirtualBaseStruct> InstantiationWithPublicVirtualBaseStruct;
+
+// Derived from template, base has *not* virtual dtor
+// CHECK-MESSAGES: :[[@LINE+8]]:8: warning: destructor of 'DerivedFromTemplateNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+7]]:8: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+6]]:8: warning: destructor of 'DerivedFromTemplateNonVirtualBaseStruct<PublicNonVirtualBaseStruct>' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-FIXES: struct DerivedFromTemplateNonVirtualBaseStruct : T {
+// CHECK-FIXES-NEXT: virtual ~DerivedFromTemplateNonVirtualBaseStruct() = default;
+// CHECK-FIXES-NEXT: virtual void foo();
+// CHECK-FIXES-NEXT: };
+template <typename T>
+struct DerivedFromTemplateNonVirtualBaseStruct : T {
+  virtual void foo();
+};
+
+DerivedFromTemplateNonVirtualBaseStruct<PublicNonVirtualBaseStruct> InstantiationWithPublicNonVirtualBaseStruct;
+
+}  // namespace Bugzilla_51912
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,14 @@
   ast_matchers::internal::Matcher<CXXRecordDecl> InheritsVirtualMethod =
       hasAnyBase(hasType(cxxRecordDecl(has(cxxMethodDecl(isVirtual())))));
 
+  ast_matchers::internal::Matcher<CXXRecordDecl> InheritsVirtualDestructor =
+      hasAnyBase(hasType(cxxRecordDecl(has(cxxDestructorDecl(isVirtual())))));
+
   Finder->addMatcher(
       cxxRecordDecl(
           anyOf(has(cxxMethodDecl(isVirtual())), InheritsVirtualMethod),
           unless(anyOf(
+              InheritsVirtualDestructor,
               has(cxxDestructorDecl(isPublic(), isVirtual())),
               has(cxxDestructorDecl(isProtected(), unless(isVirtual()))))))
           .bind("ProblematicClassOrStruct"),


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D110614.376116.patch
Type: text/x-patch
Size: 3515 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210930/53bb690d/attachment.bin>