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