diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp @@ -19,6 +19,21 @@ namespace tidy { namespace cppcoreguidelines { +AST_MATCHER(CXXRecordDecl, hasPublicVirtualOrProtectedNonVirtualDestructor) { + // We need to call Node.getDestructor() instead of matching a + // CXXDestructorDecl. Otherwise, tests will fail for class templates, since + // the primary template (not the specialization) always gets a non-virtual + // CXXDestructorDecl in the AST. https://bugs.llvm.org/show_bug.cgi?id=51912 + const CXXDestructorDecl *Destructor = Node.getDestructor(); + if (!Destructor) + return false; + + return (((Destructor->getAccess() == AccessSpecifier::AS_public) && + Destructor->isVirtual()) || + ((Destructor->getAccess() == AccessSpecifier::AS_protected) && + !Destructor->isVirtual())); +} + void VirtualClassDestructorCheck::registerMatchers(MatchFinder *Finder) { ast_matchers::internal::Matcher InheritsVirtualMethod = hasAnyBase(hasType(cxxRecordDecl(has(cxxMethodDecl(isVirtual()))))); @@ -26,9 +41,7 @@ Finder->addMatcher( cxxRecordDecl( anyOf(has(cxxMethodDecl(isVirtual())), InheritsVirtualMethod), - unless(anyOf( - has(cxxDestructorDecl(isPublic(), isVirtual())), - has(cxxDestructorDecl(isProtected(), unless(isVirtual())))))) + unless(hasPublicVirtualOrProtectedNonVirtualDestructor())) .bind("ProblematicClassOrStruct"), this); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp @@ -202,3 +202,73 @@ 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 +struct TemplatedDerived : PublicVirtualBaseStruct { +}; + +TemplatedDerived 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 +struct DerivedFromTemplateVirtualBaseStruct : T { + virtual void foo(); +}; + +DerivedFromTemplateVirtualBaseStruct 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' 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 +struct DerivedFromTemplateNonVirtualBaseStruct : T { + virtual void foo(); +}; + +DerivedFromTemplateNonVirtualBaseStruct InstantiationWithPublicNonVirtualBaseStruct; + +// Derived from template, base has virtual dtor, to be used in a typedef +// CHECK-MESSAGES-NOT: :[[@LINE+2]]:8: warning: destructor of 'DerivedFromTemplateVirtualBaseStruct2' is public and non-virtual [cppcoreguidelines-virtual-class-destructor] +template +struct DerivedFromTemplateVirtualBaseStruct2 : T { + virtual void foo(); +}; + +using DerivedFromTemplateVirtualBaseStruct2Typedef = DerivedFromTemplateVirtualBaseStruct2; +DerivedFromTemplateVirtualBaseStruct2Typedef InstantiationWithPublicVirtualBaseStruct2; + +// Derived from template, base has *not* virtual dtor, to be used in a typedef +// CHECK-MESSAGES: :[[@LINE+8]]:8: warning: destructor of 'DerivedFromTemplateNonVirtualBaseStruct2' 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 'DerivedFromTemplateNonVirtualBaseStruct2' is public and non-virtual [cppcoreguidelines-virtual-class-destructor] +// CHECK-FIXES: struct DerivedFromTemplateNonVirtualBaseStruct2 : T { +// CHECK-FIXES-NEXT: virtual ~DerivedFromTemplateNonVirtualBaseStruct2() = default; +// CHECK-FIXES-NEXT: virtual void foo(); +// CHECK-FIXES-NEXT: }; +template +struct DerivedFromTemplateNonVirtualBaseStruct2 : T { + virtual void foo(); +}; + +using DerivedFromTemplateNonVirtualBaseStruct2Typedef = DerivedFromTemplateNonVirtualBaseStruct2; +DerivedFromTemplateNonVirtualBaseStruct2Typedef InstantiationWithPublicNonVirtualBaseStruct2; + +} // namespace Bugzilla_51912