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

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 4 04:59:49 PST 2021


sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Sorry, forgot to stamp this!



================
Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:419
       CollectMainFileRefs(CollectMainFileRefs),
+      PreambleSymbols(IndexContents::Symbols),
       PreambleIndex(std::make_unique<MemIndex>()),
----------------
seems we include relations too?


================
Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:421
       PreambleIndex(std::make_unique<MemIndex>()),
+      MainFileSymbols(IndexContents::All),
       MainFileIndex(std::make_unique<MemIndex>()) {}
----------------
(this should take into account CollectMainFileRefs, but actually if you sync to HEAD it's always true now)


================
Comment at: clang-tools-extra/clangd/index/FileIndex.h:74
 public:
+  FileSymbols(IndexContents IdxContents = IndexContents::None);
   /// Updates all slabs associated with the \p Key.
----------------
It seems error-prone to have a default here.
(I guess it's for making tests shorter? Are there enough callsites for it to matter?)

If we must have a default, All seems to make more sense than None.


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