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

Reply via email to