[PATCH] D77177: [Analyzer][WebKit] RefCntblBaseVirtualDtorChecker & shared utils

Thu May 21 13:32:54 PDT 2020

NoQ added a comment.

> Reverted in 1108f5c737dbdab0277874a7e5b237491839c43a <https://reviews.llvm.org/rG1108f5c737dbdab0277874a7e5b237491839c43a> for now.

Uh-oh. Thank you for reverting!

> Project-specific checks like this usually go in clang-tidy, not in the static analyzer (which ships with the compiler binary).

There are a lot more considerations to be taken into account when choosing where to put the check. The static analyzer already contains a lot of project-specific and framework-specific checks which will never be implemented in clang-tidy because they're path-sensitive. Also these tools are not a drop-in replacement of each other: you cannot use clang-tidy in all the places where you can use the static analyzer (the opposite is partially true due to libStaticAnalyzer integration into clang-tidy but UI degrades dramatically when such integration is used) therefore many authors simply do not have a choice where to put the check (i'm slowly working on improving this situation so that most path-insensitive checks could indeed be migrated into clang-tidy but we're not there yet). So as of today we have no choice but to let our contributors decide freely where they want their checks to be.

================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:1632
+
+def WebKitRefCntblBaseVirtualDtorChecker : Checker<"WebKitRefCntblBaseVirtualDtor">,
+  HelpText<"Check for any ref-countable base class having virtual destructor.">,
----------------
While we're at it, maybe remove "WebKit" from the checker name, given that it duplicates the package name?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77177