[Lldb-commits] [lldb] 66af492 - [lldb-dap] Refactor reverse request response handlers (NFC) (#128594)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Feb 25 11:00:30 PST 2025
Author: Jonas Devlieghere
Date: 2025-02-25T13:00:26-06:00
New Revision: 66af4923ce245a0fd9427db8e4861354576d0866
URL: https://github.com/llvm/llvm-project/commit/66af4923ce245a0fd9427db8e4861354576d0866
DIFF: https://github.com/llvm/llvm-project/commit/66af4923ce245a0fd9427db8e4861354576d0866.diff
LOG: [lldb-dap] Refactor reverse request response handlers (NFC) (#128594)
This refactors the response handlers for reverse request to follow the
same architecture as the request handlers. With only two implementation
that might be overkill, but it reduces code duplication and improves
error reporting by storing the sequence ID. This PR also fixes an
unchecked Expected in the old callback for unknown sequence IDs.
Added:
lldb/tools/lldb-dap/Handler/ResponseHandler.cpp
lldb/tools/lldb-dap/Handler/ResponseHandler.h
Modified:
lldb/tools/lldb-dap/CMakeLists.txt
lldb/tools/lldb-dap/DAP.cpp
lldb/tools/lldb-dap/DAP.h
lldb/tools/lldb-dap/Handler/RequestHandler.cpp
Removed:
################################################################################
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt
index 804dd8e4cc2a0..8b3c520ec4360 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -37,6 +37,7 @@ add_lldb_tool(lldb-dap
SourceBreakpoint.cpp
Watchpoint.cpp
+ Handler/ResponseHandler.cpp
Handler/AttachRequestHandler.cpp
Handler/BreakpointLocationsHandler.cpp
Handler/CompileUnitsRequestHandler.cpp
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 01f294e14de6a..37bc1f68e4b08 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "DAP.h"
+#include "Handler/ResponseHandler.h"
#include "JSONUtils.h"
#include "LLDBUtils.h"
#include "OutputRedirector.h"
@@ -34,6 +35,7 @@
#include <cstdarg>
#include <cstdio>
#include <fstream>
+#include <memory>
#include <mutex>
#include <utility>
@@ -769,10 +771,8 @@ bool DAP::HandleObject(const llvm::json::Object &object) {
if (packet_type == "response") {
auto id = GetSigned(object, "request_seq", 0);
- ResponseCallback response_handler = [](llvm::Expected<llvm::json::Value>) {
- llvm::errs() << "Unhandled response\n";
- };
+ std::unique_ptr<ResponseHandler> response_handler;
{
std::lock_guard<std::mutex> locker(call_mutex);
auto inflight = inflight_reverse_requests.find(id);
@@ -782,19 +782,22 @@ bool DAP::HandleObject(const llvm::json::Object &object) {
}
}
+ if (!response_handler)
+ response_handler = std::make_unique<UnknownResponseHandler>("", id);
+
// Result should be given, use null if not.
if (GetBoolean(object, "success", false)) {
llvm::json::Value Result = nullptr;
if (auto *B = object.get("body")) {
Result = std::move(*B);
}
- response_handler(Result);
+ (*response_handler)(Result);
} else {
llvm::StringRef message = GetString(object, "message");
if (message.empty()) {
message = "Unknown error, response failed";
}
- response_handler(llvm::createStringError(
+ (*response_handler)(llvm::createStringError(
std::error_code(-1, std::generic_category()), message));
}
@@ -875,24 +878,6 @@ llvm::Error DAP::Loop() {
return llvm::Error::success();
}
-void DAP::SendReverseRequest(llvm::StringRef command,
- llvm::json::Value arguments,
- ResponseCallback callback) {
- int64_t id;
- {
- std::lock_guard<std::mutex> locker(call_mutex);
- id = ++reverse_request_seq;
- inflight_reverse_requests.emplace(id, std::move(callback));
- }
-
- SendJSON(llvm::json::Object{
- {"type", "request"},
- {"seq", id},
- {"command", command},
- {"arguments", std::move(arguments)},
- });
-}
-
lldb::SBError DAP::WaitForProcessToStop(uint32_t seconds) {
lldb::SBError error;
lldb::SBProcess process = target.GetProcess();
@@ -1007,17 +992,10 @@ bool StartDebuggingRequestHandler::DoExecute(
return false;
}
- dap.SendReverseRequest(
+ dap.SendReverseRequest<LogFailureResponseHandler>(
"startDebugging",
llvm::json::Object{{"request", request},
- {"configuration", std::move(*configuration)}},
- [](llvm::Expected<llvm::json::Value> value) {
- if (!value) {
- llvm::Error err = value.takeError();
- llvm::errs() << "reverse start debugging request failed: "
- << llvm::toString(std::move(err)) << "\n";
- }
- });
+ {"configuration", std::move(*configuration)}});
result.SetStatus(lldb::eReturnStatusSuccessFinishNoResult);
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index f87841a56f4d3..7ceb1d114a57d 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -13,6 +13,7 @@
#include "ExceptionBreakpoint.h"
#include "FunctionBreakpoint.h"
#include "Handler/RequestHandler.h"
+#include "Handler/ResponseHandler.h"
#include "IOStream.h"
#include "InstructionBreakpoint.h"
#include "OutputRedirector.h"
@@ -68,8 +69,6 @@ enum DAPBroadcasterBits {
eBroadcastBitStopProgressThread = 1u << 1
};
-typedef void (*ResponseCallback)(llvm::Expected<llvm::json::Value> value);
-
enum class PacketStatus {
Success = 0,
EndOfFile,
@@ -197,7 +196,7 @@ struct DAP {
llvm::DenseSet<lldb::tid_t> thread_ids;
uint32_t reverse_request_seq;
std::mutex call_mutex;
- std::map<int /* request_seq */, ResponseCallback /* reply handler */>
+ llvm::SmallDenseMap<int64_t, std::unique_ptr<ResponseHandler>>
inflight_reverse_requests;
ReplMode repl_mode;
std::string command_escape_prefix = "`";
@@ -327,12 +326,24 @@ struct DAP {
/// The reverse request command.
///
/// \param[in] arguments
- /// The reverse request arguements.
- ///
- /// \param[in] callback
- /// A callback to execute when the response arrives.
- void SendReverseRequest(llvm::StringRef command, llvm::json::Value arguments,
- ResponseCallback callback);
+ /// The reverse request arguments.
+ template <typename Handler>
+ void SendReverseRequest(llvm::StringRef command,
+ llvm::json::Value arguments) {
+ int64_t id;
+ {
+ std::lock_guard<std::mutex> locker(call_mutex);
+ id = ++reverse_request_seq;
+ inflight_reverse_requests[id] = std::make_unique<Handler>(command, id);
+ }
+
+ SendJSON(llvm::json::Object{
+ {"type", "request"},
+ {"seq", id},
+ {"command", command},
+ {"arguments", std::move(arguments)},
+ });
+ }
/// Registers a request handler.
template <typename Handler> void RegisterRequest() {
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
index ad00e43947212..0a32e39ea3aff 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
@@ -101,15 +101,8 @@ static llvm::Error RunInTerminal(DAP &dap,
#endif
llvm::json::Object reverse_request = CreateRunInTerminalReverseRequest(
launch_request, dap.debug_adaptor_path, comm_file.m_path, debugger_pid);
- dap.SendReverseRequest("runInTerminal", std::move(reverse_request),
- [](llvm::Expected<llvm::json::Value> value) {
- if (!value) {
- llvm::Error err = value.takeError();
- llvm::errs()
- << "runInTerminal request failed: "
- << llvm::toString(std::move(err)) << "\n";
- }
- });
+ dap.SendReverseRequest<LogFailureResponseHandler>("runInTerminal",
+ std::move(reverse_request));
if (llvm::Expected<lldb::pid_t> pid = comm_channel.GetLauncherPid())
attach_info.SetProcessID(*pid);
diff --git a/lldb/tools/lldb-dap/Handler/ResponseHandler.cpp b/lldb/tools/lldb-dap/Handler/ResponseHandler.cpp
new file mode 100644
index 0000000000000..27a1437de5502
--- /dev/null
+++ b/lldb/tools/lldb-dap/Handler/ResponseHandler.cpp
@@ -0,0 +1,35 @@
+//===-- ResponseHandler.cpp -----------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ResponseHandler.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
+
+namespace lldb_dap {
+
+void UnknownResponseHandler::operator()(
+ llvm::Expected<llvm::json::Value> value) const {
+ llvm::errs() << "unexpected response: ";
+ if (value) {
+ if (std::optional<llvm::StringRef> str = value->getAsString())
+ llvm::errs() << *str;
+ } else {
+ llvm::errs() << "error: " << llvm::toString(value.takeError());
+ }
+ llvm::errs() << '\n';
+}
+
+void LogFailureResponseHandler::operator()(
+ llvm::Expected<llvm::json::Value> value) const {
+ if (!value)
+ llvm::errs() << "reverse request \"" << m_command << "\" (" << m_id
+ << ") failed: " << llvm::toString(value.takeError()) << '\n';
+}
+
+} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/ResponseHandler.h b/lldb/tools/lldb-dap/Handler/ResponseHandler.h
new file mode 100644
index 0000000000000..b09153ac2edad
--- /dev/null
+++ b/lldb/tools/lldb-dap/Handler/ResponseHandler.h
@@ -0,0 +1,56 @@
+//===-- ResponseHandler.h -------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_TOOLS_LLDB_DAP_HANDLER_RESPONSEHANDLER_H
+#define LLDB_TOOLS_LLDB_DAP_HANDLER_RESPONSEHANDLER_H
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/JSON.h"
+#include <cstdint>
+
+namespace lldb_dap {
+struct DAP;
+
+/// Handler for responses to reverse requests.
+class ResponseHandler {
+public:
+ ResponseHandler(llvm::StringRef command, int64_t id)
+ : m_command(command), m_id(id) {}
+
+ /// ResponseHandlers are not copyable.
+ /// @{
+ ResponseHandler(const ResponseHandler &) = delete;
+ ResponseHandler &operator=(const ResponseHandler &) = delete;
+ /// @}
+
+ virtual ~ResponseHandler() = default;
+
+ virtual void operator()(llvm::Expected<llvm::json::Value> value) const = 0;
+
+protected:
+ llvm::StringRef m_command;
+ int64_t m_id;
+};
+
+/// Response handler used for unknown responses.
+class UnknownResponseHandler : public ResponseHandler {
+public:
+ using ResponseHandler::ResponseHandler;
+ void operator()(llvm::Expected<llvm::json::Value> value) const override;
+};
+
+/// Response handler which logs to stderr in case of a failure.
+class LogFailureResponseHandler : public ResponseHandler {
+public:
+ using ResponseHandler::ResponseHandler;
+ void operator()(llvm::Expected<llvm::json::Value> value) const override;
+};
+
+} // namespace lldb_dap
+
+#endif
More information about the lldb-commits
mailing list