Skip to content

Commit e82edd2

Browse files
DrSergeiDrSergei
authored
[lldb-dap] Migrate locations request to structured types (#171099)
This patch migrates `locations` request into structured types and adds test for it.
1 parent 9de41ee commit e82edd2

File tree

5 files changed

+143
-142
lines changed

5 files changed

+143
-142
lines changed

‎lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp‎

Lines changed: 32 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "DAP.h"
10+
#include "DAPError.h"
1011
#include "EventHelper.h"
1112
#include "JSONUtils.h"
1213
#include "LLDBUtils.h"
@@ -18,167 +19,59 @@
1819

1920
namespace lldb_dap {
2021

21-
// "LocationsRequest": {
22-
// "allOf": [ { "$ref": "#/definitions/Request" }, {
23-
// "type": "object",
24-
// "description": "Looks up information about a location reference
25-
// previously returned by the debug adapter.",
26-
// "properties": {
27-
// "command": {
28-
// "type": "string",
29-
// "enum": [ "locations" ]
30-
// },
31-
// "arguments": {
32-
// "$ref": "#/definitions/LocationsArguments"
33-
// }
34-
// },
35-
// "required": [ "command", "arguments" ]
36-
// }]
37-
// },
38-
// "LocationsArguments": {
39-
// "type": "object",
40-
// "description": "Arguments for `locations` request.",
41-
// "properties": {
42-
// "locationReference": {
43-
// "type": "integer",
44-
// "description": "Location reference to resolve."
45-
// }
46-
// },
47-
// "required": [ "locationReference" ]
48-
// },
49-
// "LocationsResponse": {
50-
// "allOf": [ { "$ref": "#/definitions/Response" }, {
51-
// "type": "object",
52-
// "description": "Response to `locations` request.",
53-
// "properties": {
54-
// "body": {
55-
// "type": "object",
56-
// "properties": {
57-
// "source": {
58-
// "$ref": "#/definitions/Source",
59-
// "description": "The source containing the location; either
60-
// `source.path` or `source.sourceReference` must be
61-
// specified."
62-
// },
63-
// "line": {
64-
// "type": "integer",
65-
// "description": "The line number of the location. The client
66-
// capability `linesStartAt1` determines whether it
67-
// is 0- or 1-based."
68-
// },
69-
// "column": {
70-
// "type": "integer",
71-
// "description": "Position of the location within the `line`. It is
72-
// measured in UTF-16 code units and the client
73-
// capability `columnsStartAt1` determines whether
74-
// it is 0- or 1-based. If no column is given, the
75-
// first position in the start line is assumed."
76-
// },
77-
// "endLine": {
78-
// "type": "integer",
79-
// "description": "End line of the location, present if the location
80-
// refers to a range. The client capability
81-
// `linesStartAt1` determines whether it is 0- or
82-
// 1-based."
83-
// },
84-
// "endColumn": {
85-
// "type": "integer",
86-
// "description": "End position of the location within `endLine`,
87-
// present if the location refers to a range. It is
88-
// measured in UTF-16 code units and the client
89-
// capability `columnsStartAt1` determines whether
90-
// it is 0- or 1-based."
91-
// }
92-
// },
93-
// "required": [ "source", "line" ]
94-
// }
95-
// }
96-
// }]
97-
// },
98-
void LocationsRequestHandler::operator()(
99-
const llvm::json::Object &request) const {
100-
llvm::json::Object response;
101-
FillResponse(request, response);
102-
auto *arguments = request.getObject("arguments");
103-
104-
const auto location_id =
105-
GetInteger<uint64_t>(arguments, "locationReference").value_or(0);
22+
// Looks up information about a location reference previously returned by the
23+
// debug adapter.
24+
llvm::Expected<protocol::LocationsResponseBody>
25+
LocationsRequestHandler::Run(const protocol::LocationsArguments &args) const {
26+
protocol::LocationsResponseBody response;
10627
// We use the lowest bit to distinguish between value location and declaration
10728
// location
108-
auto [var_ref, is_value_location] = UnpackLocation(location_id);
29+
auto [var_ref, is_value_location] = UnpackLocation(args.locationReference);
10930
lldb::SBValue variable = dap.variables.GetVariable(var_ref);
110-
if (!variable.IsValid()) {
111-
response["success"] = false;
112-
response["message"] = "Invalid variable reference";
113-
dap.SendJSON(llvm::json::Value(std::move(response)));
114-
return;
115-
}
31+
if (!variable.IsValid())
32+
return llvm::make_error<DAPError>("Invalid variable reference");
11633

117-
llvm::json::Object body;
11834
if (is_value_location) {
11935
// Get the value location
12036
if (!variable.GetType().IsPointerType() &&
121-
!variable.GetType().IsReferenceType()) {
122-
response["success"] = false;
123-
response["message"] =
124-
"Value locations are only available for pointers and references";
125-
dap.SendJSON(llvm::json::Value(std::move(response)));
126-
return;
127-
}
37+
!variable.GetType().IsReferenceType())
38+
return llvm::make_error<DAPError>(
39+
"Value locations are only available for pointers and references");
12840

12941
lldb::addr_t raw_addr = variable.GetValueAsAddress();
13042
lldb::SBAddress addr = dap.target.ResolveLoadAddress(raw_addr);
13143
lldb::SBLineEntry line_entry = GetLineEntryForAddress(dap.target, addr);
13244

133-
if (!line_entry.IsValid()) {
134-
response["success"] = false;
135-
response["message"] = "Failed to resolve line entry for location";
136-
dap.SendJSON(llvm::json::Value(std::move(response)));
137-
return;
138-
}
45+
if (!line_entry.IsValid())
46+
return llvm::make_error<DAPError>(
47+
"Failed to resolve line entry for location");
13948

140-
const std::optional<protocol::Source> source =
49+
std::optional<protocol::Source> source =
14150
CreateSource(line_entry.GetFileSpec());
142-
if (!source) {
143-
response["success"] = false;
144-
response["message"] = "Failed to resolve file path for location";
145-
dap.SendJSON(llvm::json::Value(std::move(response)));
146-
return;
147-
}
51+
if (!source)
52+
return llvm::make_error<DAPError>(
53+
"Failed to resolve file path for location");
14854

149-
body.try_emplace("source", *source);
150-
if (int line = line_entry.GetLine())
151-
body.try_emplace("line", line);
152-
if (int column = line_entry.GetColumn())
153-
body.try_emplace("column", column);
55+
response.source = std::move(*source);
56+
response.line = line_entry.GetLine();
57+
response.column = line_entry.GetColumn();
15458
} else {
15559
// Get the declaration location
15660
lldb::SBDeclaration decl = variable.GetDeclaration();
157-
if (!decl.IsValid()) {
158-
response["success"] = false;
159-
response["message"] = "No declaration location available";
160-
dap.SendJSON(llvm::json::Value(std::move(response)));
161-
return;
162-
}
61+
if (!decl.IsValid())
62+
return llvm::make_error<DAPError>("No declaration location available");
16363

164-
const std::optional<protocol::Source> source =
165-
CreateSource(decl.GetFileSpec());
166-
if (!source) {
167-
response["success"] = false;
168-
response["message"] = "Failed to resolve file path for location";
169-
dap.SendJSON(llvm::json::Value(std::move(response)));
170-
return;
171-
}
64+
std::optional<protocol::Source> source = CreateSource(decl.GetFileSpec());
65+
if (!source)
66+
return llvm::make_error<DAPError>(
67+
"Failed to resolve file path for location");
17268

173-
body.try_emplace("source", *source);
174-
if (int line = decl.GetLine())
175-
body.try_emplace("line", line);
176-
if (int column = decl.GetColumn())
177-
body.try_emplace("column", column);
69+
response.source = std::move(*source);
70+
response.line = decl.GetLine();
71+
response.column = decl.GetColumn();
17872
}
17973

180-
response.try_emplace("body", std::move(body));
181-
dap.SendJSON(llvm::json::Value(std::move(response)));
74+
return response;
18275
}
18376

18477
} // namespace lldb_dap

‎lldb/tools/lldb-dap/Handler/RequestHandler.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -564,11 +564,14 @@ class VariablesRequestHandler
564564
Run(const protocol::VariablesArguments &) const override;
565565
};
566566

567-
class LocationsRequestHandler : public LegacyRequestHandler {
567+
class LocationsRequestHandler
568+
: public RequestHandler<protocol::LocationsArguments,
569+
llvm::Expected<protocol::LocationsResponseBody>> {
568570
public:
569-
using LegacyRequestHandler::LegacyRequestHandler;
571+
using RequestHandler::RequestHandler;
570572
static llvm::StringLiteral GetCommand() { return "locations"; }
571-
void operator()(const llvm::json::Object &request) const override;
573+
llvm::Expected<protocol::LocationsResponseBody>
574+
Run(const protocol::LocationsArguments &) const override;
572575
};
573576

574577
class DisassembleRequestHandler final

‎lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -701,4 +701,24 @@ bool fromJSON(const llvm::json::Value &Params, PauseArguments &Args,
701701
return O && O.map("threadId", Args.threadId);
702702
}
703703

704+
bool fromJSON(const llvm::json::Value &Params, LocationsArguments &Args,
705+
llvm::json::Path Path) {
706+
json::ObjectMapper O(Params, Path);
707+
return O && O.map("locationReference", Args.locationReference);
708+
}
709+
710+
llvm::json::Value toJSON(const LocationsResponseBody &Body) {
711+
assert(Body.line != LLDB_INVALID_LINE_NUMBER);
712+
json::Object result{{"source", Body.source}, {"line", Body.line}};
713+
714+
if (Body.column != 0 && Body.column != LLDB_INVALID_COLUMN_NUMBER)
715+
result.insert({"column", Body.column});
716+
if (Body.endLine != 0 && Body.endLine != LLDB_INVALID_LINE_NUMBER)
717+
result.insert({"endLine", Body.endLine});
718+
if (Body.endColumn != 0 && Body.endColumn != LLDB_INVALID_COLUMN_NUMBER)
719+
result.insert({"endColumn", Body.endColumn});
720+
721+
return result;
722+
}
723+
704724
} // namespace lldb_dap::protocol

‎lldb/tools/lldb-dap/Protocol/ProtocolRequests.h‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,6 +1195,41 @@ bool fromJSON(const llvm::json::Value &, PauseArguments &, llvm::json::Path);
11951195
/// field is required.
11961196
using PauseResponse = VoidResponse;
11971197

1198+
/// Arguments for `locations` request.
1199+
struct LocationsArguments {
1200+
/// Location reference to resolve.
1201+
uint64_t locationReference = LLDB_DAP_INVALID_VALUE_LOC;
1202+
};
1203+
bool fromJSON(const llvm::json::Value &, LocationsArguments &,
1204+
llvm::json::Path);
1205+
1206+
/// Response to 'locations' request.
1207+
struct LocationsResponseBody {
1208+
/// The source containing the location; either `source.path` or
1209+
/// `source.sourceReference` must be specified.
1210+
Source source;
1211+
1212+
/// The line number of the location. The client capability `linesStartAt1`
1213+
/// determines whether it is 0- or 1-based.
1214+
uint32_t line = LLDB_INVALID_LINE_NUMBER;
1215+
1216+
/// Position of the location within the `line`. It is measured in UTF-16 code
1217+
/// units and the client capability `columnsStartAt1` determines whether it is
1218+
/// 0- or 1-based. If no column is given, the first position in the start line
1219+
/// is assumed.
1220+
uint32_t column = LLDB_INVALID_COLUMN_NUMBER;
1221+
1222+
/// End line of the location, present if the location refers to a range. The
1223+
/// client capability `linesStartAt1` determines whether it is 0- or 1-based.
1224+
uint32_t endLine = LLDB_INVALID_LINE_NUMBER;
1225+
1226+
/// End position of the location within `endLine`, present if the location
1227+
/// refers to a range. It is measured in UTF-16 code units and the client
1228+
/// capability `columnsStartAt1` determines whether it is 0- or 1-based.
1229+
uint32_t endColumn = LLDB_INVALID_COLUMN_NUMBER;
1230+
};
1231+
llvm::json::Value toJSON(const LocationsResponseBody &);
1232+
11981233
} // namespace lldb_dap::protocol
11991234

12001235
#endif

‎lldb/unittests/DAP/ProtocolRequestsTest.cpp‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,3 +193,53 @@ TEST(ProtocolRequestsTest, PauseRequestArguments) {
193193
EXPECT_THAT_EXPECTED(parse<PauseArguments>(R"({})"),
194194
FailedWithMessage("missing value at (root).threadId"));
195195
}
196+
197+
TEST(ProtocolRequestsTest, LocationsArguments) {
198+
llvm::Expected<LocationsArguments> expected =
199+
parse<LocationsArguments>(R"({"locationReference": 123})");
200+
ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
201+
EXPECT_EQ(expected->locationReference, 123U);
202+
203+
// Check required keys.
204+
EXPECT_THAT_EXPECTED(
205+
parse<LocationsArguments>(R"({})"),
206+
FailedWithMessage("missing value at (root).locationReference"));
207+
}
208+
209+
TEST(ProtocolRequestsTest, LocationsResponseBody) {
210+
LocationsResponseBody body;
211+
body.source.sourceReference = 123;
212+
body.source.name = "test.cpp";
213+
body.line = 42;
214+
215+
// Check required keys.
216+
Expected<json::Value> expected = parse(R"({
217+
"source": {
218+
"sourceReference": 123,
219+
"name": "test.cpp"
220+
},
221+
"line": 42
222+
})");
223+
224+
ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
225+
EXPECT_EQ(PrettyPrint(*expected), PrettyPrint(body));
226+
227+
// Check optional keys.
228+
body.column = 2;
229+
body.endLine = 43;
230+
body.endColumn = 4;
231+
232+
expected = parse(R"({
233+
"source": {
234+
"sourceReference": 123,
235+
"name": "test.cpp"
236+
},
237+
"line": 42,
238+
"column": 2,
239+
"endLine": 43,
240+
"endColumn": 4
241+
})");
242+
243+
ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
244+
EXPECT_EQ(PrettyPrint(*expected), PrettyPrint(body));
245+
}

0 commit comments

Comments
 (0)