kadircet added a comment. I think we also need to update `index/remote/Server.cpp` && `FileSymbols` (and `FileIndex` too).
Regarding updates to `loadIndex`, I actually think it makes sense for that index the always retrieve symbols as `Static` origin, then whoever makes use of that (we always have either a `FileSymbol` or a complete remote-index marshaling on top) should overwrite the origin while either building the final merged index or during marshaling. That would get rid of lots of extra plumbing. WDYT? ================ Comment at: clang-tools-extra/clangd/index/SymbolOrigin.cpp:17 return OS << "unknown"; - constexpr static char Sigils[] = "ADSMIR67"; + constexpr static char Sigils[] = "AOSMIR67BP012345"; for (unsigned I = 0; I < sizeof(Sigils); ++I) ---------------- `s/P/9` && `s/6/P` ================ Comment at: clang-tools-extra/clangd/index/SymbolOrigin.h:26 + Static = 1 << 2, // From a static, externally-built index. Merge = 1 << 3, // A non-trivial index merge was performed. Identifier = 1 << 4, // Raw identifiers in file. ---------------- do we think this still provides a meaningful signal ? It only provides value only when multiple indices of same type was involved in providing the result (and only that single type of indices). I suppose after this patch it can only happen for `SM` (as there are certain remote-index implementations that mark themselves as static) or `RM` (well this is not possible today, but some day we might have a stack of remote-indices). As soon as there's a different type of index is involved `M` no longer has any meanings, as it's clear that there was some sort of merge happening (or am I missing something obvious?) --- Before this patch situation is a little bit different since `FileIndex` just said `D` for both main file and preamble, but that's changing. ================ Comment at: clang-tools-extra/clangd/index/SymbolOrigin.h:30 + Preamble = 1 << 6, // From the dynamic index of preambles. + // 7 reserved + Background = 1 << 8, // From the automatic project index. ---------------- thanks for remembering that! :D Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115243/new/ https://reviews.llvm.org/D115243 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits