[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/index/Background.cpp:197
+        Cmd.CommandLine.push_back("-resource-dir=" + ResourceDir);
+        const std::string FileName = Cmd.Filename;
+        if (auto Error = index(std::move(Cmd), Storage))
----------------
use `llvm::StringRef`


================
Comment at: clangd/index/Background.cpp:259
+      // Skip if file is already up to date.
+      auto DigestIt = IndexedFileDigests.try_emplace(Path);
+      if (!DigestIt.second && DigestIt.first->second == Hash)
----------------
I now see that my previous comment about lock/unlock for each file was wrong in the first place.
It feels that's we'd better keep the `IndexedFileDigests`  and `IndexedSymbols` in sync.

Why don't we update the digest in the next loop that actually updates the symbols?


================
Comment at: clangd/index/Background.h:111
+    bool NeedsReIndexing;
+    Source(llvm::StringRef Path, bool NeedsReIndexing = true)
+        : Path(Path), NeedsReIndexing(NeedsReIndexing) {}
----------------
NIT: maybe remove the constructor? plain structs can easily be initialized with init lists and adding a constructor forbids per-field assignment, which is a bit nicer in some cases.
Not very important, since it's a private API, but still.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55224