[PATCH] D77664: [clangd] Fix broken assertion

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 7 11:23:21 PDT 2020


kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, javed.absar, ilya-biryukov.
Herald added a project: clang.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77664

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


Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -795,7 +795,7 @@
   // 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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D77664.255739.patch
Type: text/x-patch
Size: 651 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200407/1ee980ae/attachment.bin>


More information about the cfe-commits mailing list