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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to