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

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 31 04:41:12 PDT 2018

ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added inline comments.

Comment at: clangd/TUScheduler.cpp:379
+    if (DiagsWereReported) {
       // Take a shortcut and don't build the AST if neither the inputs nor the
ilya-biryukov wrote:
> ioeric wrote:
> > 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.
> The problem with that would be that the value of `DiagsWereReported` would be stale for ~40 lines between the assignment to `FileInputs` and this point.
> So I'm not sure about this trade-off. WDYT?
Update after chatting offline: we agreed to keep the `PrevDiagsWereReported` and move the early return into the original if branch to make the dependencies between DiagsWereReported and CanReuseAST more explicit. This comment should be done now.

  rCTE Clang Tools Extra


More information about the cfe-commits mailing list