[Lldb-commits] [lldb] 1041d54 - [lldb-dap] Updating the 'next' request handler use well structured types (#136642)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Apr 23 10:18:35 PDT 2025
Author: John Harrison
Date: 2025-04-23T10:18:31-07:00
New Revision: 1041d54bd4f693c1ac03077680ece67e03c99e22
URL: https://github.com/llvm/llvm-project/commit/1041d54bd4f693c1ac03077680ece67e03c99e22
DIFF: https://github.com/llvm/llvm-project/commit/1041d54bd4f693c1ac03077680ece67e03c99e22.diff
LOG: [lldb-dap] Updating the 'next' request handler use well structured types (#136642)
This updates the 'next' request to use well structured types. While
working on this I also simplified the 'RequestHandler' implementation to
better handle void responses by allowing requests to return a
'llvm::Error' instead of an 'llvm::Expected<std::monostate>'. This makes
it easier to write and understand request handles that have simple ack
responses.
Added:
Modified:
lldb/tools/lldb-dap/DAP.cpp
lldb/tools/lldb-dap/DAP.h
lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp
lldb/tools/lldb-dap/Handler/DisconnectRequestHandler.cpp
lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
lldb/tools/lldb-dap/Handler/RequestHandler.h
lldb/tools/lldb-dap/Protocol/ProtocolBase.h
lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
lldb/tools/lldb-dap/Transport.cpp
Removed:
################################################################################
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 597fe3a1e323b..134762711b89d 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -28,6 +28,7 @@
#include "lldb/Utility/Status.h"
#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-types.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
@@ -499,6 +500,10 @@ ExceptionBreakpoint *DAP::GetExceptionBPFromStopReason(lldb::SBThread &thread) {
return exc_bp;
}
+lldb::SBThread DAP::GetLLDBThread(lldb::tid_t tid) {
+ return target.GetProcess().GetThreadByID(tid);
+}
+
lldb::SBThread DAP::GetLLDBThread(const llvm::json::Object &arguments) {
auto tid = GetInteger<int64_t>(arguments, "threadId")
.value_or(LLDB_INVALID_THREAD_ID);
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index b79a0d9d0f25c..727e5c00623e8 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -266,6 +266,7 @@ struct DAP {
ExceptionBreakpoint *GetExceptionBPFromStopReason(lldb::SBThread &thread);
+ lldb::SBThread GetLLDBThread(lldb::tid_t id);
lldb::SBThread GetLLDBThread(const llvm::json::Object &arguments);
lldb::SBFrame GetLLDBFrame(const llvm::json::Object &arguments);
diff --git a/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp
index f09de13c3ff72..995fe38362f60 100644
--- a/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/CancelRequestHandler.cpp
@@ -10,7 +10,7 @@
#include "Protocol/ProtocolRequests.h"
#include "llvm/Support/Error.h"
-using namespace lldb_dap;
+using namespace llvm;
using namespace lldb_dap::protocol;
namespace lldb_dap {
@@ -45,12 +45,11 @@ namespace lldb_dap {
///
/// A client cannot assume that progress just got cancelled after sending
/// the `cancel` request.
-llvm::Expected<CancelResponseBody>
-CancelRequestHandler::Run(const CancelArguments &arguments) const {
+Error CancelRequestHandler::Run(const CancelArguments &arguments) const {
// Cancel support is built into the DAP::Loop handler for detecting
// cancellations of pending or inflight requests.
dap.ClearCancelRequest(arguments);
- return CancelResponseBody();
+ return Error::success();
}
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/DisconnectRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DisconnectRequestHandler.cpp
index f12fecfb2ff65..81e94c7551836 100644
--- a/lldb/tools/lldb-dap/Handler/DisconnectRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/DisconnectRequestHandler.cpp
@@ -18,7 +18,7 @@ using namespace lldb_dap::protocol;
namespace lldb_dap {
/// Disconnect request; value of command field is 'disconnect'.
-Expected<DisconnectResponse> DisconnectRequestHandler::Run(
+Error DisconnectRequestHandler::Run(
const std::optional<DisconnectArguments> &arguments) const {
bool terminateDebuggee = dap.is_attach ? false : true;
@@ -28,6 +28,6 @@ Expected<DisconnectResponse> DisconnectRequestHandler::Run(
if (Error error = dap.Disconnect(terminateDebuggee))
return error;
- return DisconnectResponse();
+ return Error::success();
}
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
index 216e710035cb1..1603563841005 100644
--- a/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
@@ -8,72 +8,37 @@
#include "DAP.h"
#include "EventHelper.h"
-#include "JSONUtils.h"
+#include "Protocol/ProtocolTypes.h"
#include "RequestHandler.h"
+#include "llvm/Support/Error.h"
+
+using namespace llvm;
+using namespace lldb_dap::protocol;
namespace lldb_dap {
-// "NextRequest": {
-// "allOf": [ { "$ref": "#/definitions/Request" }, {
-// "type": "object",
-// "description": "Next request; value of command field is 'next'. The
-// request starts the debuggee to run again for one step.
-// The debug adapter first sends the NextResponse and then
-// a StoppedEvent (event type 'step') after the step has
-// completed.",
-// "properties": {
-// "command": {
-// "type": "string",
-// "enum": [ "next" ]
-// },
-// "arguments": {
-// "$ref": "#/definitions/NextArguments"
-// }
-// },
-// "required": [ "command", "arguments" ]
-// }]
-// },
-// "NextArguments": {
-// "type": "object",
-// "description": "Arguments for 'next' request.",
-// "properties": {
-// "threadId": {
-// "type": "integer",
-// "description": "Execute 'next' for this thread."
-// },
-// "granularity": {
-// "$ref": "#/definitions/SteppingGranularity",
-// "description": "Stepping granularity. If no granularity is specified, a
-// granularity of `statement` is assumed."
-// }
-// },
-// "required": [ "threadId" ]
-// },
-// "NextResponse": {
-// "allOf": [ { "$ref": "#/definitions/Response" }, {
-// "type": "object",
-// "description": "Response to 'next' request. This is just an
-// acknowledgement, so no body field is required."
-// }]
-// }
-void NextRequestHandler::operator()(const llvm::json::Object &request) const {
- llvm::json::Object response;
- FillResponse(request, response);
- const auto *arguments = request.getObject("arguments");
- lldb::SBThread thread = dap.GetLLDBThread(*arguments);
- if (thread.IsValid()) {
- // Remember the thread ID that caused the resume so we can set the
- // "threadCausedFocus" boolean value in the "stopped" events.
- dap.focus_tid = thread.GetThreadID();
- if (HasInstructionGranularity(*arguments)) {
- thread.StepInstruction(/*step_over=*/true);
- } else {
- thread.StepOver();
- }
+/// The request executes one step (in the given granularity) for the specified
+/// thread and allows all other threads to run freely by resuming them. If the
+/// debug adapter supports single thread execution (see capability
+/// `supportsSingleThreadExecutionRequests`), setting the `singleThread`
+/// argument to true prevents other suspended threads from resuming. The debug
+/// adapter first sends the response and then a `stopped` event (with reason
+/// `step`) after the step has completed.
+Error NextRequestHandler::Run(const NextArguments &args) const {
+ lldb::SBThread thread = dap.GetLLDBThread(args.threadId);
+ if (!thread.IsValid())
+ return make_error<DAPError>("invalid thread");
+
+ // Remember the thread ID that caused the resume so we can set the
+ // "threadCausedFocus" boolean value in the "stopped" events.
+ dap.focus_tid = thread.GetThreadID();
+ if (args.granularity == eSteppingGranularityInstruction) {
+ thread.StepInstruction(/*step_over=*/true);
} else {
- response["success"] = llvm::json::Value(false);
+ thread.StepOver();
}
- dap.SendJSON(llvm::json::Value(std::move(response)));
+
+ return Error::success();
}
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index 7e56c258ad78a..edb9de7d0dc20 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -94,10 +94,10 @@ class LegacyRequestHandler : public BaseRequestHandler {
/// Base class for handling DAP requests. Handlers should declare their
/// arguments and response body types like:
///
-/// class MyRequestHandler : public RequestHandler<Arguments, ResponseBody> {
+/// class MyRequestHandler : public RequestHandler<Arguments, Response> {
/// ....
/// };
-template <typename Args, typename Body>
+template <typename Args, typename Resp>
class RequestHandler : public BaseRequestHandler {
using BaseRequestHandler::BaseRequestHandler;
@@ -128,41 +128,29 @@ class RequestHandler : public BaseRequestHandler {
<< "': " << llvm::toString(root.getError()) << "\n";
root.printErrorContext(*request.arguments, OS);
- protocol::ErrorMessage error_message;
- error_message.format = parse_failure;
-
- protocol::ErrorResponseBody body;
- body.error = error_message;
-
response.success = false;
- response.body = std::move(body);
+ response.body = ToResponse(llvm::make_error<DAPError>(parse_failure));
dap.Send(response);
return;
}
- llvm::Expected<Body> body = Run(arguments);
- if (auto Err = body.takeError()) {
- protocol::ErrorMessage error_message;
- error_message.sendTelemetry = false;
- if (llvm::Error unhandled = llvm::handleErrors(
- std::move(Err), [&](const DAPError &E) -> llvm::Error {
- error_message.format = E.getMessage();
- error_message.showUser = E.getShowUser();
- error_message.id = E.convertToErrorCode().value();
- error_message.url = E.getURL();
- error_message.urlLabel = E.getURLLabel();
- return llvm::Error::success();
- }))
- error_message.format = llvm::toString(std::move(unhandled));
- protocol::ErrorResponseBody body;
- body.error = error_message;
- response.success = false;
- response.body = std::move(body);
+ if constexpr (std::is_same_v<Resp, llvm::Error>) {
+ if (llvm::Error err = Run(arguments)) {
+ response.success = false;
+ response.body = ToResponse(std::move(err));
+ } else {
+ response.success = true;
+ }
} else {
- response.success = true;
- if constexpr (!std::is_same_v<Body, std::monostate>)
+ Resp body = Run(arguments);
+ if (llvm::Error err = body.takeError()) {
+ response.success = false;
+ response.body = ToResponse(std::move(err));
+ } else {
+ response.success = true;
response.body = std::move(*body);
+ }
}
// Mark the request as 'cancelled' if the debugger was interrupted while
@@ -177,7 +165,25 @@ class RequestHandler : public BaseRequestHandler {
dap.Send(response);
};
- virtual llvm::Expected<Body> Run(const Args &) const = 0;
+ virtual Resp Run(const Args &) const = 0;
+
+ protocol::ErrorResponseBody ToResponse(llvm::Error err) const {
+ protocol::ErrorMessage error_message;
+ error_message.sendTelemetry = false;
+ if (llvm::Error unhandled = llvm::handleErrors(
+ std::move(err), [&](const DAPError &E) -> llvm::Error {
+ error_message.format = E.getMessage();
+ error_message.showUser = E.getShowUser();
+ error_message.id = E.convertToErrorCode().value();
+ error_message.url = E.getURL();
+ error_message.urlLabel = E.getURLLabel();
+ return llvm::Error::success();
+ }))
+ error_message.format = llvm::toString(std::move(unhandled));
+ protocol::ErrorResponseBody body;
+ body.error = error_message;
+ return body;
+ }
};
class AttachRequestHandler : public LegacyRequestHandler {
@@ -233,7 +239,7 @@ class DisconnectRequestHandler
FeatureSet GetSupportedFeatures() const override {
return {protocol::eAdapterFeatureTerminateDebuggee};
}
- llvm::Expected<protocol::DisconnectResponse>
+ llvm::Error
Run(const std::optional<protocol::DisconnectArguments> &args) const override;
};
@@ -259,7 +265,7 @@ class ExceptionInfoRequestHandler : public LegacyRequestHandler {
class InitializeRequestHandler
: public RequestHandler<protocol::InitializeRequestArguments,
- protocol::InitializeResponseBody> {
+ llvm::Expected<protocol::InitializeResponseBody>> {
public:
using RequestHandler::RequestHandler;
static llvm::StringLiteral GetCommand() { return "initialize"; }
@@ -284,11 +290,12 @@ class RestartRequestHandler : public LegacyRequestHandler {
void operator()(const llvm::json::Object &request) const override;
};
-class NextRequestHandler : public LegacyRequestHandler {
+class NextRequestHandler
+ : public RequestHandler<protocol::NextArguments, protocol::NextResponse> {
public:
- using LegacyRequestHandler::LegacyRequestHandler;
+ using RequestHandler::RequestHandler;
static llvm::StringLiteral GetCommand() { return "next"; }
- void operator()(const llvm::json::Object &request) const override;
+ llvm::Error Run(const protocol::NextArguments &args) const override;
};
class StepInRequestHandler : public LegacyRequestHandler {
@@ -418,7 +425,7 @@ class SetVariableRequestHandler : public LegacyRequestHandler {
class SourceRequestHandler
: public RequestHandler<protocol::SourceArguments,
- protocol::SourceResponseBody> {
+ llvm::Expected<protocol::SourceResponseBody>> {
public:
using RequestHandler::RequestHandler;
static llvm::StringLiteral GetCommand() { return "source"; }
@@ -486,8 +493,7 @@ class CancelRequestHandler
FeatureSet GetSupportedFeatures() const override {
return {protocol::eAdapterFeatureCancelRequest};
}
- llvm::Expected<protocol::CancelResponseBody>
- Run(const protocol::CancelArguments &args) const override;
+ llvm::Error Run(const protocol::CancelArguments &args) const override;
};
/// A request used in testing to get the details on all breakpoints that are
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolBase.h b/lldb/tools/lldb-dap/Protocol/ProtocolBase.h
index 2c647610de11c..bad0e886d94d2 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolBase.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolBase.h
@@ -149,7 +149,7 @@ struct ErrorResponseBody {
llvm::json::Value toJSON(const ErrorResponseBody &);
/// This is just an acknowledgement, so no body field is required.
-using VoidResponse = std::monostate;
+using VoidResponse = llvm::Error;
} // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index 3523f8ac87ec9..b113299affb0f 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -7,7 +7,6 @@
//===----------------------------------------------------------------------===//
#include "Protocol/ProtocolRequests.h"
-#include "DAP.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/JSON.h"
@@ -114,4 +113,12 @@ json::Value toJSON(const SourceResponseBody &SA) {
return std::move(Result);
}
+bool fromJSON(const llvm::json::Value &Params, NextArguments &NA,
+ llvm::json::Path P) {
+ json::ObjectMapper OM(Params, P);
+ return OM && OM.map("threadId", NA.threadId) &&
+ OM.mapOptional("singleThread", NA.singleThread) &&
+ OM.mapOptional("granularity", NA.granularity);
+}
+
} // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index 6623dfa0db05c..6e3e2c6a9e2c8 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -22,6 +22,7 @@
#include "Protocol/ProtocolBase.h"
#include "Protocol/ProtocolTypes.h"
+#include "lldb/lldb-defines.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/Support/JSON.h"
#include <cstdint>
@@ -236,6 +237,25 @@ struct SourceResponseBody {
};
llvm::json::Value toJSON(const SourceResponseBody &);
+/// Arguments for `next` request.
+struct NextArguments {
+ /// Specifies the thread for which to resume execution for one step (of the
+ /// given granularity).
+ uint64_t threadId = LLDB_INVALID_THREAD_ID;
+
+ /// If this flag is true, all other suspended threads are not resumed.
+ bool singleThread = false;
+
+ /// Stepping granularity. If no granularity is specified, a granularity of
+ /// `statement` is assumed.
+ SteppingGranularity granularity = eSteppingGranularityStatement;
+};
+bool fromJSON(const llvm::json::Value &, NextArguments &, llvm::json::Path);
+
+/// Response to `next` request. This is just an acknowledgement, so no
+/// body field is required.
+using NextResponse = VoidResponse;
+
} // namespace lldb_dap::protocol
#endif
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
index 4d1e90215bbb4..e64998c4ca488 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
@@ -233,4 +233,25 @@ json::Value toJSON(const Capabilities &C) {
return result;
}
+bool fromJSON(const llvm::json::Value &Params, SteppingGranularity &SG,
+ llvm::json::Path P) {
+ auto raw_granularity = Params.getAsString();
+ if (!raw_granularity) {
+ P.report("expected a string");
+ return false;
+ }
+ std::optional<SteppingGranularity> granularity =
+ StringSwitch<std::optional<SteppingGranularity>>(*raw_granularity)
+ .Case("statement", eSteppingGranularityStatement)
+ .Case("line", eSteppingGranularityLine)
+ .Case("instruction", eSteppingGranularityInstruction)
+ .Default(std::nullopt);
+ if (!granularity) {
+ P.report("unexpected value");
+ return false;
+ }
+ SG = *granularity;
+ return true;
+}
+
} // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
index 8f38c524ea649..54941f24efbd9 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
@@ -303,6 +303,25 @@ struct Source {
};
bool fromJSON(const llvm::json::Value &, Source &, llvm::json::Path);
+/// The granularity of one `step` in the stepping requests `next`, `stepIn`,
+/// `stepOut` and `stepBack`.
+enum SteppingGranularity : unsigned {
+ /// The step should allow the program to run until the current statement has
+ /// finished executing. The meaning of a statement is determined by the
+ /// adapter and it may be considered equivalent to a line. For example
+ /// `for(int i = 0; i < 10; i++)` could be considered to have 3 statements
+ /// `int i = 0`, `i < 10`, and `i++`.
+ eSteppingGranularityStatement,
+ /// The step should allow the program to run until the current source line has
+ /// executed.
+ eSteppingGranularityLine,
+ /// The step should allow one instruction to execute (e.g. one x86
+ /// instruction).
+ eSteppingGranularityInstruction,
+};
+bool fromJSON(const llvm::json::Value &, SteppingGranularity &,
+ llvm::json::Path);
+
} // namespace lldb_dap::protocol
#endif
diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
index ffd0c49f1770b..4e322e9ff1358 100644
--- a/lldb/tools/lldb-dap/Transport.cpp
+++ b/lldb/tools/lldb-dap/Transport.cpp
@@ -137,7 +137,8 @@ Expected<Message> Transport::Read(const std::chrono::microseconds &timeout) {
DAP_LOG(m_log, "--> ({0}) {1}", m_client_name, *raw_json);
- return json::parse<Message>(*raw_json);
+ return json::parse<Message>(/*JSON=*/*raw_json,
+ /*RootName=*/"protocol_message");
}
Error Transport::Write(const Message &message) {
More information about the lldb-commits
mailing list