[PATCH] D138505: [clangd] Don't run slow clang-tidy checks by default

[Sam McCall via cfe-commits]

Very late, & by email since phab is down.

Going to land this based on "LG" comment & offline discussion, happy to do
followups if needed.

On Mon, Nov 28, 2022 at 10:08 PM Kadir Cetinkaya via Phabricator <
reviews at reviews.llvm.org> wrote:

> kadircet added a comment.
>
> thanks LG, i'd like to hear how we're planning to let downstream users
> customise the list of fast checks. otherwise they have to run with `Loose`
> at all times.
> the easiest i can think of is, generating their own `fastchecks.inc`
> fragment and #include that in addition to clangd's default list. Any other
> ideas on this one?
>

This is the plan I'd suggest.
Having to modify source is annoying, but there's always loose mode too.


> ================
> Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:488
> +             llvm::formatv(
> +                 "Speed of clang-tidy check '{0}' is not known. "
> +                 "It will only run if ClangTidy.FastCheckFilter is Loose
> or None",
> ----------------
> nit: s/Speed/Latency (or Performance?)
>
Done

================

> Comment at: clang-tools-extra/clangd/TidyProvider.cpp:312
> +  };
> +  auto It = Fast.find(Check);
> +  if (It == Fast.end())
> ----------------
> nit: some c++17 magic if you want:
> ```
> if (auto It = Fast.find(Check); It != Fast.end())
>   return It->second;
> return llvm::None;
> ```
>
Done


> ================
> Comment at: clang-tools-extra/clangd/tool/Check.cpp:88
> +llvm::cl::opt<bool> CheckWarnings{
> +    "check-warnings",
> +    llvm::cl::desc("Print warnings as well as errors"),
> ----------------
> i think `print-warnings` is probably better than `check` we're not really
> checking anything.
>
There's only one namespace for all flags to clangd, "check-" is being used
as a namespacing prefix for those that control the behavior of --check
specifically.

I agree -print-warnings would read slightly better, but I think it's
important these flags are clearly distinguished/grouped.


> ================
> Comment at: clang-tools-extra/clangd/tool/Check.cpp:92
>
>  // Print (and count) the error-level diagnostics (warnings are ignored).
>  unsigned showErrors(llvm::ArrayRef<Diag> Diags) {
> ----------------
> update the comment (or maybe even drop it?)
>
Done.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20231020/21bb7c46/attachment-0001.html>