[lldb-dap] Migrating breakpointLocations request to use typed RequestHandler (llvm#137426)

* Migrating breakpointLocations request to use typed RequestHandle 
* Preliminary step in order to implement assembly source breakpoints

Author: eronnen

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

@@ -9,136 +9,22 @@
 #include "DAP.h"
 #include "JSONUtils.h"
 #include "RequestHandler.h"
+#include <vector>
 
 namespace lldb_dap {
 
-// "BreakpointLocationsRequest": {
-//   "allOf": [ { "$ref": "#/definitions/Request" }, {
-//     "type": "object",
-//     "description": "The `breakpointLocations` request returns all possible
-//     locations for source breakpoints in a given range.\nClients should only
-//     call this request if the corresponding capability
-//     `supportsBreakpointLocationsRequest` is true.",
-//     "properties": {
-//       "command": {
-//         "type": "string",
-//         "enum": [ "breakpointLocations" ]
-//       },
-//       "arguments": {
-//         "$ref": "#/definitions/BreakpointLocationsArguments"
-//       }
-//     },
-//     "required": [ "command" ]
-//   }]
-// },
-// "BreakpointLocationsArguments": {
-//   "type": "object",
-//   "description": "Arguments for `breakpointLocations` request.",
-//   "properties": {
-//     "source": {
-//       "$ref": "#/definitions/Source",
-//       "description": "The source location of the breakpoints; either
-//       `source.path` or `source.sourceReference` must be specified."
-//     },
-//     "line": {
-//       "type": "integer",
-//       "description": "Start line of range to search possible breakpoint
-//       locations in. If only the line is specified, the request returns all
-//       possible locations in that line."
-//     },
-//     "column": {
-//       "type": "integer",
-//       "description": "Start position within `line` to search possible
-//       breakpoint locations in. It is measured in UTF-16 code units and the
-//       client capability `columnsStartAt1` determines whether it is 0- or
-//       1-based. If no column is given, the first position in the start line is
-//       assumed."
-//     },
-//     "endLine": {
-//       "type": "integer",
-//       "description": "End line of range to search possible breakpoint
-//       locations in. If no end line is given, then the end line is assumed to
-//       be the start line."
-//     },
-//     "endColumn": {
-//       "type": "integer",
-//       "description": "End position within `endLine` to search possible
-//       breakpoint locations in. It is measured in UTF-16 code units and the
-//       client capability `columnsStartAt1` determines whether it is 0- or
-//       1-based. If no end column is given, the last position in the end line
-//       is assumed."
-//     }
-//   },
-//   "required": [ "source", "line" ]
-// },
-// "BreakpointLocationsResponse": {
-//   "allOf": [ { "$ref": "#/definitions/Response" }, {
-//     "type": "object",
-//     "description": "Response to `breakpointLocations` request.\nContains
-//     possible locations for source breakpoints.",
-//     "properties": {
-//       "body": {
-//         "type": "object",
-//         "properties": {
-//           "breakpoints": {
-//             "type": "array",
-//             "items": {
-//               "$ref": "#/definitions/BreakpointLocation"
-//             },
-//             "description": "Sorted set of possible breakpoint locations."
-//           }
-//         },
-//         "required": [ "breakpoints" ]
-//       }
-//     },
-//     "required": [ "body" ]
-//   }]
-// },
-// "BreakpointLocation": {
-//   "type": "object",
-//   "description": "Properties of a breakpoint location returned from the
-//   `breakpointLocations` request.",
-//   "properties": {
-//     "line": {
-//       "type": "integer",
-//       "description": "Start line of breakpoint location."
-//     },
-//     "column": {
-//       "type": "integer",
-//       "description": "The start position of a breakpoint location. Position
-//       is measured in UTF-16 code units and the client capability
-//       `columnsStartAt1` determines whether it is 0- or 1-based."
-//     },
-//     "endLine": {
-//       "type": "integer",
-//       "description": "The end line of breakpoint location if the location
-//       covers a range."
-//     },
-//     "endColumn": {
-//       "type": "integer",
-//       "description": "The end position of a breakpoint location (if the
-//       location covers a range). Position is measured in UTF-16 code units and
-//       the client capability `columnsStartAt1` determines whether it is 0- or
-//       1-based."
-//     }
-//   },
-//   "required": [ "line" ]
-// },
-void BreakpointLocationsRequestHandler::operator()(
-    const llvm::json::Object &request) const {
-  llvm::json::Object response;
-  FillResponse(request, response);
-  auto *arguments = request.getObject("arguments");
-  auto *source = arguments->getObject("source");
-  std::string path = GetString(source, "path").value_or("").str();
-  const auto start_line = GetInteger<uint64_t>(arguments, "line")
-                              .value_or(LLDB_INVALID_LINE_NUMBER);
-  const auto start_column = GetInteger<uint64_t>(arguments, "column")
-                                .value_or(LLDB_INVALID_COLUMN_NUMBER);
-  const auto end_line =
-      GetInteger<uint64_t>(arguments, "endLine").value_or(start_line);
-  const auto end_column = GetInteger<uint64_t>(arguments, "endColumn")
-                              .value_or(std::numeric_limits<uint64_t>::max());
+/// The `breakpointLocations` request returns all possible locations for source
+/// breakpoints in a given range. Clients should only call this request if the
+/// corresponding capability `supportsBreakpointLocationsRequest` is true.
+llvm::Expected<protocol::BreakpointLocationsResponseBody>
+BreakpointLocationsRequestHandler::Run(
+    const protocol::BreakpointLocationsArguments &args) const {
+  std::string path = args.source.path.value_or("");
+  uint32_t start_line = args.line;
+  uint32_t start_column = args.column.value_or(LLDB_INVALID_COLUMN_NUMBER);
+  uint32_t end_line = args.endLine.value_or(start_line);
+  uint32_t end_column =
+      args.endColumn.value_or(std::numeric_limits<uint32_t>::max());
 
   lldb::SBFileSpec file_spec(path.c_str(), true);
   lldb::SBSymbolContextList compile_units =
@@ -191,18 +77,16 @@ void BreakpointLocationsRequestHandler::operator()(
   std::sort(locations.begin(), locations.end());
   locations.erase(llvm::unique(locations), locations.end());
 
-  llvm::json::Array locations_json;
+  std::vector<protocol::BreakpointLocation> breakpoint_locations;
   for (auto &l : locations) {
-    llvm::json::Object location;
-    location.try_emplace("line", l.first);
-    location.try_emplace("column", l.second);
-    locations_json.emplace_back(std::move(location));
+    protocol::BreakpointLocation lc;
+    lc.line = l.first;
+    lc.column = l.second;
+    breakpoint_locations.push_back(std::move(lc));
   }
 
-  llvm::json::Object body;
-  body.try_emplace("breakpoints", std::move(locations_json));
-  response.try_emplace("body", std::move(body));
-  dap.SendJSON(llvm::json::Value(std::move(response)));
+  return protocol::BreakpointLocationsResponseBody{
+      /*breakpoints=*/std::move(breakpoint_locations)};
 }
 
 } // namespace lldb_dap

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

@@ -206,14 +206,18 @@ class AttachRequestHandler
   void PostRun() const override;
 };
 
-class BreakpointLocationsRequestHandler : public LegacyRequestHandler {
+class BreakpointLocationsRequestHandler
+    : public RequestHandler<
+          protocol::BreakpointLocationsArguments,
+          llvm::Expected<protocol::BreakpointLocationsResponseBody>> {
 public:
-  using LegacyRequestHandler::LegacyRequestHandler;
+  using RequestHandler::RequestHandler;
   static llvm::StringLiteral GetCommand() { return "breakpointLocations"; }
   FeatureSet GetSupportedFeatures() const override {
     return {protocol::eAdapterFeatureBreakpointLocationsRequest};
   }
-  void operator()(const llvm::json::Object &request) const override;
+  llvm::Expected<protocol::BreakpointLocationsResponseBody>
+  Run(const protocol::BreakpointLocationsArguments &args) const override;
 };
 
 class CompletionsRequestHandler : public LegacyRequestHandler {

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

@@ -248,6 +248,23 @@ bool fromJSON(const json::Value &Params, Configuration &C, json::Path P) {
          parseTimeout(Params, C.timeout, P);
 }
 
+bool fromJSON(const json::Value &Params, BreakpointLocationsArguments &BLA,
+              json::Path P) {
+  json::ObjectMapper O(Params, P);
+  return O && O.map("source", BLA.source) && O.map("line", BLA.line) &&
+         O.mapOptional("column", BLA.column) &&
+         O.mapOptional("endLine", BLA.endLine) &&
+         O.mapOptional("endColumn", BLA.endColumn);
+}
+
+llvm::json::Value toJSON(const BreakpointLocationsResponseBody &BLRB) {
+  llvm::json::Array breakpoints_json;
+  for (const auto &breakpoint : BLRB.breakpoints) {
+    breakpoints_json.push_back(toJSON(breakpoint));
+  }
+  return llvm::json::Object{{"breakpoints", std::move(breakpoints_json)}};
+}
+
 bool fromJSON(const json::Value &Params, LaunchRequestArguments &LRA,
               json::Path P) {
   json::ObjectMapper O(Params, P);

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

@@ -31,6 +31,7 @@
 #include <cstdint>
 #include <optional>
 #include <string>
+#include <vector>
 
 namespace lldb_dap::protocol {
 
@@ -521,6 +522,43 @@ bool fromJSON(const llvm::json::Value &, StepOutArguments &, llvm::json::Path);
 /// body field is required.
 using StepOutResponse = VoidResponse;
 
+/// Arguments for `breakpointLocations` request.
+struct BreakpointLocationsArguments {
+  /// The source location of the breakpoints; either `source.path` or
+  /// `source.sourceReference` must be specified.
+  Source source;
+
+  /// Start line of range to search possible breakpoint locations in. If only
+  /// the line is specified, the request returns all possible locations in that
+  /// line.
+  uint32_t line;
+
+  /// Start position within `line` to search possible breakpoint locations in.
+  /// It is measured in UTF-16 code units and the client capability
+  /// `columnsStartAt1` determines whether it is 0- or 1-based. If no column is
+  /// given, the first position in the start line is assumed.
+  std::optional<uint32_t> column;
+
+  /// End line of range to search possible breakpoint locations in. If no end
+  /// line is given, then the end line is assumed to be the start line.
+  std::optional<uint32_t> endLine;
+
+  /// End position within `endLine` to search possible breakpoint locations in.
+  /// It is measured in UTF-16 code units and the client capability
+  /// `columnsStartAt1` determines whether it is 0- or 1-based. If no end column
+  /// is given, the last position in the end line is assumed.
+  std::optional<uint32_t> endColumn;
+};
+bool fromJSON(const llvm::json::Value &, BreakpointLocationsArguments &,
+              llvm::json::Path);
+
+/// Response to `breakpointLocations` request.
+struct BreakpointLocationsResponseBody {
+  /// Content of the source reference.
+  std::vector<BreakpointLocation> breakpoints;
+};
+llvm::json::Value toJSON(const BreakpointLocationsResponseBody &);
+
 } // namespace lldb_dap::protocol
 
 #endif

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

@@ -260,4 +260,18 @@ bool fromJSON(const llvm::json::Value &Params, ValueFormat &VF,
   return O && O.mapOptional("hex", VF.hex);
 }
 
+json::Value toJSON(const BreakpointLocation &B) {
+  json::Object result;
+
+  result.insert({"line", B.line});
+  if (B.column)
+    result.insert({"column", *B.column});
+  if (B.endLine)
+    result.insert({"endLine", *B.endLine});
+  if (B.endColumn)
+    result.insert({"endColumn", *B.endColumn});
+
+  return result;
+}
+
 } // namespace lldb_dap::protocol

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

@@ -329,6 +329,27 @@ struct ValueFormat {
 };
 bool fromJSON(const llvm::json::Value &, ValueFormat &, llvm::json::Path);
 
+/// Properties of a breakpoint location returned from the `breakpointLocations`
+/// request.
+struct BreakpointLocation {
+  /// Start line of breakpoint location.
+  uint32_t line;
+
+  /// The start position of a breakpoint location. Position is measured in
+  /// UTF-16 code units and the client capability `columnsStartAt1` determines
+  /// whether it is 0- or 1-based.
+  std::optional<uint32_t> column;
+
+  /// The end line of breakpoint location if the location covers a range.
+  std::optional<uint32_t> endLine;
+
+  /// The end position of a breakpoint location (if the location covers a
+  /// range). Position is measured in UTF-16 code units and the client
+  /// capability `columnsStartAt1` determines whether it is 0- or 1-based.
+  std::optional<uint32_t> endColumn;
+};
+llvm::json::Value toJSON(const BreakpointLocation &);
+
 } // namespace lldb_dap::protocol
 
 #endif

llvm/include/llvm/Support/JSON.h

@@ -776,6 +776,14 @@ inline bool fromJSON(const Value &E, bool &Out, Path P) {
   P.report("expected boolean");
   return false;
 }
+inline bool fromJSON(const Value &E, unsigned int &Out, Path P) {
+  if (auto S = E.getAsInteger()) {
+    Out = *S;
+    return true;
+  }
+  P.report("expected integer");
+  return false;
+}
 inline bool fromJSON(const Value &E, uint64_t &Out, Path P) {
   if (auto S = E.getAsUINT64()) {
     Out = *S;