carlosgalvezp updated this revision to Diff 375545. carlosgalvezp retitled this revision from "[clang-tidy] Fix false positive in cppcoreguidelines-virtual-class-destructor " to "[clang-tidy] Fix false positives in cppcoreguidelines-virtual-class-destructor ". carlosgalvezp edited the summary of this revision. carlosgalvezp added a comment.
Added additional test case for forward declarations, which is also broken on main. 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,23 @@ void m(); }; // inherits virtual method + +// The following two checks cover bug https://bugs.llvm.org/show_bug.cgi?id=51912 +// Forward declarations +// CHECK-MESSAGES-NOT: :[[@LINE+1]]:8: warning: destructor of 'ForwardDeclaredClass' is public and non-virtual [cppcoreguidelines-virtual-class-destructor] +struct ForwardDeclaredClass; + +struct ForwardDeclaredClass : PublicVirtualBaseStruct{ + +}; + +// Templates +// 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 { +}; + +// Templates need to be instantiated for diagnostics to show up +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,23 @@ void m(); }; // inherits virtual method + +// The following two checks cover bug https://bugs.llvm.org/show_bug.cgi?id=51912 +// Forward declarations +// CHECK-MESSAGES-NOT: :[[@LINE+1]]:8: warning: destructor of 'ForwardDeclaredClass' is public and non-virtual [cppcoreguidelines-virtual-class-destructor] +struct ForwardDeclaredClass; + +struct ForwardDeclaredClass : PublicVirtualBaseStruct{ + +}; + +// Templates +// 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 { +}; + +// Templates need to be instantiated for diagnostics to show up +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