migrate breakpoints requests handler implementations

Author: eronnen

lldb/tools/lldb-dap/Breakpoint.cpp

@@ -14,7 +14,6 @@
 #include "lldb/API/SBLineEntry.h"
 #include "lldb/API/SBMutex.h"
 #include "llvm/ADT/StringExtras.h"
-#include "llvm/Support/JSON.h"
 #include <cstddef>
 #include <cstdint>
 #include <mutex>
@@ -30,13 +29,16 @@ void Breakpoint::SetHitCondition() {
     m_bp.SetIgnoreCount(hitCount - 1);
 }
 
-void Breakpoint::CreateJsonObject(llvm::json::Object &object) {
+protocol::Breakpoint Breakpoint::ToProtocolBreakpoint() {
+  protocol::Breakpoint breakpoint;
+
   // Each breakpoint location is treated as a separate breakpoint for VS code.
   // They don't have the notion of a single breakpoint with multiple locations.
   if (!m_bp.IsValid())
-    return;
-  object.try_emplace("verified", m_bp.GetNumResolvedLocations() > 0);
-  object.try_emplace("id", m_bp.GetID());
+    return breakpoint;
+
+  breakpoint.verified = m_bp.GetNumResolvedLocations() > 0;
+  breakpoint.id = m_bp.GetID();
   // VS Code DAP doesn't currently allow one breakpoint to have multiple
   // locations so we just report the first one. If we report all locations
   // then the IDE starts showing the wrong line numbers and locations for
@@ -60,16 +62,18 @@ void Breakpoint::CreateJsonObject(llvm::json::Object &object) {
   if (bp_addr.IsValid()) {
     std::string formatted_addr =
         "0x" + llvm::utohexstr(bp_addr.GetLoadAddress(m_bp.GetTarget()));
-    object.try_emplace("instructionReference", formatted_addr);
+    breakpoint.instructionReference = formatted_addr;
     auto line_entry = bp_addr.GetLineEntry();
     const auto line = line_entry.GetLine();
     if (line != UINT32_MAX)
-      object.try_emplace("line", line);
+      breakpoint.line = line;
     const auto column = line_entry.GetColumn();
     if (column != 0)
-      object.try_emplace("column", column);
-    object.try_emplace("source", CreateSource(line_entry));
+      breakpoint.column = column;
+    breakpoint.source = CreateSource(line_entry);
   }
+
+  return breakpoint;
 }
 
 bool Breakpoint::MatchesName(const char *name) {

lldb/tools/lldb-dap/Breakpoint.h

@@ -26,7 +26,8 @@ class Breakpoint : public BreakpointBase {
 
   void SetCondition() override;
   void SetHitCondition() override;
-  void CreateJsonObject(llvm::json::Object &object) override;
+
+  protocol::Breakpoint ToProtocolBreakpoint() override;
 
   bool MatchesName(const char *name);
   void SetBreakpoint();

lldb/tools/lldb-dap/BreakpointBase.cpp

@@ -7,8 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "BreakpointBase.h"
-#include "JSONUtils.h"
-#include "llvm/ADT/StringRef.h"
 
 using namespace lldb_dap;
 

lldb/tools/lldb-dap/BreakpointBase.h

@@ -10,6 +10,7 @@
 #define LLDB_TOOLS_LLDB_DAP_BREAKPOINTBASE_H
 
 #include "DAPForward.h"
+#include "Protocol/ProtocolTypes.h"
 #include <optional>
 #include <string>
 
@@ -24,7 +25,7 @@ class BreakpointBase {
 
   virtual void SetCondition() = 0;
   virtual void SetHitCondition() = 0;
-  virtual void CreateJsonObject(llvm::json::Object &object) = 0;
+  virtual protocol::Breakpoint ToProtocolBreakpoint() = 0;
 
   void UpdateBreakpoint(const BreakpointBase &request_bp);
 

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

@@ -9,153 +9,49 @@
 #include "DAP.h"
 #include "EventHelper.h"
 #include "JSONUtils.h"
+#include "Protocol/ProtocolRequests.h"
 #include "RequestHandler.h"
+#include <vector>
 
 namespace lldb_dap {
 
-// "SetBreakpointsRequest": {
-//   "allOf": [ { "$ref": "#/definitions/Request" }, {
-//     "type": "object",
-//     "description": "SetBreakpoints request; value of command field is
-//     'setBreakpoints'. Sets multiple breakpoints for a single source and
-//     clears all previous breakpoints in that source. To clear all breakpoint
-//     for a source, specify an empty array. When a breakpoint is hit, a
-//     StoppedEvent (event type 'breakpoint') is generated.", "properties": {
-//       "command": {
-//         "type": "string",
-//         "enum": [ "setBreakpoints" ]
-//       },
-//       "arguments": {
-//         "$ref": "#/definitions/SetBreakpointsArguments"
-//       }
-//     },
-//     "required": [ "command", "arguments"  ]
-//   }]
-// },
-// "SetBreakpointsArguments": {
-//   "type": "object",
-//   "description": "Arguments for 'setBreakpoints' request.",
-//   "properties": {
-//     "source": {
-//       "$ref": "#/definitions/Source",
-//       "description": "The source location of the breakpoints; either
-//       source.path or source.reference must be specified."
-//     },
-//     "breakpoints": {
-//       "type": "array",
-//       "items": {
-//         "$ref": "#/definitions/SourceBreakpoint"
-//       },
-//       "description": "The code locations of the breakpoints."
-//     },
-//     "lines": {
-//       "type": "array",
-//       "items": {
-//         "type": "integer"
-//       },
-//       "description": "Deprecated: The code locations of the breakpoints."
-//     },
-//     "sourceModified": {
-//       "type": "boolean",
-//       "description": "A value of true indicates that the underlying source
-//       has been modified which results in new breakpoint locations."
-//     }
-//   },
-//   "required": [ "source" ]
-// },
-// "SetBreakpointsResponse": {
-//   "allOf": [ { "$ref": "#/definitions/Response" }, {
-//     "type": "object",
-//     "description": "Response to 'setBreakpoints' request. Returned is
-//     information about each breakpoint created by this request. This includes
-//     the actual code location and whether the breakpoint could be verified.
-//     The breakpoints returned are in the same order as the elements of the
-//     'breakpoints' (or the deprecated 'lines') in the
-//     SetBreakpointsArguments.", "properties": {
-//       "body": {
-//         "type": "object",
-//         "properties": {
-//           "breakpoints": {
-//             "type": "array",
-//             "items": {
-//               "$ref": "#/definitions/Breakpoint"
-//             },
-//             "description": "Information about the breakpoints. The array
-//             elements are in the same order as the elements of the
-//             'breakpoints' (or the deprecated 'lines') in the
-//             SetBreakpointsArguments."
-//           }
-//         },
-//         "required": [ "breakpoints" ]
-//       }
-//     },
-//     "required": [ "body" ]
-//   }]
-// },
-// "SourceBreakpoint": {
-//   "type": "object",
-//   "description": "Properties of a breakpoint or logpoint passed to the
-//   setBreakpoints request.", "properties": {
-//     "line": {
-//       "type": "integer",
-//       "description": "The source line of the breakpoint or logpoint."
-//     },
-//     "column": {
-//       "type": "integer",
-//       "description": "An optional source column of the breakpoint."
-//     },
-//     "condition": {
-//       "type": "string",
-//       "description": "An optional expression for conditional breakpoints."
-//     },
-//     "hitCondition": {
-//       "type": "string",
-//       "description": "An optional expression that controls how many hits of
-//       the breakpoint are ignored. The backend is expected to interpret the
-//       expression as needed."
-//     },
-//     "logMessage": {
-//       "type": "string",
-//       "description": "If this attribute exists and is non-empty, the backend
-//       must not 'break' (stop) but log the message instead. Expressions within
-//       {} are interpolated."
-//     }
-//   },
-//   "required": [ "line" ]
-// }
-void SetBreakpointsRequestHandler::operator()(
-    const llvm::json::Object &request) const {
-  llvm::json::Object response;
-  lldb::SBError error;
-  FillResponse(request, response);
-  const auto *arguments = request.getObject("arguments");
-  const auto *source = arguments->getObject("source");
-  const auto path = GetString(source, "path").value_or("");
-  const auto *breakpoints = arguments->getArray("breakpoints");
-  llvm::json::Array response_breakpoints;
+/// Sets multiple breakpoints for a single source and clears all previous
+/// breakpoints in that source. To clear all breakpoint for a source, specify an
+/// empty array. When a breakpoint is hit, a `stopped` event (with reason
+/// `breakpoint`) is generated.
+llvm::Expected<protocol::SetBreakpointsResponseBody>
+SetBreakpointsRequestHandler::Run(
+    const protocol::SetBreakpointsArguments &args) const {
+  const auto &source = args.source;
+  const auto path = source.path.value_or("");
+  std::vector<protocol::Breakpoint> response_breakpoints;
 
   // Decode the source breakpoint infos for this "setBreakpoints" request
   SourceBreakpointMap request_bps;
   // "breakpoints" may be unset, in which case we treat it the same as being set
   // to an empty array.
-  if (breakpoints) {
-    for (const auto &bp : *breakpoints) {
-      const auto *bp_obj = bp.getAsObject();
-      if (bp_obj) {
-        SourceBreakpoint src_bp(dap, *bp_obj);
-        std::pair<uint32_t, uint32_t> bp_pos(src_bp.GetLine(),
-                                             src_bp.GetColumn());
-        request_bps.try_emplace(bp_pos, src_bp);
-        const auto [iv, inserted] =
-            dap.source_breakpoints[path].try_emplace(bp_pos, src_bp);
-        // We check if this breakpoint already exists to update it
-        if (inserted)
-          iv->getSecond().SetBreakpoint(path.data());
-        else
-          iv->getSecond().UpdateBreakpoint(src_bp);
-        AppendBreakpoint(&iv->getSecond(), response_breakpoints, path,
-                         src_bp.GetLine());
-      }
+  if (args.breakpoints) {
+    for (const auto &bp : *args.breakpoints) {
+      SourceBreakpoint src_bp(dap, bp);
+      std::pair<uint32_t, uint32_t> bp_pos(src_bp.GetLine(),
+                                           src_bp.GetColumn());
+      request_bps.try_emplace(bp_pos, src_bp);
+      const auto [iv, inserted] =
+          dap.source_breakpoints[path].try_emplace(bp_pos, src_bp);
+      // We check if this breakpoint already exists to update it
+      if (inserted)
+        iv->getSecond().SetBreakpoint(path.data());
+      else
+        iv->getSecond().UpdateBreakpoint(src_bp);
+
+      protocol::Breakpoint response_bp = iv->getSecond().ToProtocolBreakpoint();
+      if (!path.empty() && !response_bp.source)
+        response_bp.source = CreateSource(path);
+      if (!response_bp.line)
+        response_bp.line = src_bp.GetLine();
+      if (!response_bp.column)
+        response_bp.column = src_bp.GetColumn();
+      response_breakpoints.push_back(response_bp);
     }
   }
 
@@ -174,10 +70,7 @@ void SetBreakpointsRequestHandler::operator()(
     }
   }
 
-  llvm::json::Object body;
-  body.try_emplace("breakpoints", std::move(response_breakpoints));
-  response.try_emplace("body", std::move(body));
-  dap.SendJSON(llvm::json::Value(std::move(response)));
+  return protocol::SetBreakpointsResponseBody{std::move(response_breakpoints)};
 }
 
 } // namespace lldb_dap