[PATCH] D50045: [clangd] Report diagnostics even if WantDiags::No AST was reused

Eric Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 31 03:25:10 PDT 2018

ioeric added a comment.

Overall LG, just a suggestion to make the code a bit easier to follow.

Comment at: clangd/TUScheduler.cpp:379
+    if (DiagsWereReported) {
       // Take a shortcut and don't build the AST if neither the inputs nor the
The implicit condition here is that we can reuse AST and diagnostics were not reported. I think we can be more explicit here by moving this early return into the `CanReuseAST` if branch. By doing this, we could also get rid of `PrevDiagsWereReported`. An additional state variable seems to have made the flow less clear.

Comment at: clangd/TUScheduler.cpp:405
     // spam us with updates.
-    if (WantDiags != WantDiagnostics::No && AST)
-      OnUpdated(AST->getDiagnostics());
+    if (WantDiags != WantDiagnostics::No && *AST) {
+      OnUpdated((*AST)->getDiagnostics());
This was preexisting, but it might worth adding a comment explaining when `*AST` would be false.

  rCTE Clang Tools Extra


More information about the cfe-commits mailing list