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

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

> It's also for textDocument/documentSymbol. For this, we technically don't need them in the static index since we could collect symbols when the document is opened, but we also want them for workspaceSymbols so we might as well use the same symbol collector, etc. There should be more things that would also use symbol in main files, like Type Hierarchy.

I see. Thanks! Looks like we would need to collect symbols in main file at some point. We could probably filter out main file symbols in dynamic index for code completion, but we would probably need a flag to turn off main file symbols in case this leads to a huge size increase. I would also expect different indexers (dynamic and static ) to share SymbolCollector, but it should be easily configured to behave differently via `SymbolCollector::Options`. Anyhow, let's discuss more about this when we are turning on main file symbols :)



================
Comment at: clangd/index/Index.h:160
+  /// The Decl::Kind for the context of the symbol, i.e. what contains it.
+  Decl::Kind DeclContextKind;
+  /// Whether or not this is an enumerator inside a scoped enum (C++11).
----------------
malaperle wrote:
> ioeric wrote:
> > ilya-biryukov wrote:
> > > How do we use `DeclContextKind`?
> > > Why did we decide to not go with a `bool ForCompletion` instead? (I'm probably missing a conversation in the workspaceSymbol review, could you point me to those instead?)
> > > 
> > > I'm asking because clang enums are very detailed and designed for use in the compiler, using them in the index seems to complicate things.
> > > It feels we don't need this level of detail in the symbols. Similar to how we don't store the whole structural type, but rely on string representation of completion label instead.
> > +1
> > 
> > `ForCompletion` sounds reasonable as the current design of index-based code completion relies on assumptions about contexts. 
> My thinking was that the "ForCompletion" boolean was too specific and tailored for one client use. I thought the Symbol information should not have that much hard-coded knowledge on how it would be used. It would be odd to have "ForWorkspaceSymbol", "ForDocumentSymbol", etc. That is why I was going for a slightly more detailed symbol information that opened the door for more arbitrary queries for symbol clients. But it complicates things a bit more and I'd be happy to bring back the "ForCompletion" if it makes more sense for now.
I think code completion, with the most complicated use of the index so far, probably deserves a flag :P I would expect/hope other features to be less "picky" about symbols. A high-level flag like `ForCompletion` would help keep knowledge about filtering for code completion (e.g. enums ...) inside symbol collector, which I think could be a win.

FWIW, `ForCompletion` makes it sound like a symbols is collected specifically for code completion. Maybe something like `bool SupportGlobalCompletion` would be better?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44954