Skip to content

Commit

Permalink
[clangd] Fix broken assertion
Browse files Browse the repository at this point in the history
Summary:
This assertion was bad. It will show up once we start running preamble
thread async. Think about the following case:

- Update 1
    builds a preamble, and an AST. Caches the AST.
- Update 2
    Invalidates the cache, preamble hasn't changed.
- Update 3
    Invalidates the cache, preamble hasn't changed
- Read
    builds AST using preamble v1, and caches it.
    preamble for v2 gets build, cache isn't invalidated since preamble is same.
    generateDiags tries to reuse cached AST but latest version is 3 not 2, so
    assertion fails.

Reviewers: sammccall

Subscribers: ilya-biryukov, javed.absar, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D77664
  • Loading branch information
kadircet committed Apr 8, 2020
1 parent aa03486 commit 130dbf6
Showing 1 changed file with 1 addition and 3 deletions.
4 changes: 1 addition & 3 deletions clang-tools-extra/clangd/TUScheduler.cpp
Expand Up @@ -795,7 +795,7 @@ void ASTWorker::generateDiagnostics(
// FIXME: It might be better to not reuse this AST. That way queued AST builds
// won't be required for diags.
llvm::Optional<std::unique_ptr<ParsedAST>> AST = IdleASTs.take(this);
if (!AST) {
if (!AST || !InputsAreLatest) {
auto RebuildStartTime = DebouncePolicy::clock::now();
llvm::Optional<ParsedAST> NewAST = buildAST(
FileName, std::move(Invocation), CIDiags, Inputs, LatestPreamble);
Expand All @@ -817,8 +817,6 @@ void ASTWorker::generateDiagnostics(
});
AST = NewAST ? std::make_unique<ParsedAST>(std::move(*NewAST)) : nullptr;
} else {
assert(InputsAreLatest && !RanASTCallback &&
"forgot to invalidate cached ast?");
log("Skipping rebuild of the AST for {0}, inputs are the same.", FileName);
Status.update([](TUStatus &Status) {
Status.Details.ReuseAST = true;
Expand Down

0 comments on commit 130dbf6

Please sign in to comment.