[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 added a comment. Whoops, probably should've updated the summary before pushing, ah well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 ___ cfe-commits

[PATCH] D91029: [clangd] Implement clang-tidy options from config

This revision was automatically updated to reflect the committed changes. Closed by commit rG73fdd998701c: [clangd] Implement clang-tidy options from config (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://rev

[PATCH] D91029: [clangd] Implement clang-tidy options from config

sammccall accepted this revision. sammccall added a comment. Hooray, ship it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 ___ cfe-commits mailing list cfe-com

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 307642. njames93 added a comment. Removed CWD references. Added some fixme comments about function_ref<->unique_function and null values. Added asserts in provideClangTidyFiles ensuring path is absolute. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 added inline comments. Comment at: clang-tools-extra/clangd/TidyProvider.h:25 +/*Filename=*/llvm::StringRef, +/*CWD*/ PathRef) const>; + sammccall wrote: > sa

[PATCH] D91029: [clangd] Implement clang-tidy options from config

sammccall added inline comments. Comment at: clang-tools-extra/clangd/TidyProvider.h:25 +/*Filename=*/llvm::StringRef, +/*CWD*/ PathRef) const>; + sammccall wrote: > n

[PATCH] D91029: [clangd] Implement clang-tidy options from config

sammccall added inline comments. Comment at: clang-tools-extra/clangd/TidyProvider.h:25 +/*Filename=*/llvm::StringRef, +/*CWD*/ PathRef) const>; + njames93 wrote: > sa

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 added inline comments. Comment at: clang-tools-extra/clangd/TidyProvider.h:25 +/*Filename=*/llvm::StringRef, +/*CWD*/ PathRef) const>; + sammccall wrote: > CW

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 307615. njames93 added a comment. Removed duplicated logging of options. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-extra/clangd/CMakeLists.t

[PATCH] D91029: [clangd] Implement clang-tidy options from config

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks, this LG! (The change to global makes sense but doesn't block us I think) Comment at: clang-tools-extra/clangd/TidyProvider.h:25 +

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 marked 4 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clangd/TidyProvider.h:25 + /*Priority=*/unsigned &, + /*CWD*/ PathRef); +} // namespace detail

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 307565. njames93 added a comment. Use DefaultOptionsProvider for initializing ClangTidyContext. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-ex

[PATCH] D91029: [clangd] Implement clang-tidy options from config

sammccall added a comment. OK, I would love to get this landed. I think the simplest thing is to compute the config eagerly and load it in with DefaultOptionsProvider as you mentioned. I might fiddle with checking the filename afterwards, but I don't think this is blocking. Similarly I want to

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 307557. njames93 added a comment. . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/clangd/ClangdS

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 307549. njames93 added a comment. Tweak disableUnusableChecks to take an ArrayRef over a vector. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-e

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 added inline comments. Comment at: clang-tools-extra/clangd/TidyProvider.cpp:194 +for (auto &Option : llvm::reverse(OptionStack)) + Opts.mergeWith(Option, ++Order); + }; sammccall wrote: > njames93 wrote: > > sammccall wrote: > > > This order b

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 307478. njames93 added a comment. Getting closer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 marked 6 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clangd/ParsedAST.cpp:303 E.instantiate()->addCheckFactories(CTFactories); -CTContext.emplace(std::make_unique( -tidy::ClangTidyGlobalOptions(), Inputs.Opts.

[PATCH] D91029: [clangd] Implement clang-tidy options from config

sammccall added inline comments. Comment at: clang-tools-extra/clangd/ParsedAST.cpp:303 E.instantiate()->addCheckFactories(CTFactories); -CTContext.emplace(std::make_unique( -tidy::ClangTidyGlobalOptions(), Inputs.Opts.ClangTidyOpts)); +CTContext.emplace(as

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 307416. njames93 marked 5 inline comments as done. njames93 added a comment. Address (most of the) comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: c

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 marked 12 inline comments as done and an inline comment as not done. njames93 added inline comments. Comment at: clang-tools-extra/clangd/ParsedAST.cpp:241 +/// Empty clang tidy provider, using this as a provider will disable clang-tidy. +static void emptyTidyProvider(t

[PATCH] D91029: [clangd] Implement clang-tidy options from config

sammccall added a comment. OK this looks really great, thanks so much for persisting with this. Comments are mostly simple nits/simplifications, with the exception of `Order` which is a... slightly trickier simplification, but seems worth doing. Tomorrow I'll adapt our internal clang-tidy confi

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 307349. njames93 added a comment. Small tweaks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/cla

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 307229. njames93 added a comment. Added PathRef to TidyProvider, this handles finding clang-tidy files when the compile command directory isn't the project root. Use elog for reporting errors instead of `llvm::errs` - It was an artefact from the code in Cla

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 307213. njames93 added a comment. Fix assertion in provideDefaultChecks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-extra/clangd/CMakeLists.tx

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 306850. njames93 added a comment. Fix ChecksToDisable to actually disable the checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-extra/clangd

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 marked 2 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clangd/TidyProvider.h:20 +/// The base class of all clang-tidy config providers for clangd. +class TidyProvider { +public: sammccall wrote: > we're using inhe

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 306796. njames93 marked 7 inline comments as done. njames93 added a comment. Messed up branches, fixed that Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: cl

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 306794. njames93 marked an inline comment as done. njames93 added a comment. Reworked everything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-

[PATCH] D91029: [clangd] Implement clang-tidy options from config

sammccall added a comment. Thanks, this looks massively simpler. It seems clear that we get config by applying a sequence of strategies in order, and these strategies (e.g. .clang-tidy files, config, disabled checks) are mostly independent. So I have a suggestion for expressing this composition

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 304940. njames93 added a comment. - Small tweaks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/c

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 added a comment. Thanks for the comments, I agree it was a little too much. Likely due in part to how I first tried to mirror the interface of ClangTidyOptionsProvider. Comment at: clang-tools-extra/clangd/ParsedAST.cpp:250 if (Preamble && Preamble->StatCache) -

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 304780. njames93 marked 4 inline comments as done. njames93 added a comment. Reworked large chunks of this: - Renamed ClangdTidyProvider->TidyProvider. - Removed the obselete interface mirroring ClangTidyOptionsProvider, it wasn't needed. - Incorporated the

[PATCH] D91029: [clangd] Implement clang-tidy options from config

sammccall added a comment. Thanks for working on this! I think the scope of this patch is probably bigger than we need, at least initially: - it adds a new TidyProvider system with a lot of flexibility. But our config needs are quite static. The only flexibility we need initially is being able

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 303774. njames93 added a comment. - Fix compile error for gcc 5.3. - Add back logging configuration when creating CTContext. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D9

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 303723. njames93 added a comment. Fix last compile error, check-clangd ran without a hitch. Testing the binary in another project seems to work ok. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ htt

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 303721. njames93 added a comment. Fix compile errors. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-ex

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 303720. njames93 added a comment. Fix potential race when updating cache in `FileTidyProvider`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-ex

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 created this revision. njames93 added reviewers: sammccall, kadircet. Herald added subscribers: cfe-commits, usaxena95, arphaman, mgorny. Herald added a project: clang. njames93 requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Added some new ClangTidyOp