Skip to content

Commit 0dbab7f

Browse files
sam-mccallsam-mccall
committed
[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 llvm-svn: 352953
1 parent fa36540 commit 0dbab7f

File tree

2 files changed

+74
-15
lines changed

2 files changed

+74
-15
lines changed

‎clang-tools-extra/clangd/ClangdLSPServer.cpp

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -726,18 +726,41 @@ void ClangdLSPServer::onSignatureHelp(const TextDocumentPositionParams &Params,
726726
std::move(Reply));
727727
}
728728

729+
// Go to definition has a toggle function: if def and decl are distinct, then
730+
// the first press gives you the def, the second gives you the matching def.
731+
// getToggle() returns the counterpart location that under the cursor.
732+
//
733+
// We return the toggled location alone (ignoring other symbols) to encourage
734+
// editors to "bounce" quickly between locations, without showing a menu.
735+
static Location *getToggle(const TextDocumentPositionParams &Point,
736+
LocatedSymbol &Sym) {
737+
// Toggle only makes sense with two distinct locations.
738+
if (!Sym.Definition || *Sym.Definition == Sym.PreferredDeclaration)
739+
return nullptr;
740+
if (Sym.Definition->uri.file() == Point.textDocument.uri.file() &&
741+
Sym.Definition->range.contains(Point.position))
742+
return &Sym.PreferredDeclaration;
743+
if (Sym.PreferredDeclaration.uri.file() == Point.textDocument.uri.file() &&
744+
Sym.PreferredDeclaration.range.contains(Point.position))
745+
return &*Sym.Definition;
746+
return nullptr;
747+
}
748+
729749
void ClangdLSPServer::onGoToDefinition(const TextDocumentPositionParams &Params,
730750
Callback<std::vector<Location>> Reply) {
731751
Server->locateSymbolAt(
732752
Params.textDocument.uri.file(), Params.position,
733753
Bind(
734-
[&](decltype(Reply) Reply,
735-
llvm::Expected<std::vector<LocatedSymbol>> Symbols) {
754+
[&, Params](decltype(Reply) Reply,
755+
llvm::Expected<std::vector<LocatedSymbol>> Symbols) {
736756
if (!Symbols)
737757
return Reply(Symbols.takeError());
738758
std::vector<Location> Defs;
739-
for (const auto &S : *Symbols)
759+
for (auto &S : *Symbols) {
760+
if (Location *Toggle = getToggle(Params, S))
761+
return Reply(std::vector<Location>{std::move(*Toggle)});
740762
Defs.push_back(S.Definition.getValueOr(S.PreferredDeclaration));
763+
}
741764
Reply(std::move(Defs));
742765
},
743766
std::move(Reply)));
@@ -749,13 +772,16 @@ void ClangdLSPServer::onGoToDeclaration(
749772
Server->locateSymbolAt(
750773
Params.textDocument.uri.file(), Params.position,
751774
Bind(
752-
[&](decltype(Reply) Reply,
753-
llvm::Expected<std::vector<LocatedSymbol>> Symbols) {
775+
[&, Params](decltype(Reply) Reply,
776+
llvm::Expected<std::vector<LocatedSymbol>> Symbols) {
754777
if (!Symbols)
755778
return Reply(Symbols.takeError());
756779
std::vector<Location> Decls;
757-
for (const auto &S : *Symbols)
758-
Decls.push_back(S.PreferredDeclaration);
780+
for (auto &S : *Symbols) {
781+
if (Location *Toggle = getToggle(Params, S))
782+
return Reply(std::vector<Location>{std::move(*Toggle)});
783+
Decls.push_back(std::move(S.PreferredDeclaration));
784+
}
759785
Reply(std::move(Decls));
760786
},
761787
std::move(Reply)));

‎clang-tools-extra/test/clangd/xrefs.test

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,54 +1,87 @@
11
# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
22
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
33
---
4-
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"int x = 0;\nint y = x;"}}}
4+
{"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;"}}}
55
---
6-
{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":1,"character":8}}}
6+
{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":8}}}
77
# CHECK: "id": 1,
88
# CHECK-NEXT: "jsonrpc": "2.0",
99
# CHECK-NEXT: "result": [
1010
# CHECK-NEXT: {
1111
# CHECK-NEXT: "range": {
1212
# CHECK-NEXT: "end": {
1313
# CHECK-NEXT: "character": 5,
14-
# CHECK-NEXT: "line": 0
14+
# CHECK-NEXT: "line": 1
1515
# CHECK-NEXT: },
1616
# CHECK-NEXT: "start": {
1717
# CHECK-NEXT: "character": 4,
18+
# CHECK-NEXT: "line": 1
19+
# CHECK-NEXT: }
20+
# CHECK-NEXT: },
21+
# CHECK-NEXT: "uri": "file://{{.*}}/{{([A-Z]:/)?}}main.cpp"
22+
# CHECK-NEXT: }
23+
# CHECK-NEXT: ]
24+
---
25+
# Toggle: we're on the definition, so jump to the declaration.
26+
{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":1,"character":4}}}
27+
# CHECK: "id": 1,
28+
# CHECK-NEXT: "jsonrpc": "2.0",
29+
# CHECK-NEXT: "result": [
30+
# CHECK-NEXT: {
31+
# CHECK-NEXT: "range": {
32+
# CHECK-NEXT: "end": {
33+
# CHECK-NEXT: "character": 12,
34+
# CHECK-NEXT: "line": 0
35+
# CHECK-NEXT: },
36+
# CHECK-NEXT: "start": {
37+
# CHECK-NEXT: "character": 11,
1838
# CHECK-NEXT: "line": 0
1939
# CHECK-NEXT: }
2040
# CHECK-NEXT: },
2141
# CHECK-NEXT: "uri": "file://{{.*}}/{{([A-Z]:/)?}}main.cpp"
2242
# CHECK-NEXT: }
2343
# CHECK-NEXT: ]
2444
---
25-
{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":1,"character":8}}}
45+
{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":8}}}
2646
# CHECK: "id": 1
2747
# CHECK-NEXT: "jsonrpc": "2.0",
2848
# CHECK-NEXT: "result": [
2949
# CHECK-NEXT: {
3050
# CHECK-NEXT: "kind": 1,
3151
# CHECK-NEXT: "range": {
3252
# CHECK-NEXT: "end": {
33-
# CHECK-NEXT: "character": 5,
53+
# CHECK-NEXT: "character": 12,
3454
# CHECK-NEXT: "line": 0
3555
# CHECK-NEXT: },
3656
# CHECK-NEXT: "start": {
37-
# CHECK-NEXT: "character": 4,
57+
# CHECK-NEXT: "character": 11,
3858
# CHECK-NEXT: "line": 0
3959
# CHECK-NEXT: }
4060
# CHECK-NEXT: }
4161
# CHECK-NEXT: },
4262
# CHECK-NEXT: {
63+
# CHECK-NEXT: "kind": 1,
64+
# CHECK-NEXT: "range": {
65+
# CHECK-NEXT: "end": {
66+
# CHECK-NEXT: "character": 5,
67+
# CHECK-NEXT: "line": 1
68+
# CHECK-NEXT: },
69+
# CHECK-NEXT: "start": {
70+
# CHECK-NEXT: "character": 4,
71+
# CHECK-NEXT: "line": 1
72+
# CHECK-NEXT: }
73+
# CHECK-NEXT: }
74+
# CHECK-NEXT: },
75+
# CHECK-NEXT: {
4376
# CHECK-NEXT: "kind": 2,
4477
# CHECK-NEXT: "range": {
4578
# CHECK-NEXT: "end": {
4679
# CHECK-NEXT: "character": 9,
47-
# CHECK-NEXT: "line": 1
80+
# CHECK-NEXT: "line": 2
4881
# CHECK-NEXT: },
4982
# CHECK-NEXT: "start": {
5083
# CHECK-NEXT: "character": 8,
51-
# CHECK-NEXT: "line": 1
84+
# CHECK-NEXT: "line": 2
5285
# CHECK-NEXT: }
5386
# CHECK-NEXT: }
5487
# CHECK-NEXT: }

0 commit comments

Comments
 (0)