[clang-tools-extra] r352953 - [Clangd] textDocument/definition and textDocument/declaration "bounce" between definition and declaration location when they are distinct.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 1 21:56:01 PST 2019
Author: sammccall
Date: Fri Feb 1 21:56:00 2019
New Revision: 352953
URL: http://llvm.org/viewvc/llvm-project?rev=352953&view=rev
Log:
[Clangd] textDocument/definition and textDocument/declaration "bounce" between definition and declaration location when they are distinct.
Summary:
This helps minimize the disruption of not returning declarations as part of
a find-definition response (r352864).
Reviewers: hokein
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, ilya-biryukov
Tags: #clang
Differential Revision: https://reviews.llvm.org/D57580
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/test/clangd/xrefs.test
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=352953&r1=352952&r2=352953&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Fri Feb 1 21:56:00 2019
@@ -726,18 +726,41 @@ void ClangdLSPServer::onSignatureHelp(co
std::move(Reply));
}
+// Go to definition has a toggle function: if def and decl are distinct, then
+// the first press gives you the def, the second gives you the matching def.
+// getToggle() returns the counterpart location that under the cursor.
+//
+// We return the toggled location alone (ignoring other symbols) to encourage
+// editors to "bounce" quickly between locations, without showing a menu.
+static Location *getToggle(const TextDocumentPositionParams &Point,
+ LocatedSymbol &Sym) {
+ // Toggle only makes sense with two distinct locations.
+ if (!Sym.Definition || *Sym.Definition == Sym.PreferredDeclaration)
+ return nullptr;
+ if (Sym.Definition->uri.file() == Point.textDocument.uri.file() &&
+ Sym.Definition->range.contains(Point.position))
+ return &Sym.PreferredDeclaration;
+ if (Sym.PreferredDeclaration.uri.file() == Point.textDocument.uri.file() &&
+ Sym.PreferredDeclaration.range.contains(Point.position))
+ return &*Sym.Definition;
+ return nullptr;
+}
+
void ClangdLSPServer::onGoToDefinition(const TextDocumentPositionParams &Params,
Callback<std::vector<Location>> Reply) {
Server->locateSymbolAt(
Params.textDocument.uri.file(), Params.position,
Bind(
- [&](decltype(Reply) Reply,
- llvm::Expected<std::vector<LocatedSymbol>> Symbols) {
+ [&, Params](decltype(Reply) Reply,
+ llvm::Expected<std::vector<LocatedSymbol>> Symbols) {
if (!Symbols)
return Reply(Symbols.takeError());
std::vector<Location> Defs;
- for (const auto &S : *Symbols)
+ for (auto &S : *Symbols) {
+ if (Location *Toggle = getToggle(Params, S))
+ return Reply(std::vector<Location>{std::move(*Toggle)});
Defs.push_back(S.Definition.getValueOr(S.PreferredDeclaration));
+ }
Reply(std::move(Defs));
},
std::move(Reply)));
@@ -749,13 +772,16 @@ void ClangdLSPServer::onGoToDeclaration(
Server->locateSymbolAt(
Params.textDocument.uri.file(), Params.position,
Bind(
- [&](decltype(Reply) Reply,
- llvm::Expected<std::vector<LocatedSymbol>> Symbols) {
+ [&, Params](decltype(Reply) Reply,
+ llvm::Expected<std::vector<LocatedSymbol>> Symbols) {
if (!Symbols)
return Reply(Symbols.takeError());
std::vector<Location> Decls;
- for (const auto &S : *Symbols)
- Decls.push_back(S.PreferredDeclaration);
+ for (auto &S : *Symbols) {
+ if (Location *Toggle = getToggle(Params, S))
+ return Reply(std::vector<Location>{std::move(*Toggle)});
+ Decls.push_back(std::move(S.PreferredDeclaration));
+ }
Reply(std::move(Decls));
},
std::move(Reply)));
Modified: clang-tools-extra/trunk/test/clangd/xrefs.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/xrefs.test?rev=352953&r1=352952&r2=352953&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/xrefs.test (original)
+++ clang-tools-extra/trunk/test/clangd/xrefs.test Fri Feb 1 21:56:00 2019
@@ -1,9 +1,9 @@
# 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":"int x = 0;\nint y = x;"}}}
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"extern int x;\nint x = 0;\nint y = x;"}}}
---
-{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":1,"character":8}}}
+{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":8}}}
# CHECK: "id": 1,
# CHECK-NEXT: "jsonrpc": "2.0",
# CHECK-NEXT: "result": [
@@ -11,10 +11,30 @@
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
# CHECK-NEXT: "character": 5,
-# CHECK-NEXT: "line": 0
+# CHECK-NEXT: "line": 1
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
# CHECK-NEXT: "character": 4,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: "uri": "file://{{.*}}/{{([A-Z]:/)?}}main.cpp"
+# CHECK-NEXT: }
+# CHECK-NEXT: ]
+---
+# Toggle: we're on the definition, so jump to the declaration.
+{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":1,"character":4}}}
+# CHECK: "id": 1,
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 12,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 11,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -22,7 +42,7 @@
# CHECK-NEXT: }
# CHECK-NEXT: ]
---
-{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":1,"character":8}}}
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":8}}}
# CHECK: "id": 1
# CHECK-NEXT: "jsonrpc": "2.0",
# CHECK-NEXT: "result": [
@@ -30,25 +50,38 @@
# CHECK-NEXT: "kind": 1,
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 5,
+# CHECK-NEXT: "character": 12,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 4,
+# CHECK-NEXT: "character": 11,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: {
+# CHECK-NEXT: "kind": 1,
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 5,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 4,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: {
# CHECK-NEXT: "kind": 2,
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
# CHECK-NEXT: "character": 9,
-# CHECK-NEXT: "line": 1
+# CHECK-NEXT: "line": 2
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
# CHECK-NEXT: "character": 8,
-# CHECK-NEXT: "line": 1
+# CHECK-NEXT: "line": 2
# CHECK-NEXT: }
# CHECK-NEXT: }
# CHECK-NEXT: }
More information about the cfe-commits
mailing list