hokein marked an inline comment as done.
hokein added inline comments.

================
Comment at: clangd/index/SymbolCollector.cpp:543
+
+  if (!(S.Flags & Symbol::IndexedForCodeCompletion))
+    return Insert(S);
----------------
ilya-biryukov wrote:
> Most of the fields updated at the bottom aren't useful. However, I feel the 
> documentation is actually important, since Sema only has doc comments for the 
> **current** file and the rest are currently expected to be provided by the 
> index.
> 
> I'm not sure if we already have the code to query the doc comments via index 
> for member completions. If not, it's an oversight.
> In any case, I suggest we **always** store the comments in **dynamic** index. 
> Not storing the comments in the static index is fine, since any data for 
> member completions should be provided by the dynamic index (we see a member 
> in completion ⇒ sema has processed the headers ⇒ the dynamic index should 
> know about those members)
This is a good point.

For class member completions, we rely solely on Sema completions (no query 
being queried). I'm not sure it is practical to query the index for member 
completions.
- this means for **every** code completion, we query the index, it may slow 
down completions
- `fuzzyFind` is not supported for class members in our internal index service 
(due to the large number of them)

So it turns two possibilities:

1) always store comments (`SymbolCollector` doesn't know whether it is used in 
static index or dynamic index)
2) or drop them for now, this wouldn't break anything, and add it back when we 
actually use them for class completions

I slightly prefer 2) at the moment. WDYT?


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56314/new/

https://reviews.llvm.org/D56314



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

Reply via email to