[PATCH] D54865: [clangd] Auto-index watches global CDB for changes.

Mon Nov 26 01:43:13 PST 2018

sammccall marked 3 inline comments as done.
sammccall added inline comments.

================
Comment at: clangd/index/Background.cpp:130
+  if (auto Cmd = CDB.getCompileCommand(File, &Project)) {
+    auto *Storage = IndexStorageFactory(Project.SourceRoot);
+    enqueueTask(Bind(
----------------
kadircet wrote:
> In case of an OverlayCDB with some commands set through LSP configuration, we might end up `Project.SourceRoot` being an empty string, which will result in creating the index storage directory in cwd. I believe we wouldn't want that, but not sure if it should be handled by `BackgroundIndex` or `DiskBackedIndexStorage`. WDYT?
You're right. To a first approximation, I think that the right behavior is to keep indexing in memory and just not persist the shards.
The storage seems like the right layer because:
 - the factory makes it easy to encapsulate this special case
 - we may want to change the strategy in future (e.g. write index files into a central location if the builddir is unavailable).

The downside I see is by encapsulating it we can't do things like "downgrade priority of indexing if storage isn't available", but we can find ways around that if we really want to.

I've added a null storage to this patch which is used if the CDB dir is empty.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54865