ilya-biryukov added inline comments.
================
Comment at: clangd/TUScheduler.cpp:424
if (*AST) {
OnUpdated((*AST)->getDiagnostics());
trace::Span Span("Running main AST callback");
----------------
sammccall wrote:
> as discussed offline, this doesn't guarantee we're not going to send
> OnUpdated calls for destroyed FileData objects. Intuitively because
> ShuttingDown may be set e.g. while building the AST, more subtly because
> atomic only prevents reordering with respect to other atomic operations.
>
> I believe we need to hold a lock:
> ```
> {
> mutex_lock Lock(SendDiagnosticsMu);
> if (SendDiagnostics)
> OnUpdated(...);
> }
> ```
> and set SendDiagnostics to false under the same lock.
Done. I've made the change before looking at the comment, so using
`ReportDiagnostics` name instead of `SendDiagnostics`. Happy to change if you
like the latter more.
================
Comment at: clangd/TUScheduler.cpp:426
trace::Span Span("Running main AST callback");
Callbacks.onMainAST(FileName, **AST);
DiagsWereReported = true;
----------------
sammccall wrote:
> This patch prevents the AST from being indexed after removal - for the same
> reasons, right?
>
> But we don't prevent onPreambleAST from being called...
> Not sure if it makes sense to fix this, but deserves at least a comment I
> think.
Added a comment.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54829
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
