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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
