[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann marked 17 inline comments as done.
mgartmann added a comment.

In D102325#2754241 <https://reviews.llvm.org/D102325#2754241>, @njames93 wrote:

> Whats the intended behaviour for derived classes and their destructors? Can test be added to demonstrate that behaviour?

If a derived class inherits a `virtual` method from a base class, according to guideline C.35, it is a potential base class as well.
Therefore, such a derived class gets flagged by the check if its destructor is not specified correctly.

In order to flag derived classes that only inherit a `virtual` method but do not override it, the following matcher had to be added:

  ast_matchers::internal::Matcher<CXXRecordDecl> inheritsVirtualMethod =
      hasAnyBase(hasType(cxxRecordDecl(has(cxxMethodDecl(isVirtual())))));

Test cases were added to demonstrate this behaviour.



================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:129
+
+AccessSpecDecl *VirtualBaseClassDestructorCheck::getPublicASDecl(
+    const CXXRecordDecl &StructOrClass) const {
----------------
njames93 wrote:
> Can be made static again. Also should probably return a `const AccessSpecDecl *`
@njames93 If I get this correctly, only `check` and `addMatcher` should be member functions of a check's class. All the other aid methods should be static and not part of the class. 

Did I get this right?


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-base-class-destructor.rst:55
+
+.. option:: IndentationWidth
+
----------------
Eugene.Zelenko wrote:
> Shouldn't `Clang-format` take care about such things?
As @njames93 also suggested leaving the indentation to `clang-format`, I removed this option and the indentation functionality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325