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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

In D102325#2816919 <https://reviews.llvm.org/D102325#2816919>, @mgartmann wrote:

> I assume that these destructors are private by on purpose. Please correct me if I am wrong.

As best I can tell, these look purposeful to me.

> A programmer working on the LLVM project would therefore see seven warnings for private destructors over the whole project.

Wellllll not quite; because these are in header files, they'd see a lot more than just seven warnings.

> Is this number of private destructors which are flagged acceptable to you?

To be honest, I think this signals that we wouldn't be able to enable this guideline for our codebase because it's too chatty with no value. Based on the results, it's only pointing out false positives and no true positives. So from that perspective, it's not super acceptable. However, I've come to the conclusion that automated checking for the C++ Core Guidelines (in general, not just with your patch!) is not particularly useful for existing code bases because of the lack of consideration put into existing code by the guideline authors. From that perspective, I think this check is fine.

LGTM, thank you for your patience while I pondered this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325