[lldb-dap] Migrate additional requests to stuctured types #172283
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-lldb Author: Sergei Druzhkov (DrSergei) ChangesThese patches update Full diff: https://github.com/llvm/llvm-project/pull/172283.diff 10 Files Affected:
diff --git a/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp
index 0e5c2b23d8d67..e79000230de1f 100644
--- a/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp
@@ -8,75 +8,35 @@
#include "DAP.h"
#include "EventHelper.h"
-#include "JSONUtils.h"
+#include "Protocol/ProtocolRequests.h"
#include "RequestHandler.h"
-namespace lldb_dap {
+using namespace lldb_dap;
+using namespace lldb_dap::protocol;
-// "compileUnitsRequest": {
-// "allOf": [ { "$ref": "#/definitions/Request" }, {
-// "type": "object",
-// "description": "Compile Unit request; value of command field is
-// 'compileUnits'.",
-// "properties": {
-// "command": {
-// "type": "string",
-// "enum": [ "compileUnits" ]
-// },
-// "arguments": {
-// "$ref": "#/definitions/compileUnitRequestArguments"
-// }
-// },
-// "required": [ "command", "arguments" ]
-// }]
-// },
-// "compileUnitsRequestArguments": {
-// "type": "object",
-// "description": "Arguments for 'compileUnits' request.",
-// "properties": {
-// "moduleId": {
-// "type": "string",
-// "description": "The ID of the module."
-// }
-// },
-// "required": [ "moduleId" ]
-// },
-// "compileUnitsResponse": {
-// "allOf": [ { "$ref": "#/definitions/Response" }, {
-// "type": "object",
-// "description": "Response to 'compileUnits' request.",
-// "properties": {
-// "body": {
-// "description": "Response to 'compileUnits' request. Array of
-// paths of compile units."
-// }
-// }
-// }]
-// }
-void CompileUnitsRequestHandler::operator()(
- const llvm::json::Object &request) const {
- llvm::json::Object response;
- FillResponse(request, response);
- llvm::json::Object body;
- llvm::json::Array units;
- const auto *arguments = request.getObject("arguments");
- const llvm::StringRef module_id =
- GetString(arguments, "moduleId").value_or("");
+static CompileUnit CreateCompileUnit(lldb::SBCompileUnit &unit) {
+ char unit_path_arr[PATH_MAX];
+ unit.GetFileSpec().GetPath(unit_path_arr, sizeof(unit_path_arr));
+ std::string unit_path(unit_path_arr);
+ return {std::move(unit_path)};
+}
+
+/// The `compileUnits` request returns an array of path of compile units for
+/// given module specified by `moduleId`.
+llvm::Expected<CompileUnitsResponseBody> CompileUnitsRequestHandler::Run(
+ const std::optional<CompileUnitsArguments> &args) const {
+ std::vector<CompileUnit> units;
int num_modules = dap.target.GetNumModules();
for (int i = 0; i < num_modules; i++) {
auto curr_module = dap.target.GetModuleAtIndex(i);
- if (module_id == llvm::StringRef(curr_module.GetUUIDString())) {
+ if (args->moduleId == llvm::StringRef(curr_module.GetUUIDString())) {
int num_units = curr_module.GetNumCompileUnits();
for (int j = 0; j < num_units; j++) {
auto curr_unit = curr_module.GetCompileUnitAtIndex(j);
units.emplace_back(CreateCompileUnit(curr_unit));
}
- body.try_emplace("compileUnits", std::move(units));
break;
}
}
- response.try_emplace("body", std::move(body));
- dap.SendJSON(llvm::json::Value(std::move(response)));
-}
-
-} // namespace lldb_dap
+ return CompileUnitsResponseBody{std::move(units)};
+};
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index 8f42a28160751..0669be50597e9 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -472,11 +472,16 @@ class SetInstructionBreakpointsRequestHandler
Run(const protocol::SetInstructionBreakpointsArguments &args) const override;
};
-class CompileUnitsRequestHandler : public LegacyRequestHandler {
+class CompileUnitsRequestHandler
+ : public RequestHandler<
+ std::optional<protocol::CompileUnitsArguments>,
+ llvm::Expected<protocol::CompileUnitsResponseBody>> {
public:
- using LegacyRequestHandler::LegacyRequestHandler;
+ using RequestHandler::RequestHandler;
static llvm::StringLiteral GetCommand() { return "compileUnits"; }
- void operator()(const llvm::json::Object &request) const override;
+ llvm::Expected<protocol::CompileUnitsResponseBody>
+ Run(const std::optional<protocol::CompileUnitsArguments> &args)
+ const override;
};
class ModulesRequestHandler final
@@ -625,17 +630,17 @@ class ModuleSymbolsRequestHandler
Run(const protocol::ModuleSymbolsArguments &args) const override;
};
-/// A request used in testing to get the details on all breakpoints that are
-/// currently set in the target. This helps us to test "setBreakpoints" and
-/// "setFunctionBreakpoints" requests to verify we have the correct set of
-/// breakpoints currently set in LLDB.
-class TestGetTargetBreakpointsRequestHandler : public LegacyRequestHandler {
+class TestGetTargetBreakpointsRequestHandler
+ : public RequestHandler<
+ protocol::TestGetTargetBreakpointsArguments,
+ llvm::Expected<protocol::TestGetTargetBreakpointsResponseBody>> {
public:
- using LegacyRequestHandler::LegacyRequestHandler;
+ using RequestHandler::RequestHandler;
static llvm::StringLiteral GetCommand() {
return "_testGetTargetBreakpoints";
}
- void operator()(const llvm::json::Object &request) const override;
+ llvm::Expected<protocol::TestGetTargetBreakpointsResponseBody>
+ Run(const protocol::TestGetTargetBreakpointsArguments &args) const override;
};
class WriteMemoryRequestHandler final
diff --git a/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp
index 5f4f016f6a1ef..d1b6a8e189032 100644
--- a/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp
@@ -8,24 +8,23 @@
#include "DAP.h"
#include "EventHelper.h"
-#include "JSONUtils.h"
+#include "Protocol/ProtocolRequests.h"
#include "RequestHandler.h"
-namespace lldb_dap {
+using namespace lldb_dap;
+using namespace lldb_dap::protocol;
-void TestGetTargetBreakpointsRequestHandler::operator()(
- const llvm::json::Object &request) const {
- llvm::json::Object response;
- FillResponse(request, response);
- llvm::json::Array response_breakpoints;
+/// A request used in testing to get the details on all breakpoints that are
+/// currently set in the target. This helps us to test "setBreakpoints" and
+/// "setFunctionBreakpoints" requests to verify we have the correct set of
+/// breakpoints currently set in LLDB.
+llvm::Expected<TestGetTargetBreakpointsResponseBody>
+TestGetTargetBreakpointsRequestHandler::Run(
+ const TestGetTargetBreakpointsArguments &args) const {
+ std::vector<protocol::Breakpoint> breakpoints;
for (uint32_t i = 0; dap.target.GetBreakpointAtIndex(i).IsValid(); ++i) {
auto bp = Breakpoint(dap, dap.target.GetBreakpointAtIndex(i));
- response_breakpoints.push_back(bp.ToProtocolBreakpoint());
+ breakpoints.push_back(bp.ToProtocolBreakpoint());
}
- llvm::json::Object body;
- body.try_emplace("breakpoints", std::move(response_breakpoints));
- response.try_emplace("body", std::move(body));
- dap.SendJSON(llvm::json::Value(std::move(response)));
+ return TestGetTargetBreakpointsResponseBody{std::move(breakpoints)};
}
-
-} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index bb3f8aaaded89..1f9719110cedb 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -865,15 +865,6 @@ std::pair<int64_t, bool> UnpackLocation(int64_t location_id) {
return std::pair{location_id >> 1, location_id & 1};
}
-llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit) {
- llvm::json::Object object;
- char unit_path_arr[PATH_MAX];
- unit.GetFileSpec().GetPath(unit_path_arr, sizeof(unit_path_arr));
- std::string unit_path(unit_path_arr);
- object.try_emplace("compileUnitPath", unit_path);
- return llvm::json::Value(std::move(object));
-}
-
/// See
/// https://microsoft.github.io/debug-adapter-protocol/specification#Reverse_Requests_RunInTerminal
llvm::json::Object CreateRunInTerminalReverseRequest(
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index 7fad7409b11fe..0a907599fc9ee 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -368,8 +368,6 @@ int64_t PackLocation(int64_t var_ref, bool is_value_location);
/// Reverse of `PackLocation`
std::pair<int64_t, bool> UnpackLocation(int64_t location_id);
-llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit);
-
/// Create a runInTerminal reverse request object
///
/// \param[in] program
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index 42acc70333a7b..bf470b78077df 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -721,4 +721,20 @@ llvm::json::Value toJSON(const LocationsResponseBody &Body) {
return result;
}
+bool fromJSON(const llvm::json::Value &Params, CompileUnitsArguments &Args,
+ llvm::json::Path Path) {
+ json::ObjectMapper O(Params, Path);
+ return O && O.map("moduleId", Args.moduleId);
+}
+
+llvm::json::Value toJSON(const CompileUnitsResponseBody &Body) {
+ json::Object result{{"compileUnits", Body.compileUnits}};
+ return result;
+}
+
+llvm::json::Value toJSON(const TestGetTargetBreakpointsResponseBody &Body) {
+ json::Object result{{"breakpoints", Body.breakpoints}};
+ return result;
+}
+
} // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index 104520f2c24c2..cc123943ec0b6 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -1230,6 +1230,31 @@ struct LocationsResponseBody {
};
llvm::json::Value toJSON(const LocationsResponseBody &);
+/// Arguments for `compileUnits` request.
+struct CompileUnitsArguments {
+ /// The ID of the module.
+ std::string moduleId;
+};
+bool fromJSON(const llvm::json::Value &, CompileUnitsArguments &,
+ llvm::json::Path);
+
+/// Response to `compileUnits` request.
+struct CompileUnitsResponseBody {
+ /// Array of compile units.
+ std::vector<CompileUnit> compileUnits;
+};
+llvm::json::Value toJSON(const CompileUnitsResponseBody &);
+
+/// Arguments for `testGetTargetBreakpoints` request.
+using TestGetTargetBreakpointsArguments = EmptyArguments;
+
+/// Response to `testGetTargetBreakpoints` request.
+struct TestGetTargetBreakpointsResponseBody {
+ /// Array of all breakpoints that are currently set in the target.
+ std::vector<Breakpoint> breakpoints;
+};
+llvm::json::Value toJSON(const TestGetTargetBreakpointsResponseBody &);
+
} // namespace lldb_dap::protocol
#endif
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
index 95007013742a0..c7f7c447b5b6f 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
@@ -1169,4 +1169,9 @@ json::Value toJSON(const ExceptionDetails &ED) {
return result;
}
+llvm::json::Value toJSON(const CompileUnit &CU) {
+ json::Object result{{"compileUnitPath", CU.compileUnitPath}};
+ return result;
+}
+
} // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
index ee103ddf0b7a2..4ead4786bc661 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
@@ -1038,6 +1038,12 @@ struct ExceptionDetails {
};
llvm::json::Value toJSON(const ExceptionDetails &);
+struct CompileUnit {
+ /// Path of compile unit.
+ std::string compileUnitPath;
+};
+llvm::json::Value toJSON(const CompileUnit &);
+
} // namespace lldb_dap::protocol
#endif
diff --git a/lldb/unittests/DAP/ProtocolRequestsTest.cpp b/lldb/unittests/DAP/ProtocolRequestsTest.cpp
index 50b8335ba46cc..710b78960ef09 100644
--- a/lldb/unittests/DAP/ProtocolRequestsTest.cpp
+++ b/lldb/unittests/DAP/ProtocolRequestsTest.cpp
@@ -243,3 +243,64 @@ TEST(ProtocolRequestsTest, LocationsResponseBody) {
ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
EXPECT_EQ(PrettyPrint(*expected), PrettyPrint(body));
}
+
+TEST(ProtocolRequestsTest, CompileUnitsArguments) {
+ llvm::Expected<CompileUnitsArguments> expected =
+ parse<CompileUnitsArguments>(R"({"moduleId": "42"})");
+ ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
+ EXPECT_EQ(expected->moduleId, "42");
+
+ // Check required keys.
+ EXPECT_THAT_EXPECTED(parse<CompileUnitsArguments>(R"({})"),
+ FailedWithMessage("missing value at (root).moduleId"));
+}
+
+TEST(ProtocolRequestsTest, CompileUnitsResponseBody) {
+ CompileUnitsResponseBody body;
+ body.compileUnits = {{"main.cpp"}, {"util.cpp"}};
+
+ // Check required keys.
+ Expected<json::Value> expected = parse(R"({
+ "compileUnits": [
+ {
+ "compileUnitPath": "main.cpp"
+ },
+ {
+ "compileUnitPath": "util.cpp"
+ }
+ ]
+ })");
+
+ ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
+ EXPECT_EQ(PrettyPrint(*expected), PrettyPrint(body));
+}
+
+TEST(ProtocolRequestsTest, TestGetTargetBreakpointsResponseBody) {
+ Breakpoint breakpoint1;
+ breakpoint1.id = 1;
+ breakpoint1.verified = true;
+ Breakpoint breakpoint2;
+ breakpoint2.id = 2;
+ breakpoint2.verified = false;
+ breakpoint2.message = "Failed to set breakpoint";
+ TestGetTargetBreakpointsResponseBody body;
+ body.breakpoints = {breakpoint1, breakpoint2};
+
+ // Check required keys.
+ Expected<json::Value> expected = parse(R"({
+ "breakpoints": [
+ {
+ "id": 1,
+ "verified": true
+ },
+ {
+ "id": 2,
+ "verified": false,
+ "message": "Failed to set breakpoint"
+ }
+ ]
+ })");
+
+ ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
+ EXPECT_EQ(PrettyPrint(*expected), PrettyPrint(body));
+}
|
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
ashgti
approved these changes
Dec 15, 2025
Contributor
ashgti
left a comment
ashgti
left a comment
There was a problem hiding this comment.
Awesome! We've almost removed all the LegacyRequestHandler instances now!
JDevlieghere
approved these changes
Dec 15, 2025
DrSergei
added a commit
that referenced
this pull request
Dec 16, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These patches update
compileUnitsandtestGetTargetBreakpointsrequests, which are not a part of DAP. I combine two commits into one MR to save maintainers time and because it is mostly NFC.