[PATCH] D56314: [clangd] Don't store completion info if the symbol is not used for code completion.

[Haojian Wu via Phabricator via cfe-commits]

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