[PATCH] D95403: [clang-tidy][analyzer][WIP] Clang-tidy reverse integration into Static Analyzer.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: vsavchenko, xazax.hun, martong, Szelethus, baloghadamsoftware, Charusso, steakhal, gribozavr2, alexfh, aaron.ballman.
Herald added subscribers: dang, ASDenysPetrov, phosek, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, mgorny.
Herald added a reviewer: jansvoboda11.
NoQ requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

This patch allows building a clang binary that has clang-tidy checks embedded into the static analyzer. I discussed these plans briefly in https://lists.llvm.org/pipermail/cfe-dev/2020-October/067002.html.

WIP because i haven't handled any build-time settings yet. This new facility //will be// under a CMake flag and most likely off by default (unless everybody suddenly loves this option, but even then, I still have to turn it into an option). I'll make sure to implement (and hopefully test) a reasonable behavior for all various combinations of CMake flags (eg., clang-tidy enabled/disabled, static analyzer enabled/disabled, static-analyzer-into-clang-tidy integration enabled/disabled, etc.).

This patch introduces a frontend flag `-analyzer-tidy-checker=...` that accepts clang-tidy checker strings (eg., `-analyzer-tidy-checker=bugprone-*,-cert-*,cert-env33-c`). As long as at least one such flag is supplied, `ClangTidyContext` is instantiated with the given checker list and a clang-tidy AST consumer gets multiplexed with `AnalysisConsumer` so that to run clang-tidy checkers. Diagnostics from these checks are pumped into a `PathDiagnosticConverterDiagnosticConsumer` (D94476 <https://reviews.llvm.org/D94476>) and displayed similarly to static analyzer diagnostics, eg. as HTML or Plist reports:

F15183921: Screen Shot 2021-01-25 at 2.59.30 PM.png <https://reviews.llvm.org/F15183921>

This patch suggests enabling a few clang-tidy checks by default in Clang Driver (under the --analyze flag, obviously, and the new reverse integration should be enabled). Requirements for enabling a clang-tidy check by default would be similar to requirements of enabling a static analyzer check by default (finds real bugs by design, understandable diagnostics, etc.). Additionally, on-by-default clang-tidy checks should have static analyzer bug categories assigned to them (eg., "Logic error", "Memory error", etc.) in the diagnostic converter (a relatively convenient way to do so is provided).

If bug categories aren't assigned, the default bug category for clang-tidy check `x-y-z` is `Clang-Tidy [x]` and the default bug type is `Clang-Tidy [x-y-z]`. This looks pretty ok in scan-build's index.html:

F15183900: Screen Shot 2021-01-25 at 2.56.47 PM.png <https://reviews.llvm.org/F15183900>

That said, we should probably provide an option to disable bug category conversion and use it whenever an arbitrary clang-tidy config is expected. As usual, we're optimizing for a reasonable default rather than for configurability.

I've considered a different approach to enabling/disabling clang-tidy checks (but didn't implement it in this patch) that's more static analyzer-like: `-analyzer-checker tidy.bugprone.assert-side-effect` etc. Here we treat `x` in `x-y-z` as a static analyzer package, hence the dot. This approach is less flexible because it doesn't allow arbitrary wildcards, so we still have to have the current approach but we should probably implement both. One thing to think about here would be to figure out whether each individual off-by-default clang-tidy check is treated as "alpha" ("we're not supporting it, don't use it") or opt-in ("we're supporting it but it's still of by default"). Probably we could additionally separate them into `alpha.tidy.*` and `optin.tidy.*`.

Clang-tidy check-specific options are not implemented yet. Again, we could provide a separate frontend flag, or we could interface them through `AnalyzerOptions` like this: `-analyzer-config tidy.bugprone.assert-side-effect:AssertMacros=assert,Assert,ASSERT`. And, again, we should probably do both.


https://reviews.llvm.org/D95403

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D95403.319125.patch
Type: text/x-patch
Size: 19413 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210125/c3777de3/attachment-0001.bin>