[lldb-dap] Refactor reverse request response handlers (NFC)

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

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);
 

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() {

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);

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

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