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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
