[PATCH] D56592: [clangd] Fix updated file detection logic in indexing

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

This looks neat, thanks for the change!
Just a few NITs from my side



================
Comment at: clangd/index/Background.cpp:264
+/// Given index results from a TU, only update symbols coming from files with
+/// different digests than \p DigestsSnapshot. Also stores new index
+/// information on IndexStorage.
----------------
NIT: maybe rephrase to "different or missing from DigestsSnapshot"?


================
Comment at: clangd/index/Background.cpp:280
+    const auto AbsPath = URICache.resolve(IGN.URI);
+    const auto &DigestIt = DigestsSnapshot.find(AbsPath);
+    // File has different contents.
----------------
Use value type, since it's an iterator:
`auto DigestIt = DigestsSnapshot.find` or `const auto DigestIt = …`


================
Comment at: clangd/index/Background.cpp:288
       auto DeclPath = URICache.resolve(Sym.CanonicalDeclaration.FileURI);
-      if (FilesToUpdate.count(DeclPath) != 0)
+      if (Files.count(DeclPath) != 0)
         Files[DeclPath].Symbols.insert(&Sym);
----------------
Maybe avoid extra lookups?
```
auto FileIt = Files.find(DeclPath);
if (FileIt != Files.end())
  FileIt->Symbols.insert(&Sym);
```


================
Comment at: clangd/index/Background.cpp:298
       auto DefPath = URICache.resolve(Sym.Definition.FileURI);
-      if (FilesToUpdate.count(DefPath) != 0)
+      if (Files.count(DefPath) != 0)
         Files[DefPath].Symbols.insert(&Sym);
----------------
Maybe avoid extra lookups here too?


================
Comment at: clangd/index/Background.cpp:306
       auto Path = URICache.resolve(R.Location.FileURI);
-      if (FilesToUpdate.count(Path) != 0) {
+      if (Files.count(Path) != 0) {
         auto &F = Files[Path];
----------------
Same here, maybe avoid extra lookups?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56592