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.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50045
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
