Skip to content

Commit 35a2271

Browse files
DrSergeiDrSergei
authored
[lldb-dap] Migrate additional requests to stuctured types (#172283)
These patches update `compileUnits` and `testGetTargetBreakpoints` requests, which are not a part of DAP. I combine two commits into one MR to save maintainers time and because it is mostly NFC.
1 parent c1e829f commit 35a2271

File tree

10 files changed

+158
-92
lines changed

10 files changed

+158
-92
lines changed

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

Lines changed: 17 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -8,75 +8,35 @@
88

99
#include "DAP.h"
1010
#include "EventHelper.h"
11-
#include "JSONUtils.h"
11+
#include "Protocol/ProtocolRequests.h"
1212
#include "RequestHandler.h"
1313

14-
namespace lldb_dap {
14+
using namespace lldb_dap;
15+
using namespace lldb_dap::protocol;
1516

16-
// "compileUnitsRequest": {
17-
// "allOf": [ { "$ref": "#/definitions/Request" }, {
18-
// "type": "object",
19-
// "description": "Compile Unit request; value of command field is
20-
// 'compileUnits'.",
21-
// "properties": {
22-
// "command": {
23-
// "type": "string",
24-
// "enum": [ "compileUnits" ]
25-
// },
26-
// "arguments": {
27-
// "$ref": "#/definitions/compileUnitRequestArguments"
28-
// }
29-
// },
30-
// "required": [ "command", "arguments" ]
31-
// }]
32-
// },
33-
// "compileUnitsRequestArguments": {
34-
// "type": "object",
35-
// "description": "Arguments for 'compileUnits' request.",
36-
// "properties": {
37-
// "moduleId": {
38-
// "type": "string",
39-
// "description": "The ID of the module."
40-
// }
41-
// },
42-
// "required": [ "moduleId" ]
43-
// },
44-
// "compileUnitsResponse": {
45-
// "allOf": [ { "$ref": "#/definitions/Response" }, {
46-
// "type": "object",
47-
// "description": "Response to 'compileUnits' request.",
48-
// "properties": {
49-
// "body": {
50-
// "description": "Response to 'compileUnits' request. Array of
51-
// paths of compile units."
52-
// }
53-
// }
54-
// }]
55-
// }
56-
void CompileUnitsRequestHandler::operator()(
57-
const llvm::json::Object &request) const {
58-
llvm::json::Object response;
59-
FillResponse(request, response);
60-
llvm::json::Object body;
61-
llvm::json::Array units;
62-
const auto *arguments = request.getObject("arguments");
63-
const llvm::StringRef module_id =
64-
GetString(arguments, "moduleId").value_or("");
17+
static CompileUnit CreateCompileUnit(lldb::SBCompileUnit &unit) {
18+
char unit_path_arr[PATH_MAX];
19+
unit.GetFileSpec().GetPath(unit_path_arr, sizeof(unit_path_arr));
20+
std::string unit_path(unit_path_arr);
21+
return {std::move(unit_path)};
22+
}
23+
24+
/// The `compileUnits` request returns an array of path of compile units for
25+
/// given module specified by `moduleId`.
26+
llvm::Expected<CompileUnitsResponseBody> CompileUnitsRequestHandler::Run(
27+
const std::optional<CompileUnitsArguments> &args) const {
28+
std::vector<CompileUnit> units;
6529
int num_modules = dap.target.GetNumModules();
6630
for (int i = 0; i < num_modules; i++) {
6731
auto curr_module = dap.target.GetModuleAtIndex(i);
68-
if (module_id == llvm::StringRef(curr_module.GetUUIDString())) {
32+
if (args->moduleId == llvm::StringRef(curr_module.GetUUIDString())) {
6933
int num_units = curr_module.GetNumCompileUnits();
7034
for (int j = 0; j < num_units; j++) {
7135
auto curr_unit = curr_module.GetCompileUnitAtIndex(j);
7236
units.emplace_back(CreateCompileUnit(curr_unit));
7337
}
74-
body.try_emplace("compileUnits", std::move(units));
7538
break;
7639
}
7740
}
78-
response.try_emplace("body", std::move(body));
79-
dap.SendJSON(llvm::json::Value(std::move(response)));
41+
return CompileUnitsResponseBody{std::move(units)};
8042
}
81-
82-
} // namespace lldb_dap

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

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -472,11 +472,16 @@ class SetInstructionBreakpointsRequestHandler
472472
Run(const protocol::SetInstructionBreakpointsArguments &args) const override;
473473
};
474474

475-
class CompileUnitsRequestHandler : public LegacyRequestHandler {
475+
class CompileUnitsRequestHandler
476+
: public RequestHandler<
477+
std::optional<protocol::CompileUnitsArguments>,
478+
llvm::Expected<protocol::CompileUnitsResponseBody>> {
476479
public:
477-
using LegacyRequestHandler::LegacyRequestHandler;
480+
using RequestHandler::RequestHandler;
478481
static llvm::StringLiteral GetCommand() { return "compileUnits"; }
479-
void operator()(const llvm::json::Object &request) const override;
482+
llvm::Expected<protocol::CompileUnitsResponseBody>
483+
Run(const std::optional<protocol::CompileUnitsArguments> &args)
484+
const override;
480485
};
481486

482487
class ModulesRequestHandler final
@@ -625,17 +630,17 @@ class ModuleSymbolsRequestHandler
625630
Run(const protocol::ModuleSymbolsArguments &args) const override;
626631
};
627632

628-
/// A request used in testing to get the details on all breakpoints that are
629-
/// currently set in the target. This helps us to test "setBreakpoints" and
630-
/// "setFunctionBreakpoints" requests to verify we have the correct set of
631-
/// breakpoints currently set in LLDB.
632-
class TestGetTargetBreakpointsRequestHandler : public LegacyRequestHandler {
633+
class TestGetTargetBreakpointsRequestHandler
634+
: public RequestHandler<
635+
protocol::TestGetTargetBreakpointsArguments,
636+
llvm::Expected<protocol::TestGetTargetBreakpointsResponseBody>> {
633637
public:
634-
using LegacyRequestHandler::LegacyRequestHandler;
638+
using RequestHandler::RequestHandler;
635639
static llvm::StringLiteral GetCommand() {
636640
return "_testGetTargetBreakpoints";
637641
}
638-
void operator()(const llvm::json::Object &request) const override;
642+
llvm::Expected<protocol::TestGetTargetBreakpointsResponseBody>
643+
Run(const protocol::TestGetTargetBreakpointsArguments &args) const override;
639644
};
640645

641646
class WriteMemoryRequestHandler final

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

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,23 @@
88

99
#include "DAP.h"
1010
#include "EventHelper.h"
11-
#include "JSONUtils.h"
11+
#include "Protocol/ProtocolRequests.h"
1212
#include "RequestHandler.h"
1313

14-
namespace lldb_dap {
14+
using namespace lldb_dap;
15+
using namespace lldb_dap::protocol;
1516

16-
void TestGetTargetBreakpointsRequestHandler::operator()(
17-
const llvm::json::Object &request) const {
18-
llvm::json::Object response;
19-
FillResponse(request, response);
20-
llvm::json::Array response_breakpoints;
17+
/// A request used in testing to get the details on all breakpoints that are
18+
/// currently set in the target. This helps us to test "setBreakpoints" and
19+
/// "setFunctionBreakpoints" requests to verify we have the correct set of
20+
/// breakpoints currently set in LLDB.
21+
llvm::Expected<TestGetTargetBreakpointsResponseBody>
22+
TestGetTargetBreakpointsRequestHandler::Run(
23+
const TestGetTargetBreakpointsArguments &args) const {
24+
std::vector<protocol::Breakpoint> breakpoints;
2125
for (uint32_t i = 0; dap.target.GetBreakpointAtIndex(i).IsValid(); ++i) {
2226
auto bp = Breakpoint(dap, dap.target.GetBreakpointAtIndex(i));
23-
response_breakpoints.push_back(bp.ToProtocolBreakpoint());
27+
breakpoints.push_back(bp.ToProtocolBreakpoint());
2428
}
25-
llvm::json::Object body;
26-
body.try_emplace("breakpoints", std::move(response_breakpoints));
27-
response.try_emplace("body", std::move(body));
28-
dap.SendJSON(llvm::json::Value(std::move(response)));
29+
return TestGetTargetBreakpointsResponseBody{std::move(breakpoints)};
2930
}
30-
31-
} // namespace lldb_dap

‎lldb/tools/lldb-dap/JSONUtils.cpp‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -865,15 +865,6 @@ std::pair<int64_t, bool> UnpackLocation(int64_t location_id) {
865865
return std::pair{location_id >> 1, location_id & 1};
866866
}
867867

868-
llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit) {
869-
llvm::json::Object object;
870-
char unit_path_arr[PATH_MAX];
871-
unit.GetFileSpec().GetPath(unit_path_arr, sizeof(unit_path_arr));
872-
std::string unit_path(unit_path_arr);
873-
object.try_emplace("compileUnitPath", unit_path);
874-
return llvm::json::Value(std::move(object));
875-
}
876-
877868
/// See
878869
/// https://microsoft.github.io/debug-adapter-protocol/specification#Reverse_Requests_RunInTerminal
879870
llvm::json::Object CreateRunInTerminalReverseRequest(

‎lldb/tools/lldb-dap/JSONUtils.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,8 +368,6 @@ int64_t PackLocation(int64_t var_ref, bool is_value_location);
368368
/// Reverse of `PackLocation`
369369
std::pair<int64_t, bool> UnpackLocation(int64_t location_id);
370370

371-
llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit);
372-
373371
/// Create a runInTerminal reverse request object
374372
///
375373
/// \param[in] program

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -721,4 +721,20 @@ llvm::json::Value toJSON(const LocationsResponseBody &Body) {
721721
return result;
722722
}
723723

724+
bool fromJSON(const llvm::json::Value &Params, CompileUnitsArguments &Args,
725+
llvm::json::Path Path) {
726+
json::ObjectMapper O(Params, Path);
727+
return O && O.map("moduleId", Args.moduleId);
728+
}
729+
730+
llvm::json::Value toJSON(const CompileUnitsResponseBody &Body) {
731+
json::Object result{{"compileUnits", Body.compileUnits}};
732+
return result;
733+
}
734+
735+
llvm::json::Value toJSON(const TestGetTargetBreakpointsResponseBody &Body) {
736+
json::Object result{{"breakpoints", Body.breakpoints}};
737+
return result;
738+
}
739+
724740
} // namespace lldb_dap::protocol

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1230,6 +1230,31 @@ struct LocationsResponseBody {
12301230
};
12311231
llvm::json::Value toJSON(const LocationsResponseBody &);
12321232

1233+
/// Arguments for `compileUnits` request.
1234+
struct CompileUnitsArguments {
1235+
/// The ID of the module.
1236+
std::string moduleId;
1237+
};
1238+
bool fromJSON(const llvm::json::Value &, CompileUnitsArguments &,
1239+
llvm::json::Path);
1240+
1241+
/// Response to `compileUnits` request.
1242+
struct CompileUnitsResponseBody {
1243+
/// Array of compile units.
1244+
std::vector<CompileUnit> compileUnits;
1245+
};
1246+
llvm::json::Value toJSON(const CompileUnitsResponseBody &);
1247+
1248+
/// Arguments for `testGetTargetBreakpoints` request.
1249+
using TestGetTargetBreakpointsArguments = EmptyArguments;
1250+
1251+
/// Response to `testGetTargetBreakpoints` request.
1252+
struct TestGetTargetBreakpointsResponseBody {
1253+
/// Array of all breakpoints that are currently set in the target.
1254+
std::vector<Breakpoint> breakpoints;
1255+
};
1256+
llvm::json::Value toJSON(const TestGetTargetBreakpointsResponseBody &);
1257+
12331258
} // namespace lldb_dap::protocol
12341259

12351260
#endif

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,4 +1169,9 @@ json::Value toJSON(const ExceptionDetails &ED) {
11691169
return result;
11701170
}
11711171

1172+
llvm::json::Value toJSON(const CompileUnit &CU) {
1173+
json::Object result{{"compileUnitPath", CU.compileUnitPath}};
1174+
return result;
1175+
}
1176+
11721177
} // namespace lldb_dap::protocol

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,6 +1038,12 @@ struct ExceptionDetails {
10381038
};
10391039
llvm::json::Value toJSON(const ExceptionDetails &);
10401040

1041+
struct CompileUnit {
1042+
/// Path of compile unit.
1043+
std::string compileUnitPath;
1044+
};
1045+
llvm::json::Value toJSON(const CompileUnit &);
1046+
10411047
} // namespace lldb_dap::protocol
10421048

10431049
#endif

‎lldb/unittests/DAP/ProtocolRequestsTest.cpp‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,3 +243,64 @@ TEST(ProtocolRequestsTest, LocationsResponseBody) {
243243
ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
244244
EXPECT_EQ(PrettyPrint(*expected), PrettyPrint(body));
245245
}
246+
247+
TEST(ProtocolRequestsTest, CompileUnitsArguments) {
248+
llvm::Expected<CompileUnitsArguments> expected =
249+
parse<CompileUnitsArguments>(R"({"moduleId": "42"})");
250+
ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
251+
EXPECT_EQ(expected->moduleId, "42");
252+
253+
// Check required keys.
254+
EXPECT_THAT_EXPECTED(parse<CompileUnitsArguments>(R"({})"),
255+
FailedWithMessage("missing value at (root).moduleId"));
256+
}
257+
258+
TEST(ProtocolRequestsTest, CompileUnitsResponseBody) {
259+
CompileUnitsResponseBody body;
260+
body.compileUnits = {{"main.cpp"}, {"util.cpp"}};
261+
262+
// Check required keys.
263+
Expected<json::Value> expected = parse(R"({
264+
"compileUnits": [
265+
{
266+
"compileUnitPath": "main.cpp"
267+
},
268+
{
269+
"compileUnitPath": "util.cpp"
270+
}
271+
]
272+
})");
273+
274+
ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
275+
EXPECT_EQ(PrettyPrint(*expected), PrettyPrint(body));
276+
}
277+
278+
TEST(ProtocolRequestsTest, TestGetTargetBreakpointsResponseBody) {
279+
Breakpoint breakpoint1;
280+
breakpoint1.id = 1;
281+
breakpoint1.verified = true;
282+
Breakpoint breakpoint2;
283+
breakpoint2.id = 2;
284+
breakpoint2.verified = false;
285+
breakpoint2.message = "Failed to set breakpoint";
286+
TestGetTargetBreakpointsResponseBody body;
287+
body.breakpoints = {breakpoint1, breakpoint2};
288+
289+
// Check required keys.
290+
Expected<json::Value> expected = parse(R"({
291+
"breakpoints": [
292+
{
293+
"id": 1,
294+
"verified": true
295+
},
296+
{
297+
"id": 2,
298+
"verified": false,
299+
"message": "Failed to set breakpoint"
300+
}
301+
]
302+
})");
303+
304+
ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
305+
EXPECT_EQ(PrettyPrint(*expected), PrettyPrint(body));
306+
}

0 commit comments

Comments
 (0)