[Lldb-commits] [lldb] [lldb-dap] Migrate restart request to structured types (PR #172488)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Dec 16 06:56:27 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Sergei Druzhkov (DrSergei)
<details>
<summary>Changes</summary>
This patch migrates `restart` request to structured types. Also, I added some checks that at least one of the required fields was provided for `launch` and `attach` requests. Maybe I missed some possible configurations, so please double check.
---
Full diff: https://github.com/llvm/llvm-project/pull/172488.diff
5 Files Affected:
- (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+6-3)
- (modified) lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp (+27-90)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+68-20)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+13)
- (modified) lldb/unittests/DAP/ProtocolRequestsTest.cpp (+40)
``````````diff
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index 0669be50597e9..a18a8049c804d 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -340,11 +340,14 @@ class LaunchRequestHandler
void PostRun() const override;
};
-class RestartRequestHandler : public LegacyRequestHandler {
+class RestartRequestHandler
+ : public RequestHandler<std::optional<protocol::RestartArguments>,
+ protocol::RestartResponse> {
public:
- using LegacyRequestHandler::LegacyRequestHandler;
+ using RequestHandler::RequestHandler;
static llvm::StringLiteral GetCommand() { return "restart"; }
- void operator()(const llvm::json::Object &request) const override;
+ llvm::Error
+ Run(const std::optional<protocol::RestartArguments> &args) const override;
};
class NextRequestHandler
diff --git a/lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp
index 100173bfc3082..3a2be3f30c7cf 100644
--- a/lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp
@@ -7,91 +7,37 @@
//===----------------------------------------------------------------------===//
#include "DAP.h"
+#include "DAPError.h"
#include "EventHelper.h"
-#include "JSONUtils.h"
#include "LLDBUtils.h"
#include "Protocol/ProtocolRequests.h"
#include "RequestHandler.h"
-#include "llvm/Support/JSON.h"
-#include "llvm/Support/raw_ostream.h"
-namespace lldb_dap {
+using namespace lldb_dap;
+using namespace lldb_dap::protocol;
-// "RestartRequest": {
-// "allOf": [ { "$ref": "#/definitions/Request" }, {
-// "type": "object",
-// "description": "Restarts a debug session. Clients should only call this
-// request if the corresponding capability `supportsRestartRequest` is
-// true.\nIf the capability is missing or has the value false, a typical
-// client emulates `restart` by terminating the debug adapter first and then
-// launching it anew.",
-// "properties": {
-// "command": {
-// "type": "string",
-// "enum": [ "restart" ]
-// },
-// "arguments": {
-// "$ref": "#/definitions/RestartArguments"
-// }
-// },
-// "required": [ "command" ]
-// }]
-// },
-// "RestartArguments": {
-// "type": "object",
-// "description": "Arguments for `restart` request.",
-// "properties": {
-// "arguments": {
-// "oneOf": [
-// { "$ref": "#/definitions/LaunchRequestArguments" },
-// { "$ref": "#/definitions/AttachRequestArguments" }
-// ],
-// "description": "The latest version of the `launch` or `attach`
-// configuration."
-// }
-// }
-// },
-// "RestartResponse": {
-// "allOf": [ { "$ref": "#/definitions/Response" }, {
-// "type": "object",
-// "description": "Response to `restart` request. This is just an
-// acknowledgement, so no body field is required."
-// }]
-// },
-void RestartRequestHandler::operator()(
- const llvm::json::Object &request) const {
- llvm::json::Object response;
- FillResponse(request, response);
- if (!dap.target.GetProcess().IsValid()) {
- response["success"] = llvm::json::Value(false);
- EmplaceSafeString(response, "message",
- "Restart request received but no process was launched.");
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
+/// Restarts a debug session. Clients should only call this request if the
+/// corresponding capability `supportsRestartRequest` is true.
+/// If the capability is missing or has the value false, a typical client
+/// emulates `restart` by terminating the debug adapter first and then launching
+/// it anew.
+llvm::Error
+RestartRequestHandler::Run(const std::optional<RestartArguments> &args) const {
+ if (!dap.target.GetProcess().IsValid())
+ return llvm::make_error<DAPError>(
+ "Restart request received but no process was launched.");
- const llvm::json::Object *arguments = request.getObject("arguments");
- if (arguments) {
- // The optional `arguments` field in RestartRequest can contain an updated
- // version of the launch arguments. If there's one, use it.
- if (const llvm::json::Value *restart_arguments =
- arguments->get("arguments")) {
- protocol::LaunchRequestArguments updated_arguments;
- llvm::json::Path::Root root;
- if (!fromJSON(*restart_arguments, updated_arguments, root)) {
- response["success"] = llvm::json::Value(false);
- EmplaceSafeString(
- response, "message",
- llvm::formatv("Failed to parse updated launch arguments: {0}",
- llvm::toString(root.getError()))
- .str());
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
- dap.last_launch_request = updated_arguments;
+ if (args) {
+ if (std::holds_alternative<AttachRequestArguments>(args->arguments))
+ return llvm::make_error<DAPError>(
+ "Restarting an AttachRequest is not supported.");
+ if (const auto *arguments =
+ std::get_if<LaunchRequestArguments>(&args->arguments);
+ arguments) {
+ dap.last_launch_request = *arguments;
// Update DAP configuration based on the latest copy of the launch
// arguments.
- dap.SetConfiguration(updated_arguments.configuration, false);
+ dap.SetConfiguration(arguments->configuration, false);
dap.ConfigureSourceMaps();
}
}
@@ -117,12 +63,8 @@ void RestartRequestHandler::operator()(
// FIXME: Should we run 'preRunCommands'?
// FIXME: Should we add a 'preRestartCommands'?
- if (llvm::Error err = LaunchProcess(*dap.last_launch_request)) {
- response["success"] = llvm::json::Value(false);
- EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
+ if (llvm::Error err = LaunchProcess(*dap.last_launch_request))
+ return llvm::make_error<DAPError>(llvm::toString(std::move(err)));
SendProcessEvent(dap, Launch);
@@ -130,16 +72,11 @@ void RestartRequestHandler::operator()(
// Because we're restarting, configuration has already happened so we can
// continue the process right away.
if (dap.stop_at_entry) {
- if (llvm::Error err = SendThreadStoppedEvent(dap, /*on_entry=*/true)) {
- EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
+ if (llvm::Error err = SendThreadStoppedEvent(dap, /*on_entry=*/true))
+ return llvm::make_error<DAPError>(llvm::toString(std::move(err)));
} else {
dap.target.GetProcess().Continue();
}
- dap.SendJSON(llvm::json::Value(std::move(response)));
+ return llvm::Error::success();
}
-
-} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index bf470b78077df..1c2cd158ffd29 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -296,31 +296,53 @@ bool fromJSON(const json::Value &Params, Console &C, json::Path P) {
bool fromJSON(const json::Value &Params, LaunchRequestArguments &LRA,
json::Path P) {
json::ObjectMapper O(Params, P);
- return O && fromJSON(Params, LRA.configuration, P) &&
- O.mapOptional("noDebug", LRA.noDebug) &&
- O.mapOptional("launchCommands", LRA.launchCommands) &&
- O.mapOptional("cwd", LRA.cwd) && O.mapOptional("args", LRA.args) &&
- O.mapOptional("detachOnError", LRA.detachOnError) &&
- O.mapOptional("disableASLR", LRA.disableASLR) &&
- O.mapOptional("disableSTDIO", LRA.disableSTDIO) &&
- O.mapOptional("shellExpandArguments", LRA.shellExpandArguments) &&
- O.mapOptional("runInTerminal", LRA.console) &&
- O.mapOptional("console", LRA.console) &&
- O.mapOptional("stdio", LRA.stdio) && parseEnv(Params, LRA.env, P);
+ bool success =
+ O && fromJSON(Params, LRA.configuration, P) &&
+ O.mapOptional("noDebug", LRA.noDebug) &&
+ O.mapOptional("launchCommands", LRA.launchCommands) &&
+ O.mapOptional("cwd", LRA.cwd) && O.mapOptional("args", LRA.args) &&
+ O.mapOptional("detachOnError", LRA.detachOnError) &&
+ O.mapOptional("disableASLR", LRA.disableASLR) &&
+ O.mapOptional("disableSTDIO", LRA.disableSTDIO) &&
+ O.mapOptional("shellExpandArguments", LRA.shellExpandArguments) &&
+ O.mapOptional("runInTerminal", LRA.console) &&
+ O.mapOptional("console", LRA.console) &&
+ O.mapOptional("stdio", LRA.stdio) && parseEnv(Params, LRA.env, P);
+ if (!success)
+ return false;
+ if (LRA.configuration.program.empty() && LRA.launchCommands.empty()) {
+ P.report("`program` or `launchCommands` should be provided");
+ return false;
+ }
+ return true;
}
bool fromJSON(const json::Value &Params, AttachRequestArguments &ARA,
json::Path P) {
json::ObjectMapper O(Params, P);
- return O && fromJSON(Params, ARA.configuration, P) &&
- O.mapOptional("attachCommands", ARA.attachCommands) &&
- O.mapOptional("pid", ARA.pid) &&
- O.mapOptional("waitFor", ARA.waitFor) &&
- O.mapOptional("gdb-remote-port", ARA.gdbRemotePort) &&
- O.mapOptional("gdb-remote-hostname", ARA.gdbRemoteHostname) &&
- O.mapOptional("coreFile", ARA.coreFile) &&
- O.mapOptional("targetId", ARA.targetId) &&
- O.mapOptional("debuggerId", ARA.debuggerId);
+ bool success = O && fromJSON(Params, ARA.configuration, P) &&
+ O.mapOptional("attachCommands", ARA.attachCommands) &&
+ O.mapOptional("pid", ARA.pid) &&
+ O.mapOptional("waitFor", ARA.waitFor) &&
+ O.mapOptional("gdb-remote-port", ARA.gdbRemotePort) &&
+ O.mapOptional("gdb-remote-hostname", ARA.gdbRemoteHostname) &&
+ O.mapOptional("coreFile", ARA.coreFile) &&
+ O.mapOptional("targetId", ARA.targetId) &&
+ O.mapOptional("debuggerId", ARA.debuggerId);
+ if (!success)
+ return false;
+ if (ARA.debuggerId.has_value())
+ return true;
+ if (ARA.targetId.has_value())
+ return true;
+ if ((ARA.pid == LLDB_INVALID_PROCESS_ID) &&
+ ARA.configuration.program.empty() && ARA.attachCommands.empty() &&
+ ARA.coreFile.empty() && (ARA.gdbRemotePort == LLDB_DAP_INVALID_PORT)) {
+ P.report("`pid` or `program` or `attachCommands` or `coreFile` or "
+ "`gdbRemotePort` should be provided");
+ return false;
+ }
+ return true;
}
bool fromJSON(const json::Value &Params, ContinueArguments &CA, json::Path P) {
@@ -737,4 +759,30 @@ llvm::json::Value toJSON(const TestGetTargetBreakpointsResponseBody &Body) {
return result;
}
+bool fromJSON(const llvm::json::Value &Params, RestartArguments &Args,
+ llvm::json::Path Path) {
+ const json::Object *O = Params.getAsObject();
+ if (!O) {
+ Path.report("expected object");
+ return false;
+ }
+ const json::Value *arguments = O->get("arguments");
+ if (arguments == nullptr)
+ return true;
+ LaunchRequestArguments launchArguments;
+ llvm::json::Path::Root root;
+ if (fromJSON(*arguments, launchArguments, root)) {
+ Args.arguments = std::move(launchArguments);
+ return true;
+ }
+ AttachRequestArguments attachArguments;
+ if (fromJSON(*arguments, attachArguments, root)) {
+ Args.arguments = std::move(attachArguments);
+ return true;
+ }
+ Path.report(
+ "failed to parse arguments, expected `launch` or `attach` arguments");
+ return false;
+}
+
} // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index cc123943ec0b6..33fcaae1710b5 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -31,6 +31,7 @@
#include <cstdint>
#include <optional>
#include <string>
+#include <variant>
#include <vector>
namespace lldb_dap::protocol {
@@ -1255,6 +1256,18 @@ struct TestGetTargetBreakpointsResponseBody {
};
llvm::json::Value toJSON(const TestGetTargetBreakpointsResponseBody &);
+/// Arguments for `restart` request.
+struct RestartArguments {
+ /// The latest version of the `launch` or `attach` configuration.
+ std::variant<std::monostate, LaunchRequestArguments, AttachRequestArguments>
+ arguments = std::monostate{};
+};
+bool fromJSON(const llvm::json::Value &, RestartArguments &, llvm::json::Path);
+
+/// Response to `restart` request. This is just an acknowledgement, so no body
+/// field is required.
+using RestartResponse = VoidResponse;
+
} // namespace lldb_dap::protocol
#endif
diff --git a/lldb/unittests/DAP/ProtocolRequestsTest.cpp b/lldb/unittests/DAP/ProtocolRequestsTest.cpp
index 710b78960ef09..c639e40453fb0 100644
--- a/lldb/unittests/DAP/ProtocolRequestsTest.cpp
+++ b/lldb/unittests/DAP/ProtocolRequestsTest.cpp
@@ -304,3 +304,43 @@ TEST(ProtocolRequestsTest, TestGetTargetBreakpointsResponseBody) {
ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
EXPECT_EQ(PrettyPrint(*expected), PrettyPrint(body));
}
+
+TEST(ProtocolRequestsTest, RestartArguments) {
+ llvm::Expected<RestartArguments> expected = parse<RestartArguments>(R"({})");
+ ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
+ EXPECT_TRUE(std::holds_alternative<std::monostate>(expected->arguments));
+
+ // Check missed keys.
+ expected = parse<RestartArguments>(R"({
+ "arguments": {}
+ })");
+ EXPECT_THAT_EXPECTED(expected,
+ FailedWithMessage("failed to parse arguments, expected "
+ "`launch` or `attach` arguments"));
+
+ // Check launch arguments.
+ expected = parse<RestartArguments>(R"({
+ "arguments": {
+ "program": "main.exe",
+ "cwd": "/home/root"
+ }
+ })");
+ ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
+ const LaunchRequestArguments *launch_args =
+ std::get_if<LaunchRequestArguments>(&expected->arguments);
+ EXPECT_NE(launch_args, nullptr);
+ EXPECT_EQ(launch_args->configuration.program, "main.exe");
+ EXPECT_EQ(launch_args->cwd, "/home/root");
+
+ // Check attach arguments.
+ expected = parse<RestartArguments>(R"({
+ "arguments": {
+ "pid": 123
+ }
+ })");
+ ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
+ const AttachRequestArguments *attach_args =
+ std::get_if<AttachRequestArguments>(&expected->arguments);
+ EXPECT_NE(attach_args, nullptr);
+ EXPECT_EQ(attach_args->pid, 123U);
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/172488
More information about the lldb-commits
mailing list