The protocol is based on the spec found here:
https://microsoft.github.io/language-server-protocol/specifications/specification-3-16/#textDocument_prepareCallHierarchy
Details
- Reviewers
kadircet - Commits
- rG9d77584fe040: [clangd] Call hierarchy (Protocol layer)
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Keeping in the draft state for now, as I still need to address this comment from the issue discussion:
The request still has a progressToken attached to it, I presume(it is unclear from the spec) it is preserved between prepare and subsequent requests. So we can keep a mapping in ClangdServer and stash symbolIDs.
The protocol has since been amended to introduce a data field to CallHierarchyItem in which we can stash the SymbolID, making this workaround unnecessary.
The updated patch uses the data field, and should be ready for review.
Note, testing this with vscode requires this vscode-clangd patch applied (to use a newer version of the client libraries that support the data field).
Kadir, would you like to take a look at this, as you're already familiar with the index changes? :)
Thanks, I skimmed over it (again) and I think this looks pretty great in general. I couldn't take a look at the details yet, I would really appreciate it if you could chop this one into smaller pieces (i know, i asked it before, but this is still too big). I suppose something like:
- Changes in the XRefs side, introducing {prepare,incoming,outgoing} CallHierarchy bits, relevant Protocol.h additions and unittests for those.
- Surfacing this in ClangdServer and ClangdLSPServer layer, with relevant unit/lit tests. (I would be even more happier if you could split this in two again one for each layer :) )
would be great.
If you can't split it up, that's also fine. I'll go over it eventually, it will just take longer and probably be harder to remember where we left off after each iteration :D
clang-tools-extra/clangd/Protocol.cpp | ||
---|---|---|
1174 | we should also populate tags and detail conditionally. rather than providing empty strings/vectors. | |
1203 | s/"from"/"to" | |
clang-tools-extra/clangd/Protocol.h | ||
1351 | we use exactly the same names in protocol here, even if they don't adhere to the style guide. i.e. s/Name/name. same for others. | |
1367 | s/Rng/range | |
1376 | i would drop the optional, as there's no difference between empty vs none. | |
1405 | could you put this before CallHierarchyItem ? | |
1408 | could you put this before CallHierarchyIncomingCall, same for outgoing |
we use exactly the same names in protocol here, even if they don't adhere to the style guide. i.e. s/Name/name. same for others.