[cfe-dev] RFC: Pumping Clang-Tidy warnings through the Static Analyzer's BugReporter.

[Dmitri Gribenko via cfe-dev]

Hi Artem,

On Wed, Aug 14, 2019 at 5:00 AM Artem Dergachev <noqnoqneo at gmail.com> wrote:
>
> As i've been vaguely hinting on EuroLLVM, i plan to invest some time in
> decreasing the gap between Clang-Tidy users and Static Analyzer users
> and make sure it's always possible for our users to take the best of
> both worlds. In particular, i'd like to make Clang-Tidy easily
> integratable into all UIs that already integrate the Static Analyzer.

I like the idea of integrating the two tools more closely. From the
user's point of view, I think, ClangTidy and Clang Static Analyzer are
doing more or less the same thing, but somehow have completely
different workflows and integrations.

> Ideally i'd love to do the same to Clang-Tidy that Clang-Tidy does to
> us: make the Static Analyzer load it as a library and expose Clang-Tidy
> checks as its own, maybe in a separate package. This is harder to do
> though, because Clang-Tidy lives in a separate repo and also it's a hard
> sell to build it into the Clang binary. I'm open to suggestions here: we
> can keep such integration under an off-by-default CMake flag (which
> requires enabling compilation of clang-tools-extra) or we may even use
> it as a run-time plugin (i mean, clang plugins are super wonky, but this
> time it's actually fairly easy to avoid version conflicts, as they all
> get compiled from the same sources simultaneously).

I would like to suggest something different: move Clang Static
Analyzer to clang-tools-extra. Build it either as a separate binary or
compile it into the clang-tidy binary. Then let `clang -analyze`
delegate to that binary for backwards compatibility.

Clang Static Analyzer is being developed in the clang repository for
historical reasons -- we didn't have clang-tools-extra at the time the
project was started. The "non-core" nature of Clang Static Analyzer is
also validated by the fact that it is an optional part of Clang -- we
have a CMake flag `CLANG_ENABLE_STATIC_ANALYZER`.

Long-term, my preference would be to have all static analysis tooling
(Clang Static Analyzer, ClangTidy, any prospective dataflow-based
analysis) to have a consistent UI -- including invocation, integration
with build systems, configuration strategy etc. While the
implementation of these tools is very different, I think the audience
is largely the same.

> But regardless of how far do i end up going with such integration, first
> thing first: i'll anyway need to teach Clang-Tidy how to route its
> diagnostics through our diagnostic engine. This is also the only thing
> that's absolutely necessary; the rest can always be hacked up on the UI
> side.
>
> It's a great question why didn't we extend Clang's DiagnosticEngine to
> begin with, but instead made our own facility. I don't know the answer
> to that; this happened way before i even learned what an AST is :) We
> generally needed a much more sophisticated diagnostic engine because our
> reports are much more complicated. While we could try to unify these
> diagnostic engines completely, i don't think it's worth it.
>
> So i think it's more feasible to make Clang-Tidy's diag() method return
> a proxy object that mimics the DiagnosticBuilder interface but may also
> pump the diagnostics to an instance of the Static Analyzer's
> BugReporter. I hope this would avoid API breakages for clang-tidy checks.

I'm very concerned about introducing lookalike APIs that don't behave
exactly the same way as original ones. In LLVM, we are generally not
concerned with backwards compatibility for C++ APIs. Instead, we
should be looking to keep our code maintainable in the long run.

With that in mind, my suggestion is to refactor ClangTidy to use
BugReporter instead of DiagnosticBuilder.

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/