[PATCH] D54529: [clangd] Add USR to textDocument/definition response

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

@benlangmuir: I hadn't seen your comment while replying, sorry for any confusion...

> The most critical piece of this is being able to ask "what symbol is at this location" and correlate that with index queries.

This makes sense (at least doing follow-up index queries, I'm not sure exactly what "correlate" would entail).
clangd does have an internal concept of an index that is designed as an integration point for out-of-tree implementations.
Of course you're free not to use that and to query a separate index based on results returned from the LSP layer, but this is likely to be lossy (e.g. find refs will give you an empty list if the symbol under the cursor has no refs visible to clangd, leaving nothing to query the external index for).
The SymbolIndex API is not USR based, but in practice SymbolIDs are truncated hashes of USRs and we could probably add some guarantees there if it would help - integration may be an option.

> Beyond this one case, my experience is that any time a language service API is providing information about a symbol or list of symbols it may be useful to add the USR (e.g. hover, definition or workspace symbols).

So in general I'm afraid this is unlikely.

- As mentioned, clangd doesn't use USRs to identify symbols, but rather SymbolID. This representation was chosen specifically to avoid coupling to USR, and to be more compact. Index results do not have USRs available.
- in principle embedding SymbolID in results seems ok. I do think in practice there's not much you can *do* with one outside clangd, without guarantees about SymbolID format. At that point (with guarantees) we're closer to the C++-embedding level of stability/coupling rather than the LSP-protocol level.

> Our experience with sourcekitd for Swift is that a cursor info request is a useful place to put an assortment of information that may grow over time

Good to know, this does seem like a reasonable direction, and exposing USR there seems totally fine. The only high-level question I can see here is should such a request hit the index or rely on AST only? The index is needed for e.g. definitions not visible in the AST, but may spend some CPU or even a network RPC.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54529