[clang-tools-extra] 130dbf6 - [clangd] Fix broken assertion

Kadir Cetinkaya via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 8 00:58:16 PDT 2020


Author: Kadir Cetinkaya
Date: 2020-04-08T09:57:55+02:00
New Revision: 130dbf63ff12271be9451e259447c5fab768dce6

URL: https://github.com/llvm/llvm-project/commit/130dbf63ff12271be9451e259447c5fab768dce6
DIFF: https://github.com/llvm/llvm-project/commit/130dbf63ff12271be9451e259447c5fab768dce6.diff

LOG: [clangd] Fix broken assertion

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/TUScheduler.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp
index be4d52e7d881..46ec8e91d5a1 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -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);
@@ -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;


        


More information about the cfe-commits mailing list