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.
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
