https://github.com/eronnen updated https://github.com/llvm/llvm-project/pull/137426
>From 85c1e0de0a5cb8dd09b6f84b2514c7eb06aadac2 Mon Sep 17 00:00:00 2001 From: Ely Ronnen <elyron...@gmail.com> Date: Sat, 26 Apr 2025 01:22:05 +0200 Subject: [PATCH] Migrating breakpointLocations request to use typed RequestHandler --- .../Handler/BreakpointLocationsHandler.cpp | 156 ++---------------- lldb/tools/lldb-dap/Handler/RequestHandler.h | 10 +- .../lldb-dap/Protocol/ProtocolRequests.cpp | 17 ++ .../lldb-dap/Protocol/ProtocolRequests.h | 38 +++++ .../tools/lldb-dap/Protocol/ProtocolTypes.cpp | 14 ++ lldb/tools/lldb-dap/Protocol/ProtocolTypes.h | 21 +++ llvm/include/llvm/Support/JSON.h | 8 + 7 files changed, 122 insertions(+), 142 deletions(-) diff --git a/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp b/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp index 7a477f3e97875..537f1e7b76f1d 100644 --- a/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp @@ -9,136 +9,24 @@ #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::vector<protocol::BreakpointLocation> breakpoint_locations; + + 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 +79,8 @@ void BreakpointLocationsRequestHandler::operator()( std::sort(locations.begin(), locations.end()); locations.erase(llvm::unique(locations), locations.end()); - llvm::json::Array locations_json; - 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)); - } - - 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 diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index fa3d76ed4a125..d4d98220c21d2 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -204,14 +204,18 @@ class AttachRequestHandler : public LegacyRequestHandler { void operator()(const llvm::json::Object &request) 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 { diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp index 61fea66490c30..a90d28742c990 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -245,6 +245,23 @@ bool fromJSON(const json::Value &Params, Configuration &C, json::Path P) { parseSourceMap(Params, C.sourceMap, 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); diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index 33f93cc38799a..fd49e585b1dd5 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -30,6 +30,7 @@ #include <cstdint> #include <optional> #include <string> +#include <vector> namespace lldb_dap::protocol { @@ -377,6 +378,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 diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp index e64998c4ca488..1056751503461 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp @@ -254,4 +254,18 @@ bool fromJSON(const llvm::json::Value &Params, SteppingGranularity &SG, return true; } +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 diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h index 54941f24efbd9..0e8710a736832 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h @@ -322,6 +322,27 @@ enum SteppingGranularity : unsigned { bool fromJSON(const llvm::json::Value &, SteppingGranularity &, 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 diff --git a/llvm/include/llvm/Support/JSON.h b/llvm/include/llvm/Support/JSON.h index 7f7f5f6228763..f1f4f4db709dd 100644 --- a/llvm/include/llvm/Support/JSON.h +++ b/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; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits