[lldb-dap] Updating RequestHandler to encode/decode arguments and response.

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)
@@ -233,6 +237,10 @@ void DAP::SendJSON(const llvm::json::Value &json) {
                   transport.GetClientName());
     return;
   }
+  Send(message);
+}
+
+void DAP::Send(const protocol::Message &message) {
   if (llvm::Error err = transport.Write(message))
     DAP_LOG_ERROR(log, std::move(err), "({1}) write failed: {0}",
                   transport.GetClientName());
@@ -663,58 +671,65 @@ 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);
       }
     }
 
     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));
     }
 
     return true;
   }
 
+  if (log)
+    *log << "Unsupported protocol message" << std::endl;
+
   return false;
 }
 
@@ -728,9 +743,9 @@ void DAP::SendTerminatedEvent() {
   });
 }
 
-lldb::SBError DAP::Disconnect() { return Disconnect(is_attach); }
+llvm::Error DAP::Disconnect() { return Disconnect(is_attach); }
 
-lldb::SBError DAP::Disconnect(bool terminateDebuggee) {
+llvm::Error DAP::Disconnect(bool terminateDebuggee) {
   lldb::SBError error;
   lldb::SBProcess process = target.GetProcess();
   auto state = process.GetState();
@@ -758,7 +773,7 @@ lldb::SBError DAP::Disconnect(bool terminateDebuggee) {
 
   disconnecting = true;
 
-  return error;
+  return takeError(error);
 }
 
 llvm::Error DAP::Loop() {

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
@@ -248,6 +246,7 @@ struct DAP {
   // Serialize the JSON value into a string and send the JSON packet to
   // the "out" stream.
   void SendJSON(const llvm::json::Value &json);
+  void Send(const protocol::Message &message);
 
   void SendOutput(OutputType o, const llvm::StringRef output);
 
@@ -324,10 +323,10 @@ struct DAP {
   bool HandleObject(const protocol::Message &M);
 
   /// Disconnect the DAP session.
-  lldb::SBError Disconnect();
+  llvm::Error Disconnect();
 
   /// Disconnect the DAP session and optionally terminate the debuggee.
-  lldb::SBError Disconnect(bool terminateDebuggee);
+  llvm::Error Disconnect(bool terminateDebuggee);
 
   /// Send a "terminated" event to indicate the process is done being debugged.
   void SendTerminatedEvent();

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 {

lldb/tools/lldb-dap/Handler/DisconnectRequestHandler.cpp

@@ -7,70 +7,27 @@
 //===----------------------------------------------------------------------===//
 
 #include "DAP.h"
-#include "EventHelper.h"
-#include "JSONUtils.h"
+#include "Protocol.h"
 #include "RequestHandler.h"
+#include "llvm/Support/Error.h"
+#include <optional>
+
+using namespace llvm;
+using namespace lldb_dap::protocol;
 
 namespace lldb_dap {
 
-// "DisconnectRequest": {
-//   "allOf": [ { "$ref": "#/definitions/Request" }, {
-//     "type": "object",
-//     "description": "Disconnect request; value of command field is
-//                     'disconnect'.",
-//     "properties": {
-//       "command": {
-//         "type": "string",
-//         "enum": [ "disconnect" ]
-//       },
-//       "arguments": {
-//         "$ref": "#/definitions/DisconnectArguments"
-//       }
-//     },
-//     "required": [ "command" ]
-//   }]
-// },
-// "DisconnectArguments": {
-//   "type": "object",
-//   "description": "Arguments for 'disconnect' request.",
-//   "properties": {
-//     "terminateDebuggee": {
-//       "type": "boolean",
-//       "description": "Indicates whether the debuggee should be terminated
-//                       when the debugger is disconnected. If unspecified,
-//                       the debug adapter is free to do whatever it thinks
-//                       is best. A client can only rely on this attribute
-//                       being properly honored if a debug adapter returns
-//                       true for the 'supportTerminateDebuggee' capability."
-//     },
-//     "restart": {
-//       "type": "boolean",
-//       "description": "Indicates whether the debuggee should be restart
-//                       the process."
-//     }
-//   }
-// },
-// "DisconnectResponse": {
-//   "allOf": [ { "$ref": "#/definitions/Response" }, {
-//     "type": "object",
-//     "description": "Response to 'disconnect' request. This is just an
-//                     acknowledgement, so no body field is required."
-//   }]
-// }
-void DisconnectRequestHandler::operator()(
-    const llvm::json::Object &request) const {
-  llvm::json::Object response;
-  FillResponse(request, response);
-  const auto *arguments = request.getObject("arguments");
+/// Disconnect request; value of command field is 'disconnect'.
+Expected<DisconnectResponse> DisconnectRequestHandler::Run(
+    const std::optional<DisconnectArguments> &arguments) const {
+  bool terminateDebuggee = dap.is_attach ? false : true;
 
-  bool defaultTerminateDebuggee = dap.is_attach ? false : true;
-  bool terminateDebuggee = GetBoolean(arguments, "terminateDebuggee")
-                               .value_or(defaultTerminateDebuggee);
+  if (arguments && arguments->terminateDebuggee)
+    terminateDebuggee = *arguments->terminateDebuggee;
 
-  lldb::SBError error = dap.Disconnect(terminateDebuggee);
-  if (error.Fail())
-    EmplaceSafeString(response, "error", error.GetCString());
+  if (Error error = dap.Disconnect(terminateDebuggee))
+    return error;
 
-  dap.SendJSON(llvm::json::Value(std::move(response)));
+  return DisconnectResponse();
 }
 } // namespace lldb_dap

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