carlosgalvezp added a subscriber: whisperity. carlosgalvezp added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp:26 + ast_matchers::internal::Matcher<CXXRecordDecl> InheritsVirtualDestructor = + hasAnyBase(hasType(cxxRecordDecl(has(cxxDestructorDecl(isVirtual()))))); ---------------- aaron.ballman wrote: > I'm confused as to why this is necessary -- this AST matcher calls through to > `CXXMethodDecl::isVirtual()` which is different from `isVirtualAsWritten()` > and should already account for inheriting the virtual specifier. In the Bug report, @whisperity mentioned this problem could be somewhere else: > Nevertheless, if you check the AST for the test code at > http://godbolt.org/z/6MqG4Kb85, we can see that the derived destructor is in > fact **not** marked virtual. So it's Sema which does not give this flag to > the AST-clients properly. I don't even know what Sema is so I don't really know how to fix it at that level, and I wonder if it would break other things. If anyone has some time to walk me through where the fix should go I'm happy to try it out! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110614/new/ https://reviews.llvm.org/D110614 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
