[PATCH] D63986: [clangd] Also cache failures while indexing

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks!



================
Comment at: clang-tools-extra/clangd/index/Background.cpp:140
+
+void storeSlabs(BackgroundIndexStorage *IndexStorage, PathRef Identifier,
+                SymbolSlab *Syms, RefSlab *Refs, RelationSlab *Rels,
----------------
(I'd suggest inlining this again as it's fairly clear straight-line code, but up to you)


================
Comment at: clang-tools-extra/clangd/index/Background.cpp:313
+    // we don't even know what absolute path they should fall in.
+    if (HadErrors && !IGN.IsTU)
+      continue;
----------------
Maybe add a FIXME that we should store other contents too, but only if the current shard contents are missing or also had errors.




================
Comment at: clang-tools-extra/clangd/index/Background.cpp:500
+  if (HadErrors)
+    return llvm::createStringError(
+        llvm::inconvertibleErrorCode(),
----------------
why do we want to skip (maybe) rebuilding the index in this case?


================
Comment at: clang-tools-extra/clangd/index/Background.cpp:500
+  if (HadErrors)
+    return llvm::createStringError(
+        llvm::inconvertibleErrorCode(),
----------------
sammccall wrote:
> why do we want to skip (maybe) rebuilding the index in this case?
I think this is now a "warning" rather than an error, maybe log something like "failed to compile {Cmd.Filename}, index may be incomplete"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63986/new/

https://reviews.llvm.org/D63986