[Lldb-commits] [lldb] [lldb-dap] Updating RequestHandler to encode/decode arguments and response. (PR #130090)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Mar 12 13:34:17 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: John Harrison (ashgti)
<details>
<summary>Changes</summary>
This is a work in progress refactor to add explicit types instead of
generic 'llvm::json::Value' types to the DAP protocol.
This updates RequestHandler to have take the type of the arguments and
response body for serialization for requests.
The 'source' request is updated to show how the new flow works.
This is built on top of #<!-- -->130026
---
Patch is 43.12 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/130090.diff
8 Files Affected:
- (modified) lldb/tools/lldb-dap/DAP.cpp (+34-23)
- (modified) lldb/tools/lldb-dap/DAP.h (+1-3)
- (modified) lldb/tools/lldb-dap/DAPForward.h (+2)
- (modified) lldb/tools/lldb-dap/Handler/RequestHandler.cpp (+6-5)
- (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+139-75)
- (modified) lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp (+26-97)
- (modified) lldb/tools/lldb-dap/Protocol.cpp (+44)
- (modified) lldb/tools/lldb-dap/Protocol.h (+117-163)
``````````diff
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 4080e2c211035..9d42af2d7091f 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -8,10 +8,12 @@
#include "DAP.h"
#include "DAPLog.h"
+#include "Handler/RequestHandler.h"
#include "Handler/ResponseHandler.h"
#include "JSONUtils.h"
#include "LLDBUtils.h"
#include "OutputRedirector.h"
+#include "Protocol.h"
#include "Transport.h"
#include "lldb/API/SBBreakpoint.h"
#include "lldb/API/SBCommandInterpreter.h"
@@ -25,6 +27,7 @@
#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
@@ -41,6 +44,7 @@
#include <fstream>
#include <memory>
#include <mutex>
+#include <string>
#include <utility>
#if defined(_WIN32)
@@ -663,31 +667,23 @@ void DAP::SetTarget(const lldb::SBTarget target) {
}
bool DAP::HandleObject(const protocol::Message &M) {
- // FIXME: Directly handle `Message` instead of serializing to JSON.
- llvm::json::Value v = toJSON(M);
- llvm::json::Object object = *v.getAsObject();
- const auto packet_type = GetString(object, "type");
- if (packet_type == "request") {
- const auto command = GetString(object, "command");
-
- auto new_handler_pos = request_handlers.find(command);
+ if (const auto *req = std::get_if<protocol::Request>(&M)) {
+ auto new_handler_pos = request_handlers.find(req->command);
if (new_handler_pos != request_handlers.end()) {
- (*new_handler_pos->second)(object);
+ (*new_handler_pos->second)(*req);
return true; // Success
}
DAP_LOG(log, "({0}) error: unhandled command '{1}'",
- transport.GetClientName(), command);
+ transport.GetClientName(), req->command);
return false; // Fail
}
- if (packet_type == "response") {
- auto id = GetInteger<int64_t>(object, "request_seq").value_or(0);
-
+ if (const auto *resp = std::get_if<protocol::Response>(&M)) {
std::unique_ptr<ResponseHandler> response_handler;
{
std::lock_guard<std::mutex> locker(call_mutex);
- auto inflight = inflight_reverse_requests.find(id);
+ auto inflight = inflight_reverse_requests.find(resp->request_seq);
if (inflight != inflight_reverse_requests.end()) {
response_handler = std::move(inflight->second);
inflight_reverse_requests.erase(inflight);
@@ -695,19 +691,31 @@ bool DAP::HandleObject(const protocol::Message &M) {
}
if (!response_handler)
- response_handler = std::make_unique<UnknownResponseHandler>("", id);
+ response_handler =
+ std::make_unique<UnknownResponseHandler>("", resp->request_seq);
// Result should be given, use null if not.
- if (GetBoolean(object, "success").value_or(false)) {
- llvm::json::Value Result = nullptr;
- if (auto *B = object.get("body"))
- Result = std::move(*B);
- (*response_handler)(Result);
+ if (resp->success) {
+ (*response_handler)(resp->rawBody);
} else {
- llvm::StringRef message = GetString(object, "message");
- if (message.empty()) {
- message = "Unknown error, response failed";
+ std::string message = "Unknown error, response failed";
+ if (resp->message) {
+ message = std::visit(
+ llvm::makeVisitor(
+ [](const std::string &message) -> std::string {
+ return message;
+ },
+ [](const protocol::Response::Message &message) -> std::string {
+ switch (message) {
+ case protocol::Response::Message::cancelled:
+ return "cancelled";
+ case protocol::Response::Message::notStopped:
+ return "notStopped";
+ }
+ }),
+ *resp->message);
}
+
(*response_handler)(llvm::createStringError(
std::error_code(-1, std::generic_category()), message));
}
@@ -715,6 +723,9 @@ bool DAP::HandleObject(const protocol::Message &M) {
return true;
}
+ if (log)
+ *log << "Unsupported protocol message" << std::endl;
+
return false;
}
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index db3473b7c7027..f6b63e4a3e0bc 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -12,8 +12,6 @@
#include "DAPForward.h"
#include "ExceptionBreakpoint.h"
#include "FunctionBreakpoint.h"
-#include "Handler/RequestHandler.h"
-#include "Handler/ResponseHandler.h"
#include "InstructionBreakpoint.h"
#include "OutputRedirector.h"
#include "ProgressEvent.h"
@@ -187,7 +185,7 @@ struct DAP {
// the old process here so we can detect this case and keep running.
lldb::pid_t restarting_process_id;
bool configuration_done_sent;
- llvm::StringMap<std::unique_ptr<RequestHandler>> request_handlers;
+ llvm::StringMap<std::unique_ptr<BaseRequestHandler>> request_handlers;
bool waiting_for_run_in_terminal;
ProgressEventReporter progress_event_reporter;
// Keep track of the last stop thread index IDs as threads won't go away
diff --git a/lldb/tools/lldb-dap/DAPForward.h b/lldb/tools/lldb-dap/DAPForward.h
index 0196d83dcd6a9..667aef23abd0f 100644
--- a/lldb/tools/lldb-dap/DAPForward.h
+++ b/lldb/tools/lldb-dap/DAPForward.h
@@ -19,6 +19,8 @@ struct SourceBreakpoint;
struct Watchpoint;
struct InstructionBreakpoint;
struct DAP;
+class BaseRequestHandler;
+class ResponseHandler;
} // namespace lldb_dap
namespace lldb {
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
index e43fa36d25e3f..13cf166e200e8 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
@@ -6,8 +6,9 @@
//
//===----------------------------------------------------------------------===//
-#include "RequestHandler.h"
+#include "Handler/RequestHandler.h"
#include "DAP.h"
+#include "Handler/ResponseHandler.h"
#include "JSONUtils.h"
#include "LLDBUtils.h"
#include "RunInTerminal.h"
@@ -45,7 +46,7 @@ static uint32_t SetLaunchFlag(uint32_t flags, const llvm::json::Object *obj,
// Both attach and launch take either a sourcePath or a sourceMap
// argument (or neither), from which we need to set the target.source-map.
-void RequestHandler::SetSourceMapFromArguments(
+void BaseRequestHandler::SetSourceMapFromArguments(
const llvm::json::Object &arguments) const {
const char *sourceMapHelp =
"source must be be an array of two-element arrays, "
@@ -159,7 +160,7 @@ static llvm::Error RunInTerminal(DAP &dap,
}
lldb::SBError
-RequestHandler::LaunchProcess(const llvm::json::Object &request) const {
+BaseRequestHandler::LaunchProcess(const llvm::json::Object &request) const {
lldb::SBError error;
const auto *arguments = request.getObject("arguments");
auto launchCommands = GetStrings(arguments, "launchCommands");
@@ -228,13 +229,13 @@ RequestHandler::LaunchProcess(const llvm::json::Object &request) const {
return error;
}
-void RequestHandler::PrintWelcomeMessage() const {
+void BaseRequestHandler::PrintWelcomeMessage() const {
#ifdef LLDB_DAP_WELCOME_MESSAGE
dap.SendOutput(OutputType::Console, LLDB_DAP_WELCOME_MESSAGE);
#endif
}
-bool RequestHandler::HasInstructionGranularity(
+bool BaseRequestHandler::HasInstructionGranularity(
const llvm::json::Object &arguments) const {
if (std::optional<llvm::StringRef> value = arguments.getString("granularity"))
return value == "instruction";
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index b44367518bcb9..44a1aa4cefcde 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -9,25 +9,36 @@
#ifndef LLDB_TOOLS_LLDB_DAP_HANDLER_HANDLER_H
#define LLDB_TOOLS_LLDB_DAP_HANDLER_HANDLER_H
+#include "DAP.h"
+#include "Protocol.h"
#include "lldb/API/SBError.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/JSON.h"
namespace lldb_dap {
struct DAP;
-class RequestHandler {
+/// Base class for request handlers. Do not extend this directly: Extend
+/// the RequestHandler template subclass instead.
+class BaseRequestHandler {
public:
- RequestHandler(DAP &dap) : dap(dap) {}
+ BaseRequestHandler(DAP &dap) : dap(dap) {}
- /// RequestHandler are not copyable.
+ /// BaseRequestHandler are not copyable.
/// @{
- RequestHandler(const RequestHandler &) = delete;
- RequestHandler &operator=(const RequestHandler &) = delete;
+ BaseRequestHandler(const BaseRequestHandler &) = delete;
+ BaseRequestHandler &operator=(const BaseRequestHandler &) = delete;
/// @}
- virtual ~RequestHandler() = default;
+ virtual ~BaseRequestHandler() = default;
+ virtual void operator()(const protocol::Request &request) const {
+ auto req = toJSON(request);
+ (*this)(*req.getAsObject());
+ }
+
+ /// FIXME: Migrate callers to typed RequestHandler for improved type handling.
virtual void operator()(const llvm::json::Object &request) const = 0;
protected:
@@ -57,235 +68,288 @@ class RequestHandler {
DAP &dap;
};
-class AttachRequestHandler : public RequestHandler {
-public:
- using RequestHandler::RequestHandler;
+/// Base class for handling DAP requests. Handlers should declare their
+/// arguments and response body types like:
+///
+/// class MyRequestHandler : public RequestHandler<Arguments, ResponseBody> {
+/// ....
+/// };
+template <typename Args, typename Body>
+class RequestHandler : public BaseRequestHandler {
+ using BaseRequestHandler::BaseRequestHandler;
+
+ void operator()(const llvm::json::Object &request) const override {
+ /* no-op, the other overload handles json coding. */
+ }
+
+ void operator()(const protocol::Request &request) const override {
+ protocol::Response response;
+ response.request_seq = request.seq;
+ response.command = request.command;
+ Args arguments;
+ llvm::json::Path::Root root;
+ if (request.rawArguments &&
+ !fromJSON(request.rawArguments, arguments, root)) {
+ std::string parseFailure;
+ llvm::raw_string_ostream OS(parseFailure);
+ root.printErrorContext(request.rawArguments, OS);
+ response.success = false;
+ response.message = parseFailure;
+ dap.SendJSON(std::move(response));
+ return;
+ }
+
+ auto ResponseBody = Run(arguments);
+ // FIXME: Add a dedicated DAPError for enhanced errors that are
+ // user-visibile.
+ if (auto Err = ResponseBody.takeError()) {
+ response.success = false;
+ // FIXME: Build ErrorMessage based on error details instead of using the
+ // 'message' field.
+ response.message = llvm::toString(std::move(Err));
+ } else {
+ response.success = true;
+ response.rawBody = std::move(*ResponseBody);
+ }
+
+ dap.SendJSON(std::move(response));
+ };
+
+ virtual llvm::Expected<Body> Run(const Args &) const = 0;
+};
+
+class AttachRequestHandler : public BaseRequestHandler {
+public:
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "attach"; }
void operator()(const llvm::json::Object &request) const override;
};
-class BreakpointLocationsRequestHandler : public RequestHandler {
+class BreakpointLocationsRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "breakpointLocations"; }
void operator()(const llvm::json::Object &request) const override;
};
-class CompletionsRequestHandler : public RequestHandler {
+class CompletionsRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "completions"; }
void operator()(const llvm::json::Object &request) const override;
};
-class ContinueRequestHandler : public RequestHandler {
+class ContinueRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "continue"; }
void operator()(const llvm::json::Object &request) const override;
};
-class ConfigurationDoneRequestHandler : public RequestHandler {
+class ConfigurationDoneRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "configurationDone"; }
void operator()(const llvm::json::Object &request) const override;
};
-class DisconnectRequestHandler : public RequestHandler {
+class DisconnectRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "disconnect"; }
void operator()(const llvm::json::Object &request) const override;
};
-class EvaluateRequestHandler : public RequestHandler {
+class EvaluateRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "evaluate"; }
void operator()(const llvm::json::Object &request) const override;
};
-class ExceptionInfoRequestHandler : public RequestHandler {
+class ExceptionInfoRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "exceptionInfo"; }
void operator()(const llvm::json::Object &request) const override;
};
-class InitializeRequestHandler : public RequestHandler {
+class InitializeRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "initialize"; }
void operator()(const llvm::json::Object &request) const override;
};
-class LaunchRequestHandler : public RequestHandler {
+class LaunchRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "launch"; }
void operator()(const llvm::json::Object &request) const override;
};
-class RestartRequestHandler : public RequestHandler {
+class RestartRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "restart"; }
void operator()(const llvm::json::Object &request) const override;
};
-class NextRequestHandler : public RequestHandler {
+class NextRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "next"; }
void operator()(const llvm::json::Object &request) const override;
};
-class StepInRequestHandler : public RequestHandler {
+class StepInRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "stepIn"; }
void operator()(const llvm::json::Object &request) const override;
};
-class StepInTargetsRequestHandler : public RequestHandler {
+class StepInTargetsRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "stepInTargets"; }
void operator()(const llvm::json::Object &request) const override;
};
-class StepOutRequestHandler : public RequestHandler {
+class StepOutRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "stepOut"; }
void operator()(const llvm::json::Object &request) const override;
};
-class SetBreakpointsRequestHandler : public RequestHandler {
+class SetBreakpointsRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "setBreakpoints"; }
void operator()(const llvm::json::Object &request) const override;
};
-class SetExceptionBreakpointsRequestHandler : public RequestHandler {
+class SetExceptionBreakpointsRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "setExceptionBreakpoints"; }
void operator()(const llvm::json::Object &request) const override;
};
-class SetFunctionBreakpointsRequestHandler : public RequestHandler {
+class SetFunctionBreakpointsRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "setFunctionBreakpoints"; }
void operator()(const llvm::json::Object &request) const override;
};
-class DataBreakpointInfoRequestHandler : public RequestHandler {
+class DataBreakpointInfoRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "dataBreakpointInfo"; }
void operator()(const llvm::json::Object &request) const override;
};
-class SetDataBreakpointsRequestHandler : public RequestHandler {
+class SetDataBreakpointsRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "setDataBreakpoints"; }
void operator()(const llvm::json::Object &request) const override;
};
-class SetInstructionBreakpointsRequestHandler : public RequestHandler {
+class SetInstructionBreakpointsRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() {
return "setInstructionBreakpoints";
}
void operator()(const llvm::json::Object &request) const override;
};
-class CompileUnitsRequestHandler : public RequestHandler {
+class CompileUnitsRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "compileUnits"; }
void operator()(const llvm::json::Object &request) const override;
};
-class ModulesRequestHandler : public RequestHandler {
+class ModulesRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "modules"; }
void operator()(const llvm::json::Object &request) const override;
};
-class PauseRequestHandler : public RequestHandler {
+class PauseRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "pause"; }
void operator()(const llvm::json::Object &request) const override;
};
-class ScopesRequestHandler : public RequestHandler {
+class ScopesRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseRequestHandler;
static llvm::StringLiteral getCommand() { return "scopes"; }
void operator()(const llvm::json::Object &request) const override;
};
-class SetVariableRequestHandler : public RequestHandler {
+class SetVariableRequestHandler : public BaseRequestHandler {
public:
- using RequestHandler::RequestHandler;
+ using BaseRequestHandler::BaseReque...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/130090
More information about the lldb-commits
mailing list