[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

I think this is in a good shape as initial patch!



================
Comment at: clangd/ClangdUnit.cpp:168
+    // The placeholder check here does not use hasAncestor() so is unaffected.
+    CTOpts.Checks = "bugprone-sizeof-expression";
+    CTContext.emplace(llvm::make_unique<tidy::DefaultOptionsProvider>(
----------------
It seems we also support clang-tidy checks that analyze preprocessor-dependent properties. I think we can add a clang-tidy check to make sure `PPCallback` actually work, `google-readability-todo` is a good candidate. 

Ah just realized limitations (truncated `PPCallback` events) you wrote in the patch description, maybe mention them in the source comment, so that we won't forget in the future when reading the code.


================
Comment at: clangd/ClangdUnit.cpp:175
+    CTFactories.createChecks(CTContext.getPointer(), CTChecks);
+    for (const auto &Check : CTChecks) {
+      Check->registerPPCallbacks(*Clang);
----------------
Maybe add the check names to the `Trace`?


================
Comment at: clangd/ClangdUnit.cpp:468
+      X##ModuleAnchorSource
+LINK_TIDY_MODULE(CERT);
+LINK_TIDY_MODULE(Abseil);
----------------
I'm curious how much does clangd binary size get increased.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54204