sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/index/Background.cpp:133
+ if (!Buf) {
+ elog("Background-index: Couldn't read {0 to validate stored index: {1}",
+ LS.AbsolutePath, Buf.getError().message());
----------------
`{0` is missing its `}`
================
Comment at: clang-tools-extra/clangd/index/Background.cpp:497
+ llvm::DenseSet<PathRef> TUsToIndex;
+ // Check for staleness of loaded shards.
+ for (auto &LS : Result) {
----------------
// We'll accept data from stale shards, but ensure the files get reindexed soon.
================
Comment at: clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp:93
+
+void BackgroundIndexLoader::load(PathRef MainFile,
+ BackgroundIndexStorage *Storage) {
----------------
This handles only one main file, I can't see where you skip loading shards for
headers that were loaded for a previous file
================
Comment at: clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp:104
+ while (!ToVisit.empty()) {
+ PathRef ShardIdentifier = ToVisit.front();
+ ToVisit.pop();
----------------
ShardIdentifier still here
================
Comment at: clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp:126
+ assert(TUsIt != FileToTUs.end() && "No TU registered for the shard");
+ Result.back().DependentTUs = std::move(TUsIt->second);
+ }
----------------
I don't think this works, the storage for these StringRefs is the LoadedShards
you just moved from.
(In practice, maybe you get away with it if the strings are longer than the
SSO?)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64712/new/
https://reviews.llvm.org/D64712
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
