[PATCH] D91721: [clangd] textDocument/implementation (LSP layer)
kadircet added inline comments. Herald added projects: clang-tools-extra, All. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1306 +for (const LocatedSymbol : *Overrides) + Impls.push_back(Sym.PreferredDeclaration); +return Reply(std::move(Impls)); any particular reason for preferring declarations here that I am missing (in presence of a definition)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91721/new/ https://reviews.llvm.org/D91721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D91721: [clangd] textDocument/implementation (LSP layer)
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb31486ad9717: [clangd] textDocument/implementation (LSP
layer) (authored by usaxena95).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91721/new/
https://reviews.llvm.org/D91721
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/test/implementations.test
clang-tools-extra/clangd/test/initialize-params.test
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -67,7 +67,8 @@
# CHECK-NEXT:]
# CHECK-NEXT: },
# CHECK-NEXT: "hoverProvider": true,
-# CHECK-NEXT: "memoryUsageProvider": true
+# CHECK-NEXT: "implementationProvider": true,
+# CHECK-NEXT: "memoryUsageProvider": true,
# CHECK-NEXT: "referencesProvider": true,
# CHECK-NEXT: "renameProvider": true,
# CHECK-NEXT: "selectionRangeProvider": true,
Index: clang-tools-extra/clangd/test/implementations.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/implementations.test
@@ -0,0 +1,39 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"struct Parent { virtual void Foo(); };\nstruct Child1 : Parent { void Foo() override(); };\nstruct Child2 : Parent { void Foo() override(); };"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/implementation","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":32}}}
+# CHECK: "id": 1
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT: "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT: "character": 33,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT: "character": 30,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT:}
+# CHECK-NEXT: },
+# CHECK-NEXT: "uri": "file://{{.*}}/clangd-test/main.cpp"
+# CHECK-NEXT:},
+# CHECK-NEXT:{
+# CHECK-NEXT: "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT: "character": 33,
+# CHECK-NEXT: "line": 2
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT: "character": 30,
+# CHECK-NEXT: "line": 2
+# CHECK-NEXT:}
+# CHECK-NEXT: },
+# CHECK-NEXT:"uri": "file://{{.*}}/clangd-test/main.cpp"
+# CHECK-NEXT: }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -253,6 +253,10 @@
/// Retrieve ranges that can be used to fold code within the specified file.
void foldingRanges(StringRef File, Callback> CB);
+ /// Retrieve implementations for virtual method.
+ void findImplementations(PathRef File, Position Pos,
+ Callback> CB);
+
/// Retrieve locations for symbol references.
void findReferences(PathRef File, Position Pos, uint32_t Limit,
Callback CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -718,6 +718,18 @@
TUScheduler::InvalidateOnUpdate);
}
+void ClangdServer::findImplementations(
+PathRef File, Position Pos, Callback> CB) {
+ auto Action = [Pos, CB = std::move(CB),
+ this](llvm::Expected InpAST) mutable {
+if (!InpAST)
+ return CB(InpAST.takeError());
+CB(clangd::findImplementations(InpAST->AST, Pos, Index));
+ };
+
+ WorkScheduler.runWithAST("Implementations", File, std::move(Action));
+}
+
void ClangdServer::findReferences(PathRef File, Position Pos, uint32_t Limit,
Callback CB) {
auto Action = [Pos, Limit, CB = std::move(CB),
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -115,6 +115,8 @@
Callback>);
void
[PATCH] D91721: [clangd] textDocument/implementation (LSP layer)
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. thanks, looks good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91721/new/ https://reviews.llvm.org/D91721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D91721: [clangd] textDocument/implementation (LSP layer)
usaxena95 updated this revision to Diff 306969.
usaxena95 added a comment.
Addressed comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91721/new/
https://reviews.llvm.org/D91721
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/test/implementations.test
clang-tools-extra/clangd/test/initialize-params.test
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -67,7 +67,8 @@
# CHECK-NEXT:]
# CHECK-NEXT: },
# CHECK-NEXT: "hoverProvider": true,
-# CHECK-NEXT: "memoryUsageProvider": true
+# CHECK-NEXT: "implementationProvider": true,
+# CHECK-NEXT: "memoryUsageProvider": true,
# CHECK-NEXT: "referencesProvider": true,
# CHECK-NEXT: "renameProvider": true,
# CHECK-NEXT: "selectionRangeProvider": true,
Index: clang-tools-extra/clangd/test/implementations.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/implementations.test
@@ -0,0 +1,39 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"struct Parent { virtual void Foo(); };\nstruct Child1 : Parent { void Foo() override(); };\nstruct Child2 : Parent { void Foo() override(); };"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/implementation","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":32}}}
+# CHECK: "id": 1
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT: "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT: "character": 33,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT: "character": 30,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT:}
+# CHECK-NEXT: },
+# CHECK-NEXT: "uri": "file://{{.*}}/clangd-test/main.cpp"
+# CHECK-NEXT:},
+# CHECK-NEXT:{
+# CHECK-NEXT: "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT: "character": 33,
+# CHECK-NEXT: "line": 2
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT: "character": 30,
+# CHECK-NEXT: "line": 2
+# CHECK-NEXT:}
+# CHECK-NEXT: },
+# CHECK-NEXT:"uri": "file://{{.*}}/clangd-test/main.cpp"
+# CHECK-NEXT: }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -253,6 +253,10 @@
/// Retrieve ranges that can be used to fold code within the specified file.
void foldingRanges(StringRef File, Callback> CB);
+ /// Retrieve implementations for virtual method.
+ void findImplementations(PathRef File, Position Pos,
+ Callback> CB);
+
/// Retrieve locations for symbol references.
void findReferences(PathRef File, Position Pos, uint32_t Limit,
Callback CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -718,6 +718,18 @@
TUScheduler::InvalidateOnUpdate);
}
+void ClangdServer::findImplementations(
+PathRef File, Position Pos, Callback> CB) {
+ auto Action = [Pos, CB = std::move(CB),
+ this](llvm::Expected InpAST) mutable {
+if (!InpAST)
+ return CB(InpAST.takeError());
+CB(clangd::findImplementations(InpAST->AST, Pos, Index));
+ };
+
+ WorkScheduler.runWithAST("Implementations", File, std::move(Action));
+}
+
void ClangdServer::findReferences(PathRef File, Position Pos, uint32_t Limit,
Callback CB) {
auto Action = [Pos, Limit, CB = std::move(CB),
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -115,6 +115,8 @@
Callback>);
void onGoToDefinition(const TextDocumentPositionParams &,
Callback>);
+ void onGoToImplementation(const
[PATCH] D91721: [clangd] textDocument/implementation (LSP layer)
usaxena95 marked 3 inline comments as done. usaxena95 added a comment. > Looks like the patch is based on the old revision (pre-merging tests are > failing), I assume you have fixed the failure tests last week? Yes. That was fixed last week. Rebased. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91721/new/ https://reviews.llvm.org/D91721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D91721: [clangd] textDocument/implementation (LSP layer)
hokein added a comment.
just nits.
Looks like the patch is based on the old revision (pre-merging tests are
failing), I assume you have fixed the failure tests last week?
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:615
{"workspaceSymbolProvider", true},
+{"implementationProvider", true},
{"referencesProvider", true},
nit: move it to line 605 (right after `definitionProvider`).
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1443
MsgHandler->bind("textDocument/references", ::onReference);
+ MsgHandler->bind("textDocument/implementation",
::onImplementation);
MsgHandler->bind("textDocument/switchSourceHeader",
::onSwitchSourceHeader);
nit: `onGoToImplementation`, move a line up to be closer with
Go-to-{declaration/definition}.
Comment at: clang-tools-extra/clangd/Protocol.h:1482
+struct ImplementationParams : public TextDocumentPositionParams {};
+bool fromJSON(const llvm::json::Value &, ImplementationParams &,
nit: since this is just a mirror of `TextDocumentPositionParams`, I'd use
`TextDocumentPositionParams` directly (looks like this is a pattern of other
features, go-to-definition etc.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91721/new/
https://reviews.llvm.org/D91721
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D91721: [clangd] textDocument/implementation (LSP layer)
usaxena95 created this revision.
Herald added subscribers: cfe-commits, kadircet, arphaman.
Herald added a project: clang.
usaxena95 requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D91721
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/test/implementations.test
clang-tools-extra/clangd/test/initialize-params.test
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -66,7 +66,8 @@
# CHECK-NEXT:]
# CHECK-NEXT: },
# CHECK-NEXT: "hoverProvider": true,
-# CHECK-NEXT: "memoryUsageProvider": true
+# CHECK-NEXT: "implementationProvider": true,
+# CHECK-NEXT: "memoryUsageProvider": true,
# CHECK-NEXT: "referencesProvider": true,
# CHECK-NEXT: "renameProvider": true,
# CHECK-NEXT: "selectionRangeProvider": true,
Index: clang-tools-extra/clangd/test/implementations.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/implementations.test
@@ -0,0 +1,39 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"struct Parent { virtual void Foo(); };\nstruct Child1 : Parent { void Foo() override(); };\nstruct Child2 : Parent { void Foo() override(); };"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/implementation","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":32}}}
+# CHECK: "id": 1
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT: "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT: "character": 33,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT: "character": 30,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT:}
+# CHECK-NEXT: },
+# CHECK-NEXT: "uri": "file://{{.*}}/clangd-test/main.cpp"
+# CHECK-NEXT:},
+# CHECK-NEXT:{
+# CHECK-NEXT: "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT: "character": 33,
+# CHECK-NEXT: "line": 2
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT: "character": 30,
+# CHECK-NEXT: "line": 2
+# CHECK-NEXT:}
+# CHECK-NEXT: },
+# CHECK-NEXT:"uri": "file://{{.*}}/clangd-test/main.cpp"
+# CHECK-NEXT: }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1479,6 +1479,10 @@
};
bool fromJSON(const llvm::json::Value &, ReferenceParams &, llvm::json::Path);
+struct ImplementationParams : public TextDocumentPositionParams {};
+bool fromJSON(const llvm::json::Value &, ImplementationParams &,
+ llvm::json::Path);
+
/// Clangd extension: indicates the current state of the file in clangd,
/// sent from server via the `textDocument/clangd.fileStatus` notification.
struct FileStatus {
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1261,6 +1261,12 @@
return llvm::json::Object{{"to", C.to}, {"fromRanges", C.fromRanges}};
}
+bool fromJSON(const llvm::json::Value , ImplementationParams ,
+ llvm::json::Path P) {
+ TextDocumentPositionParams = R;
+ return fromJSON(Params, Base, P);
+}
+
static const char *toString(OffsetEncoding OE) {
switch (OE) {
case OffsetEncoding::UTF8:
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -253,6 +253,10 @@
/// Retrieve ranges that can be used to fold code within the specified file.
void foldingRanges(StringRef File, Callback> CB);
+ /// Retrieve implementations for virtual method.
+ void findImplementations(PathRef File, Position Pos,
+ Callback> CB);
+
/// Retrieve locations for symbol references.
void findReferences(PathRef File,
