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"),
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"),
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
