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

[Dmitri Gribenko via cfe-dev]

On Wed, Aug 14, 2019 at 2:46 PM Christian Gagneraud <chgans at gmail.com> wrote:
>
> On Wed, 14 Aug 2019 at 14:39, Dmitri Gribenko via cfe-dev
> <cfe-dev at lists.llvm.org> wrote:
> >
> > On Wed, Aug 14, 2019 at 2:35 PM Aaron Ballman <aaron at aaronballman.com> wrote:
> > >
> > > On Wed, Aug 14, 2019 at 8:30 AM Dmitri Gribenko <gribozavr at gmail.com> wrote:
> > > >
> > > > On Wed, Aug 14, 2019 at 2:16 PM Aaron Ballman <aaron at aaronballman.com> wrote:
> > > > >
> > > > > On Wed, Aug 14, 2019 at 3:24 AM Dmitri Gribenko via cfe-dev
> > > > > <cfe-dev at lists.llvm.org> 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.
> > > > >
> > > > > Strong +1 from me!
> > > > >
> > > > > > > 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.
> > > > >
> > > > > I am not keen on this approach. A stand-alone tool is harder for users
> > > > > to integrate into their workflows compared to the compiler itself,
> > > > > which is already (generally) a part of their existing workflow. It's
> > > > > far easier for a user to add a flag like -tidy to an existing clang
> > > > > execution than it is to insert clang-tidy into a complex build system.
> > > > > Personally, I would much rather see libClangTidy integrated into Clang
> > > > > and exposed via a driver flag, similar to how the static analyzer
> > > > > already works.
> > > >
> > > > Integrating by adding a flag to the build is only one possibility, and
> > > > it does not work for all users.
> > >
> > > I agree, my point was that a stand-alone tool doesn't work for all
> > > users either. For my needs, moving the static analyzer into clang-tidy
> > > makes things harder, not easier, whereas moving clang-tidy into the
> > > compiler proper makes things easier rather than harder.
> > >
> > > > Not everyone is willing to pay the
> > > > analysis slowdown during compilation (otherwise we would just expose
> > > > static analyzer and clang-tidy as -W flags in the compiler as the only
> > > > integration option).
> > >
> > > This is why analysis is controlled by a separate flag. I see no reason
> > > why clang-tidy checks would not behave similarly.
> >
> > Agreed, users who *can* integrate by analyzing during the build,
> > should be able to do so.
> >
> > > > Running a separate build just to analyze forces
> > > > the compiler to run code generation and produce object files and
> > > > binaries, which are not necessary for static analysis.
> > >
> > > I don't see how that follows; -fsyntax-only exists to support that
> > > sort of use case.
> >
> > The build system won't proceed with the build if .o files and code
> > generation binaries are not produced.
>
> Does it makes sense to keep support for compiler wrapper?
> The compilation database is a much better approach.

It depends whom we are optimizing for.

I think that generally, small projects will prefer direct compiler
integration (`clang -c -tidy -analyze`) or the compiler wrapper
approach.

Bigger projects with more complex build processes, a CI, pre-commit
testing etc. will probably prefer an approach based on the compilation
database.

If we want to cover everyone, I guess we have to maintain both approaches.

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>*/