kadircet added a comment.
i can't think of a proper way to test this out either. unless we somehow let
slow-tidy-check list to be configurable, so probably fine to make sure it works
locally and hope that new people introducing tidy checks do complain.
================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:518
+
+ if (F.SlowChecks.has_value())
+ Out.Apply.push_back([V = **F.SlowChecks](const Params &, Config &C) {
----------------
nit: drop `has_value`?
================
Comment at: clang-tools-extra/clangd/TidyProvider.cpp:286
bool isRegisteredTidyCheck(llvm::StringRef Check) {
assert(!Check.empty());
----------------
should we update callers here to also emit a warning when they're turning on a
slow check (and possibly mention SlowChecks override?) this might as well be
our way of testing this to some extent. we'd still rely on a certain check-name
always being part of the list (and pick a new element whenever we're updating
the list), but at least we wouldn't rely on semantics of the check (i.e. also
have a test case that'd trigger the warning).
================
Comment at: clang-tools-extra/clangd/tool/Check.cpp:468
+ // is counterproductive!
+ if (CheckTidyTime.getNumOccurrences())
+ F.Diagnostics.ClangTidy.SlowChecks = true;
----------------
sammccall wrote:
> 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.
i think it's better to always have the provider, in case we decide to override
more options later on (it'd be nice if we didn't come up with a new provider
for every option we override). but seems fine either way for now.
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
