[PATCH] D126859: [clangd] Validate clang-tidy CheckOptions in clangd config

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 22 02:52:34 PDT 2022


njames93 added a comment.

In D126859#3599340 <https://reviews.llvm.org/D126859#3599340>, @kadircet wrote:

> I also agree with the typo correction verdict. In theory there'll be two cases:
>
> - typo correction helps,  in which case it'll be obvious from the warning itself.
> - typo correction doesn't help, because the option doesn't exist at all, we'll be just showing a random option. I don't think how this'll be helpful.
>
> So I don't think this is providing much of a value compared to the extra logic.

The correction only shows near misses, so it wouldn't show completely random options.
However it's only really there to prevent you having to look at the documentation for the check, so I'm happy to remove it.



================
Comment at: clang-tools-extra/clangd/TidyProvider.cpp:291
+  static void *call() {
+    return new tidy::NamesAndOptions(tidy::getAllChecksAndOptions(false));
+  }
----------------
kadircet wrote:
> sammccall wrote:
> > it seems strange that clang-tidy provides this API to query what checks are linked in, but it constructs an expensive object every time rather than just creating a static one once and returning a reference to it. (i.e. the memoization is on the caller side)
> > Should we fix that API instead?
> +1
The clang-tidy API is still a WIP. but it doesn't really make sense to memoize it on the tidy side as its s function that is only ever going to be called once in clang-tidy.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126859/new/

https://reviews.llvm.org/D126859



More information about the cfe-commits mailing list