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

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Artem, could you set the repository to `rG LLVM Github Monorepo` when uploading patches as mentioned in https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface ? This way you'll allow pre-merge checks to run.

In D95403#2534714 <https://reviews.llvm.org/D95403#2534714>, @aaron.ballman wrote:

>> This patch introduces a frontend flag -analyzer-tidy-checker=...
>
> FWIW, the usual clang-tidy nomenclature is to call these "checks" rather than "checkers", so it might make sense to expose `-analyzer-tidy-checks=...` to be analogous to `clang-tidy -checks=...`

I vaguely remember discussing (most probably, with @gribozavr2) this difference and coming to a conclusion that there's no strong reason to keep the nomenclature difference between the static analyzer and clang-tidy. This didn't go much farther beyond creating the clang-tools-extra/test/clang-tidy/checkers/ directory, but we could revisit this, since we're looking into more interaction between the tools.



================
Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:54-60
+#include "../../clang-tools-extra/clang-tidy/ClangTidyCheck.h"
+#include "../../clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h"
+#include "../../clang-tools-extra/clang-tidy/ClangTidyForceLinker.h"
+#include "../../clang-tools-extra/clang-tidy/ClangTidyModule.h"
+#include "../../clang-tools-extra/clang-tidy/ClangTidyModuleRegistry.h"
+#include "../../clang-tools-extra/clang-tidy/ClangTidyOptions.h"
+#include "../../clang-tools-extra/clang-tidy/ClangTidyProfiling.h"
----------------
NoQ wrote:
> alexfh wrote:
> > Isn't this a layering violation, since clang-tidy depends on clangStaticAnalyzerCore and clangStaticAnalyzerFrontend?
> Yes, absolutely.
> 
> That said, the only purpose of clang-tidy's dependency on libStaticAnalyzer* is integration of static analyzer into clang tidy which is definitely not something we want to enable when we're baking clang-tidy back into clang. It never makes sense to run static analyzer through clang-tidy integration into static analyzer.
> 
> So ideally these two dependencies are temporally separated. I could make these dependencies mutually exclusive by making the upcoming option of baking clang-tidy into clang explicitly incompatible with `CLANG_TIDY_ENABLE_STATIC_ANALYZER`.
> 
> But if we want to support building both clang-tidy with static analyzer and static analyzer with clang-tidy from the same sources into the same build directory, that'll probably involve either building two variants of clang-tidy (one with static analyzer for standalone clang-tidy binary and one without to use inside clang binary only) or two variants of static analyzer (one with clang-tidy for the clang binary and one without to use inside clang-tidy binary only).
> 
> Do you have any preference on how should i untangle this?
I hope we could break the dependency cycle without "flipping" the direction of dependencies based on a CMake switch. This way it would be easier to replicate the dependency structure in other BUILD systems.

Now the dependencies look very roughly like this:
```
clangStaticAnalyzerCore:
clangStaticAnalyzerCheckers: clangStaticAnalyzerCore
clangStaticAnalyzerFrontend: clangStaticAnalyzerCheckers clangStaticAnalyzerCore
clangFrontendTool: clangStaticAnalyzerFrontend

clangTidy: clangStaticAnalyzerCore clangStaticAnalyzerFrontend
clang-tidy check libraries gathered under $ALL_CLANG_TIDY_CHECKS
clangTidyMain: clangTidy
clang-tidy: clangTidyMain clangTidy $ALL_CLANG_TIDY_CHECKS
```

Untangled version could look like this:
```
clangStaticAnalyzerCore:
clangStaticAnalyzerCheckers: clangStaticAnalyzerCore
clangStaticAnalyzerFrontend: clangStaticAnalyzerCheckers clangStaticAnalyzerCore clangTidyInterface
clangStaticAnalyzerFrontendWithClangTidy: clangTidyImpl $ALL_CLANG_TIDY_CHECKS
clangFrontendTool: clangStaticAnalyzerFrontendWithClangTidy

clangTidyInterface: (with ClangTidy.h, ClangTidyOptions.h, ClangTidyDiagnosticConsumer.h and ClangTidyProfiling.h, for example)
clangTidyImpl: clangTidyInterface clangStaticAnalyzerCore clangStaticAnalyzerFrontend
clang-tidy check libraries gathered under $ALL_CLANG_TIDY_CHECKS
clangTidyMain: clangTidyImpl
clang-tidy: clangTidyMain clangTidyImpl $ALL_CLANG_TIDY_CHECKS
```

What do you think?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95403