LegalizeAdulthood accepted this revision. LegalizeAdulthood added a comment. This revision is now accepted and ready to land.
Still LGTM and my comments are just thought experiments, not must haves before pushing. ================ Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:165 : DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)), - Profile(false), + CurrentlyProcessing(nullptr), Profile(false), AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers) { ---------------- Does the LLVM style guide say anything about preferring member initializers over initializing constant expressions? ================ Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:239 + const ast_matchers::BoundNodes &Result) const { + if (CurrentlyProcessing) + CurrentlyProcessing->onProcessingCheckStart(CheckName, Result); ---------------- We repeat this `if (CurrentlyProcessing)` fragment 3 times... Maybe initializing to a Null Object would be better? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118520/new/ https://reviews.llvm.org/D118520 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits