[PATCH] D44954: [clangd] Add "member" symbols to the index

[Marc-Andre Laperle via Phabricator via cfe-commits]

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


================
Comment at: clangd/index/Index.h:158
   unsigned References = 0;
-
+  /// Whether or not this is symbol is meant to be used for the global
+  /// completion.
----------------
sammccall wrote:
> ioeric wrote:
> > s/this is symbol/this symbol/?
> Sorry, I hadn't seen this patch until now.
> When it was part of the `workspace/symbol` patch, I was the other person concerned this was too coupled to existing use cases and preferred something more descriptive.
> 
> I dug up an analysis @hokein and I worked on. One concluseion we came to was that we thought results needed by `completion` were a subset of what `workspace/symbol` needs, so a boolean would work. It seemed a bit ad-hoc and fragile though.
> 
> The cases we looked at were:
> 
> ||private|member|local|primary template|template specialization|nested in template|
> | code complete |N|N|N|Y|N|N|
> | workspace/symbol |Y|Y|N|Y|Y|Y|
> | go to defn |Y|Y|?|?|?|?|
> (Y = index should return it, N = index should not return it, ? = don't care)
> 
> So the most relevant information seems to be:
>  - is this private (private member, internal linkage, no decl outside cpp file, etc)
>  - is this nested in a class type (or template)
>  - is this a template specialization
> 
> I could live with bundling these into a single property (though they seem like good ranking signals, and we'd lose them for that purpose), it will certainly make a posting-list based index more efficient.
> 
> In that case I think we should have canonical documentation *somewhere* about exactly what this subset is, and this field should reference that.
> e.g. `isIndexedForCodeCompletion()` in `CodeComplete.h` with docs and `IsIndexedForCodeCompletion` here. (Please avoid "global" as it has too many different meanings - here we mean "index-based").
OK, I added documentation on the SymbolCollector (which was outdated) about what kinds of symbols are collected, with a reference to shouldFilterDecl. The subset is documented on isIndexedForCodeCompletion(), also referenced from the Symbol field. Was that more or less what you meant?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44954