[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.

hokein created this revision. hokein added reviewers: ilya-biryukov, ioeric. Herald added subscribers: arphaman, jkorous, MaskRay. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50896 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clang

[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.

ilya-biryukov added inline comments. Comment at: clangd/XRefs.cpp:665 +std::vector references(ParsedAST &AST, Position Pos, + bool IncludeDeclaration, + const SymbolIndex *Index) { Are we going to su

[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.

hokein added inline comments. Comment at: clangd/XRefs.cpp:665 +std::vector references(ParsedAST &AST, Position Pos, + bool IncludeDeclaration, + const SymbolIndex *Index) { ilya-biryukov wrote: > Ar

[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.

ilya-biryukov added a comment. This change LG, but I would not commit it before we have an actual implementation. As soon as we have the `references` function in `ClangdUnit.cpp` implemented, the merge of this change should be trivial. Is there any value in committing empty stubs before an actu

[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.

hokein added a comment. In https://reviews.llvm.org/D50896#1204310, @ilya-biryukov wrote: > This change LG, but I would not commit it before we have an actual > implementation. > As soon as we have the `references` function in `ClangdUnit.cpp` > implemented, the merge of this change should be

[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.

ilya-biryukov added a comment. Having unimplemented stubs in the codebase creates some confusion, but that's my personal opinion. Not terribly opposed to that if others feel it's the right way to go. For experiments, we could simply patch this in locally without committing upstream. With git i

[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.

ioeric added a comment. Agreed with Ilya. I'd probably also make this depend on the ongoing implementation, as exposing LSP endpoints without proper implementation might be confusing to clangd users who only look at the the LSP endpoints. Users need to dig two levels of abstraction to find out

[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.

sammccall commandeered this revision. sammccall added a reviewer: hokein. sammccall added a comment. Herald added a subscriber: kadircet. (Taking this as @hokein is on leave and the dependency has landed in https://reviews.llvm.org/D50958) Repository: rCTE Clang Tools Extra https://reviews.l

[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.

sammccall updated this revision to Diff 164010. sammccall added a comment. Updated based on findReferences implementation which has now landed. Removed ReferenceContext support for now, implementation has no options. references->findReferences in ClangdServer (more consistent with other methods) A

[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.

sammccall marked 4 inline comments as done. sammccall added a comment. PTAL Comment at: clangd/ClangdServer.h:158 + /// Retrieve locations for symbol references. + void references(PathRef File, Position Pos, bool includeDeclaration, + Callback> CB); -

[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Thanks, LGTM, ship it! Comment at: clangd/Protocol.h:881 +struct ReferenceParams : public TextDocumentPositionParams {}; +bool fromJSON(const llvm::json::Value &, Reference

[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.

This revision was automatically updated to reflect the committed changes. sammccall marked 3 inline comments as done. Closed by commit rCTE341462: [clangd] Add xrefs LSP boilerplate implementation. (authored by sammccall, committed by ). Changed prior to commit: https://reviews.llvm.org/D50896?