[PATCH] D94952: [clangd] Take into account what is in the index (symbols, references, etc.) at indexes merge

Aleksandr Platonov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 25 22:22:42 PST 2021


ArcsinX added inline comments.


================
Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:433
     PreambleSymbols.update(
-        Uri, std::make_unique<SymbolSlab>(std::move(*IF->Symbols)),
+        FilePath ? *FilePath : (consumeError(FilePath.takeError()), Uri),
+        std::make_unique<SymbolSlab>(std::move(*IF->Symbols)),
----------------
ArcsinX wrote:
> sammccall wrote:
> > Is this change related? It changes the key scheme for the preamble index from URIs to paths, but I'm not sure why.
> > 
> > Do we have multiple URIs pointing to the same path? What are they concretely?
> This is the main thing in this patch. I will try to explain.
> We use these keys to create the file list, which is used by `indexedFiles()`.
> Currently, the preamble index contains URI's instead of paths (as a file list), that leads to the function returned by `PreambleIndex::indexedFiles()` always return `false` (because we pass to this function paths, not URI's). So, we always take data from the preamble index (but maybe we should not in some cases).
> 
> that leads to the function returned by `PreambleIndex::indexedFiles()` always return `false` (because we pass to this function paths, not URI's)

This is a bit incorrect.
We pass to this function URI, but this URI is converted to path. i.e. `MemIndex::indexedFiles()`, `Dex::indexedFiles()` expect that `Files` are paths, but they are URI's for the preamble index. That's why `PreambleIndex::indexedFiles()` always return `false`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94952



More information about the cfe-commits mailing list