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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Moved the clang-tidy changes to https://reviews.llvm.org/D54579.
Sorry for mixing everything up!



================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:518
 void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(translationUnitDecl().bind("top"), this);
+  Finder->addMatcher(matchOnce(&MatchedOnce), this);
 
----------------
hokein wrote:
> maybe add a comment  describing we are trying to find the top level decl?
We're not, we're trying to match any node (but only one). Extended the comment on the matcher.


================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:566
     replaceCompoundReturnWithCondition(Result, Compound, true);
-  else if (const auto TU = Result.Nodes.getNodeAs<Decl>("top"))
-    Visitor(this, Result).TraverseDecl(const_cast<Decl*>(TU));
+  else // MatchOnce matcher
+    Visitor(this, Result).TraverseAST(*Result.Context);
----------------
hokein wrote:
> add an `assert (MatchOnce)`?
That doesn't compile, I'm not sure what you want here.


================
Comment at: clangd/ClangdUnit.cpp:158
+  // Clang-tidy has some limitiations to ensure reasonable performance:
+  //  - checks don't see all preprocessor events in the preamble
+  //  - matchers run only over the main-file top-level decls (and can't see
----------------
hokein wrote:
> just want to make sure -- do we receive all preprocessor events in the main file when we use preamble? We have a few checks that generate `#include` insertion as part of FixIt.
> 
> `TestTU` doesn't use preamble, it would be nice to have a test running on a main file with a real preamble, but this can be a follow-up I think.
Nope, you're right: `#include` from the preamble are not replayed. Can't fix this now, added a FIXME. (Neither of the hardcoded checks care).

TestTU does use a preamble now :-)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54204