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

[Artem Dergachev via cfe-dev]

Ugh, i just realized this.

Clang-Tidy doesn't really need BugReporter; all it needs is 
PathDiagnosticConsumer. (the word "Path" is rudimentary)

It should be perfectly comfortable to build PathDiagnostics by hand - it 
maps nicely to how you already build your warnings and notes. There's no 
need to force you to do the job of packing your notes into an auxiliary 
BugReport object and then use BugReporter to unwrap it back into a 
PathDiagnostic. It makes a huge lot of sense for path-sensitive reports 
to de-duplicate the work of turning a report into a PathDiagnostic, but 
it doesn't really mean much for path-insensitive reports.

Change of plans, i guess.

On 8/14/19 12:23 AM, Dmitri Gribenko wrote:
> 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
>