[lldb-dap] update gototargets to use the new protocol

Author: da-viper

lldb/test/API/tools/lldb-dap/gotoTargets/TestDAP_gotoTargets.py

@@ -63,7 +63,7 @@ def test_default(self):
         response = self.dap_server.request_goto(thread_id, target_id)
         self.assertEqual(response["success"], True, "goto request with targetId should be successful")
 
-        stopped_events = self.dap_server.wait_for_stopped()
+        stopped_events = self.dap_server.wait_for_stopped(timeout=0.200)
         is_goto = lambda event: event["body"]["reason"] == "goto"
         has_goto_event = any(map(is_goto, stopped_events))
         self.assertEqual(has_goto_event, True, "expected a stopped event with reason `goto`")
@@ -126,7 +126,7 @@ def test_execute_again(self):
         response = self.dap_server.request_goto(thread_id, target_id)
         self.assertEqual(response["success"], True, "expects success to go to targetId")
 
-        stopped_events = self.dap_server.wait_for_stopped()
+        stopped_events = self.dap_server.wait_for_stopped(timeout=0.200)  # 200ms
         is_goto = lambda event: event["body"]["reason"] == "goto"
         has_goto_event = any(map(is_goto, stopped_events))
         self.assertEqual(has_goto_event, True, "expects stopped event with reason goto")

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

@@ -9,6 +9,8 @@
 #include "DAP.h"
 #include "EventHelper.h"
 #include "JSONUtils.h"
+#include "Protocol/ProtocolRequests.h"
+#include "RequestHandler.h"
 
 namespace lldb_dap {
 
@@ -26,89 +28,35 @@ static void SendThreadGotoEvent(DAP &dap, lldb::tid_t thread_id) {
   dap.SendJSON(llvm::json::Value(std::move(event)));
 }
 
-// "GotoRequest": {
-//   "allOf": [ { "$ref": "#/definitions/Request" }, {
-//     "type": "object",
-//     "description": "The request sets the location where the debuggee will
-//     continue to run.\nThis makes it possible to skip the execution of code or
-//     to execute code again.\nThe code between the current location and the
-//     goto target is not executed but skipped.\nThe debug adapter first sends
-//     the response and then a `stopped` event with reason `goto`.\nClients
-//     should only call this request if the corresponding capability
-//     `supportsGotoTargetsRequest` is true (because only then goto targets
-//     exist that can be passed as arguments).",
-//.    "properties": {
-//       "command": {
-//         "type": "string",
-//         "enum": [ "goto" ]
-//       },
-//       "arguments": {
-//         "$ref": "#/definitions/GotoArguments"
-//       }
-//     },
-//     "required": [ "command", "arguments"  ]
-//   }]
-// }
-// "GotoArguments": {
-//   "type": "object",
-//   "description": "Arguments for `goto` request.",
-//   "properties": {
-//     "threadId": {
-//       "type": "integer",
-//       "description": "Set the goto target for this thread."
-//     },
-//     "targetId": {
-//       "type": "integer",
-//       "description": "The location where the debuggee will continue to run."
-//     }
-//   },
-//   "required": [ "threadId", "targetId" ]
-// }
-// "GotoResponse": {
-//   "allOf": [ { "$ref": "#/definitions/Response" }, {
-//     "type": "object",
-//     "description": "Response to `goto` request. This is just an
-//     acknowledgement, so no body field is required."
-//   }]
-// }
-void GoToRequestHandler::operator()(const llvm::json::Object &request) const {
-  llvm::json::Object response;
-  FillResponse(request, response);
+llvm::Expected<protocol::GotoResponseBody>
+GotoRequestHandler::Run(const protocol::GotoArguments &args) const {
+  const lldb::tid_t thread_id = args.threadId;
+  lldb::SBThread current_thread =
+      dap.target.GetProcess().GetThreadByID(thread_id);
 
-  auto SendError = [&](auto &&message) {
-    response["success"] = false;
-    response["message"] = message;
-    dap.SendJSON(llvm::json::Value(std::move(response)));
-  };
-
-  const auto *goto_arguments = request.getObject("arguments");
-  if (goto_arguments == nullptr) {
-    return SendError("Arguments is empty");
-  }
-
-  lldb::SBThread current_thread = dap.GetLLDBThread(*goto_arguments);
   if (!current_thread.IsValid()) {
-    return SendError(llvm::formatv("Thread id `{0}` is not valid",
-                                   current_thread.GetThreadID()));
+    return llvm::createStringError(llvm::formatv("Thread id `{0}` is not valid",
+                                                 current_thread.GetThreadID()));
   }
 
-  const auto target_id = GetInteger<uint64_t>(goto_arguments, "targetId");
-  const auto line_entry = dap.gotos.GetLineEntry(target_id.value());
-  if (!target_id || !line_entry) {
-    return SendError(llvm::formatv("Target id `{0}` is not valid",
-                                   current_thread.GetThreadID()));
+  const uint64_t target_id = args.targetId;
+  const std::optional<lldb::SBLineEntry> line_entry =
+      dap.gotos.GetLineEntry(target_id);
+  if (!line_entry) {
+    return llvm::createStringError(
+        llvm::formatv("Target id `{0}` is not valid", thread_id));
   }
 
-  auto file_spec = line_entry->GetFileSpec();
-  const auto error =
+  lldb::SBFileSpec file_spec = line_entry->GetFileSpec();
+  const lldb::SBError error =
       current_thread.JumpToLine(file_spec, line_entry->GetLine());
+
   if (error.Fail()) {
-    return SendError(error.GetCString());
+    return llvm::createStringError(error.GetCString());
   }
 
-  dap.SendJSON(llvm::json::Value(std::move(response)));
-
-  SendThreadGotoEvent(dap, current_thread.GetThreadID());
+  SendThreadGotoEvent(dap, thread_id);
+  return protocol::GotoResponseBody();
 }
 
 } // namespace lldb_dap

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

@@ -9,6 +9,8 @@
 #include "DAP.h"
 
 #include "JSONUtils.h"
+#include "Protocol/ProtocolRequests.h"
+#include "RequestHandler.h"
 
 #include <lldb/API/SBBreakpointLocation.h>
 #include <lldb/API/SBListener.h>
@@ -20,7 +22,7 @@ static llvm::SmallVector<lldb::SBLineEntry>
 GetLineValidEntry(DAP &dap, const lldb::SBFileSpec &file_spec, uint32_t line) {
   // Disable breakpoint listeners so they do not send events to the DAP client.
   lldb::SBListener listener = dap.debugger.GetListener();
-  lldb::SBBroadcaster broadcaster = dap.target.GetBroadcaster();
+  const lldb::SBBroadcaster broadcaster = dap.target.GetBroadcaster();
   constexpr auto event_mask = lldb::SBTarget::eBroadcastBitBreakpointChanged;
   listener.StopListeningForEvents(broadcaster, event_mask);
 
@@ -55,111 +57,35 @@ GetLineValidEntry(DAP &dap, const lldb::SBFileSpec &file_spec, uint32_t line) {
   return entry_locations;
 }
 
-//  "GotoTargetsRequest": {
-//    "allOf": [ { "$ref": "#/definitions/Request" }, {
-//      "type": "object",
-//      "description": "This request retrieves the possible goto targets for the
-//      specified source location.\nThese targets can be used in the `goto`
-//      request.\nClients should only call this request if the corresponding
-//      capability `supportsGotoTargetsRequest` is true.",
-//.     "properties": {
-//        "command": {
-//          "type": "string",
-//          "enum": [ "gotoTargets" ]
-//        },
-//        "arguments": {
-//          "$ref": "#/definitions/GotoTargetsArguments"
-//        }
-//      },
-//      "required": [ "command", "arguments"  ]
-//    }]
-//  },
-//  "GotoTargetsArguments": {
-//    "type": "object",
-//    "description": "Arguments for `gotoTargets` request.",
-//    "properties": {
-//      "source": {
-//        "$ref": "#/definitions/Source",
-//        "description": "The source location for which the goto targets are
-//        determined."
-//      },
-//      "line": {
-//        "type": "integer",
-//        "description": "The line location for which the goto targets are
-//        determined."
-//      },
-//      "column": {
-//        "type": "integer",
-//        "description": "The position within `line` for which the goto targets
-//        are determined. It is measured in UTF-16 code units and the client
-//        capability `columnsStartAt1` determines whether it is 0- or 1-based."
-//      }
-//    },
-//    "required": [ "source", "line" ]
-//  },
-//  "GotoTargetsResponse": {
-//    "allOf": [ { "$ref": "#/definitions/Response" }, {
-//      "type": "object",
-//      "description": "Response to `gotoTargets` request.",
-//      "properties": {
-//        "body": {
-//          "type": "object",
-//          "properties": {
-//            "targets": {
-//              "type": "array",
-//              "items": {
-//                "$ref": "#/definitions/GotoTarget"
-//              },
-//              "description": "The possible goto targets of the specified
-//              location."
-//            }
-//          },
-//          "required": [ "targets" ]
-//        }
-//      },
-//      "required": [ "body" ]
-//    }]
-//  },
-void GoToTargetsRequestHandler::operator()(
-    const llvm::json::Object &request) const {
-  llvm::json::Object response;
-  FillResponse(request, response);
-
-  const llvm::json::Object *arguments = request.getObject("arguments");
-  const llvm::json::Object *source = arguments->getObject("source");
-  const std::string path = GetString(source, "path").str();
-  const lldb::SBFileSpec file_spec(path.c_str(), true);
-  const uint64_t goto_line =
-      GetInteger<uint64_t>(arguments, "line").value_or(1U);
-
-  llvm::json::Object body;
+/// GotoTargets request; value of command field is 'gotoTargets'.
+llvm::Expected<protocol::GotoTargetsResponseBody>
+GotoTargetsRequestHandler::Run(
+    const protocol::GotoTargetsArguments &args) const {
+  const lldb::SBFileSpec file_spec(args.source.path.value_or("").c_str(), true);
+  const uint64_t goto_line = args.line;
 
   llvm::SmallVector<lldb::SBLineEntry> goto_locations =
       GetLineValidEntry(dap, file_spec, goto_line);
-  if (goto_locations.empty()) {
-    response["success"] = false;
-    response["message"] = "Invalid jump location";
-  } else {
-    llvm::json::Array response_targets;
-    for (lldb::SBLineEntry &line_entry : goto_locations) {
-      const uint64_t target_id = dap.gotos.InsertLineEntry(line_entry);
-      const uint32_t target_line = line_entry.GetLine();
-      auto target = llvm::json::Object();
-      target.try_emplace("id", target_id);
-
-      lldb::SBStream stream;
-      line_entry.GetDescription(stream);
-      target.try_emplace("label",
-                         llvm::StringRef(stream.GetData(), stream.GetSize()));
-      target.try_emplace("line", target_line);
-      response_targets.push_back(std::move(target));
-    }
-
-    body.try_emplace("targets", std::move(response_targets));
+
+  if (goto_locations.empty())
+    return llvm::createStringError("Invalid jump location");
+
+  protocol::GotoTargetsResponseBody body{};
+
+  for (lldb::SBLineEntry &line_entry : goto_locations) {
+    const uint64_t target_id = dap.gotos.InsertLineEntry(line_entry);
+    const uint32_t target_line = line_entry.GetLine();
+    protocol::GotoTarget target{};
+    target.id = target_id;
+
+    lldb::SBStream stream;
+    line_entry.GetDescription(stream);
+    target.label = std::string(stream.GetData(), stream.GetSize());
+    target.line = target_line;
+    body.targets.emplace_back(target);
   }
 
-  response.try_emplace("body", std::move(body));
-  dap.SendJSON(llvm::json::Value(std::move(response)));
+  return body;
 }
 
 } // namespace lldb_dap

lldb/tools/lldb-dap/Handler/RequestHandler.h

@@ -215,18 +215,26 @@ class ExceptionInfoRequestHandler : public LegacyRequestHandler {
   void operator()(const llvm::json::Object &request) const override;
 };
 
-class GoToRequestHandler : public LegacyRequestHandler {
+class GotoRequestHandler final
+    : public RequestHandler<protocol::GotoArguments,
+                            protocol::GotoResponseBody> {
 public:
-  using LegacyRequestHandler::LegacyRequestHandler;
-  static llvm::StringLiteral getCommand() { return "goto"; }
-  void operator()(const llvm::json::Object &request) const override;
+  using RequestHandler::RequestHandler;
+  static llvm::StringLiteral GetCommand() { return "goto"; }
+
+  llvm::Expected<protocol::GotoResponseBody>
+  Run(const protocol::GotoArguments &args) const override;
 };
 
-class GoToTargetsRequestHandler : public LegacyRequestHandler {
+class GotoTargetsRequestHandler final
+    : public RequestHandler<protocol::GotoTargetsArguments,
+                            protocol::GotoTargetsResponseBody> {
 public:
-  using LegacyRequestHandler::LegacyRequestHandler;
-  static llvm::StringLiteral getCommand() { return "gotoTargets"; }
-  void operator()(const llvm::json::Object &request) const override;
+  using RequestHandler::RequestHandler;
+  static llvm::StringLiteral GetCommand() { return "gotoTargets"; }
+
+  llvm::Expected<protocol::GotoTargetsResponseBody>
+  Run(const protocol::GotoTargetsArguments &args) const override;
 };
 
 class InitializeRequestHandler : public LegacyRequestHandler {

lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp

@@ -22,6 +22,26 @@ bool fromJSON(const json::Value &Params, DisconnectArguments &DA,
          O.mapOptional("terminateDebuggee", DA.terminateDebuggee) &&
          O.mapOptional("suspendDebuggee", DA.suspendDebuggee);
 }
+bool fromJSON(const llvm::json::Value &Params, GotoArguments &GA,
+              llvm::json::Path P) {
+  json::ObjectMapper O(Params, P);
+  return O && O.map("targetId", GA.targetId) && O.map("threadId", GA.threadId);
+}
+
+bool fromJSON(const llvm::json::Value &Params, GotoTargetsArguments &GTA,
+              llvm::json::Path P) {
+  json::ObjectMapper O(Params, P);
+  return O && O.map("source", GTA.source) && O.map("line", GTA.line) &&
+         O.mapOptional("column", GTA.column);
+}
+
+llvm::json::Value toJSON(const GotoTargetsResponseBody &GTA) {
+  json::Array targets;
+  for (const auto &target : GTA.targets) {
+    targets.emplace_back(target);
+  }
+  return json::Object{{"targets", std::move(targets)}};
+}
 
 bool fromJSON(const json::Value &Params, SourceArguments &SA, json::Path P) {
   json::ObjectMapper O(Params, P);

lldb/tools/lldb-dap/Protocol/ProtocolRequests.h

@@ -54,6 +54,43 @@ bool fromJSON(const llvm::json::Value &, DisconnectArguments &,
 /// body field is required.
 using DisconnectResponse = VoidResponse;
 
+/// Arguments for `goto` request.
+struct GotoArguments {
+  /// Set the goto target for this thread.
+  uint64_t threadId;
+
+  /// The location where the debuggee will continue to run.
+  uint64_t targetId;
+};
+bool fromJSON(const llvm::json::Value &, GotoArguments &, llvm::json::Path);
+
+/// Response to goto request. This is just an acknowledgement, so no
+/// body field is required.
+using GotoResponseBody = VoidResponse;
+
+/// Arguments for `gotoTargets` request.
+struct GotoTargetsArguments {
+  /// The source location for which the goto targets are determined.
+  Source source;
+
+  /// The line location for which the goto targets are determined.
+  uint64_t line;
+
+  /// The position within `line` for which the goto targets are determined. It
+  /// is
+  ///  measured in UTF-16 code units and the client capability `columnsStartAt1`
+  ///  determines whether it is 0- or 1-based.
+  std::optional<uint64_t> column;
+};
+bool fromJSON(const llvm::json::Value &, GotoTargetsArguments &,
+              llvm::json::Path);
+
+struct GotoTargetsResponseBody {
+  /// The possible goto targets of the specified location.
+  llvm::SmallVector<GotoTarget> targets;
+};
+llvm::json::Value toJSON(const GotoTargetsResponseBody &);
+
 /// Arguments for `source` request.
 struct SourceArguments {
   /// Specifies the source content to load. Either `source.path` or