sammccall added inline comments.
================ Comment at: clang-tools-extra/clangd/ParsedAST.cpp:487 + } + CTFactories = std::move(FastFactories); + } ---------------- njames93 wrote: > Not exactly related but surely both check factories could be made into static > variables and then just choose the factory based on the config. `createChecks` is nonconst, so this requires some fast-and-loose assumptions about nonconst operations being threadsafe. (In principle you're right, but I think this is mostly an argument that ClangTidyCheckFactories and its adjacent APIs could be improved, which isn't something I can get sidetracked by at the moment) ================ Comment at: clang-tools-extra/clangd/tool/Check.cpp:468 + // is counterproductive! + if (CheckTidyTime.getNumOccurrences()) + F.Diagnostics.ClangTidy.SlowChecks = true; ---------------- njames93 wrote: > How about changing this provide to always enable slow checks, but only use > the provider if the flag is passed? This doesn't seem like it would simplify the code, but it does mean that if --check wants to override other config options then we'd need to add a second provider. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138505/new/ https://reviews.llvm.org/D138505 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
