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

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 1 09:10:03 PDT 2019


kadircet added inline comments.


================
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;
----------------
sammccall wrote:
> Maybe add a FIXME that we should store other contents too, but only if the current shard contents are missing or also had errors.
> 
> 
I agree with storing contents if they are missing, but does it make sense to replace one broken shard information with other?


================
Comment at: clang-tools-extra/clangd/index/Background.cpp:500
+  if (HadErrors)
+    return llvm::createStringError(
+        llvm::inconvertibleErrorCode(),
----------------
sammccall wrote:
> 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"
actually, we don't want to skip. I had written that part with the old assumption; that is, "we don't get any symbol info when compilation fails".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63986





More information about the cfe-commits mailing list