Author: Sergei Druzhkov Date: 2025-12-14T18:37:24+03:00 New Revision: e82edd28f6dcaeca9fa166855dd36a3c179ee151
URL: https://github.com/llvm/llvm-project/commit/e82edd28f6dcaeca9fa166855dd36a3c179ee151 DIFF: https://github.com/llvm/llvm-project/commit/e82edd28f6dcaeca9fa166855dd36a3c179ee151.diff LOG: [lldb-dap] Migrate locations request to structured types (#171099) This patch migrates `locations` request into structured types and adds test for it. Added: Modified: lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp lldb/tools/lldb-dap/Handler/RequestHandler.h lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp lldb/tools/lldb-dap/Protocol/ProtocolRequests.h lldb/unittests/DAP/ProtocolRequestsTest.cpp Removed: ################################################################################ diff --git a/lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp index cf9b5a3dbd06b..923c8b1aefa34 100644 --- a/lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "DAP.h" +#include "DAPError.h" #include "EventHelper.h" #include "JSONUtils.h" #include "LLDBUtils.h" @@ -18,167 +19,59 @@ namespace lldb_dap { -// "LocationsRequest": { -// "allOf": [ { "$ref": "#/definitions/Request" }, { -// "type": "object", -// "description": "Looks up information about a location reference -// previously returned by the debug adapter.", -// "properties": { -// "command": { -// "type": "string", -// "enum": [ "locations" ] -// }, -// "arguments": { -// "$ref": "#/definitions/LocationsArguments" -// } -// }, -// "required": [ "command", "arguments" ] -// }] -// }, -// "LocationsArguments": { -// "type": "object", -// "description": "Arguments for `locations` request.", -// "properties": { -// "locationReference": { -// "type": "integer", -// "description": "Location reference to resolve." -// } -// }, -// "required": [ "locationReference" ] -// }, -// "LocationsResponse": { -// "allOf": [ { "$ref": "#/definitions/Response" }, { -// "type": "object", -// "description": "Response to `locations` request.", -// "properties": { -// "body": { -// "type": "object", -// "properties": { -// "source": { -// "$ref": "#/definitions/Source", -// "description": "The source containing the location; either -// `source.path` or `source.sourceReference` must be -// specified." -// }, -// "line": { -// "type": "integer", -// "description": "The line number of the location. The client -// capability `linesStartAt1` determines whether it -// is 0- or 1-based." -// }, -// "column": { -// "type": "integer", -// "description": "Position of the location within the `line`. 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 the location, present if the location -// refers to a range. The client capability -// `linesStartAt1` determines whether it is 0- or -// 1-based." -// }, -// "endColumn": { -// "type": "integer", -// "description": "End position of the location within `endLine`, -// present if the location refers to a range. It is -// measured in UTF-16 code units and the client -// capability `columnsStartAt1` determines whether -// it is 0- or 1-based." -// } -// }, -// "required": [ "source", "line" ] -// } -// } -// }] -// }, -void LocationsRequestHandler::operator()( - const llvm::json::Object &request) const { - llvm::json::Object response; - FillResponse(request, response); - auto *arguments = request.getObject("arguments"); - - const auto location_id = - GetInteger<uint64_t>(arguments, "locationReference").value_or(0); +// Looks up information about a location reference previously returned by the +// debug adapter. +llvm::Expected<protocol::LocationsResponseBody> +LocationsRequestHandler::Run(const protocol::LocationsArguments &args) const { + protocol::LocationsResponseBody response; // We use the lowest bit to distinguish between value location and declaration // location - auto [var_ref, is_value_location] = UnpackLocation(location_id); + auto [var_ref, is_value_location] = UnpackLocation(args.locationReference); lldb::SBValue variable = dap.variables.GetVariable(var_ref); - if (!variable.IsValid()) { - response["success"] = false; - response["message"] = "Invalid variable reference"; - dap.SendJSON(llvm::json::Value(std::move(response))); - return; - } + if (!variable.IsValid()) + return llvm::make_error<DAPError>("Invalid variable reference"); - llvm::json::Object body; if (is_value_location) { // Get the value location if (!variable.GetType().IsPointerType() && - !variable.GetType().IsReferenceType()) { - response["success"] = false; - response["message"] = - "Value locations are only available for pointers and references"; - dap.SendJSON(llvm::json::Value(std::move(response))); - return; - } + !variable.GetType().IsReferenceType()) + return llvm::make_error<DAPError>( + "Value locations are only available for pointers and references"); lldb::addr_t raw_addr = variable.GetValueAsAddress(); lldb::SBAddress addr = dap.target.ResolveLoadAddress(raw_addr); lldb::SBLineEntry line_entry = GetLineEntryForAddress(dap.target, addr); - if (!line_entry.IsValid()) { - response["success"] = false; - response["message"] = "Failed to resolve line entry for location"; - dap.SendJSON(llvm::json::Value(std::move(response))); - return; - } + if (!line_entry.IsValid()) + return llvm::make_error<DAPError>( + "Failed to resolve line entry for location"); - const std::optional<protocol::Source> source = + std::optional<protocol::Source> source = CreateSource(line_entry.GetFileSpec()); - if (!source) { - response["success"] = false; - response["message"] = "Failed to resolve file path for location"; - dap.SendJSON(llvm::json::Value(std::move(response))); - return; - } + if (!source) + return llvm::make_error<DAPError>( + "Failed to resolve file path for location"); - body.try_emplace("source", *source); - if (int line = line_entry.GetLine()) - body.try_emplace("line", line); - if (int column = line_entry.GetColumn()) - body.try_emplace("column", column); + response.source = std::move(*source); + response.line = line_entry.GetLine(); + response.column = line_entry.GetColumn(); } else { // Get the declaration location lldb::SBDeclaration decl = variable.GetDeclaration(); - if (!decl.IsValid()) { - response["success"] = false; - response["message"] = "No declaration location available"; - dap.SendJSON(llvm::json::Value(std::move(response))); - return; - } + if (!decl.IsValid()) + return llvm::make_error<DAPError>("No declaration location available"); - const std::optional<protocol::Source> source = - CreateSource(decl.GetFileSpec()); - if (!source) { - response["success"] = false; - response["message"] = "Failed to resolve file path for location"; - dap.SendJSON(llvm::json::Value(std::move(response))); - return; - } + std::optional<protocol::Source> source = CreateSource(decl.GetFileSpec()); + if (!source) + return llvm::make_error<DAPError>( + "Failed to resolve file path for location"); - body.try_emplace("source", *source); - if (int line = decl.GetLine()) - body.try_emplace("line", line); - if (int column = decl.GetColumn()) - body.try_emplace("column", column); + response.source = std::move(*source); + response.line = decl.GetLine(); + response.column = decl.GetColumn(); } - response.try_emplace("body", std::move(body)); - dap.SendJSON(llvm::json::Value(std::move(response))); + return response; } } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index fdce33de3f680..8f42a28160751 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -564,11 +564,14 @@ class VariablesRequestHandler Run(const protocol::VariablesArguments &) const override; }; -class LocationsRequestHandler : public LegacyRequestHandler { +class LocationsRequestHandler + : public RequestHandler<protocol::LocationsArguments, + llvm::Expected<protocol::LocationsResponseBody>> { public: - using LegacyRequestHandler::LegacyRequestHandler; + using RequestHandler::RequestHandler; static llvm::StringLiteral GetCommand() { return "locations"; } - void operator()(const llvm::json::Object &request) const override; + llvm::Expected<protocol::LocationsResponseBody> + Run(const protocol::LocationsArguments &) const override; }; class DisassembleRequestHandler final diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp index dc61740f1d51e..42acc70333a7b 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -701,4 +701,24 @@ bool fromJSON(const llvm::json::Value &Params, PauseArguments &Args, return O && O.map("threadId", Args.threadId); } +bool fromJSON(const llvm::json::Value &Params, LocationsArguments &Args, + llvm::json::Path Path) { + json::ObjectMapper O(Params, Path); + return O && O.map("locationReference", Args.locationReference); +} + +llvm::json::Value toJSON(const LocationsResponseBody &Body) { + assert(Body.line != LLDB_INVALID_LINE_NUMBER); + json::Object result{{"source", Body.source}, {"line", Body.line}}; + + if (Body.column != 0 && Body.column != LLDB_INVALID_COLUMN_NUMBER) + result.insert({"column", Body.column}); + if (Body.endLine != 0 && Body.endLine != LLDB_INVALID_LINE_NUMBER) + result.insert({"endLine", Body.endLine}); + if (Body.endColumn != 0 && Body.endColumn != LLDB_INVALID_COLUMN_NUMBER) + result.insert({"endColumn", Body.endColumn}); + + return result; +} + } // namespace lldb_dap::protocol diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index dc84e90ae03b4..104520f2c24c2 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -1195,6 +1195,41 @@ bool fromJSON(const llvm::json::Value &, PauseArguments &, llvm::json::Path); /// field is required. using PauseResponse = VoidResponse; +/// Arguments for `locations` request. +struct LocationsArguments { + /// Location reference to resolve. + uint64_t locationReference = LLDB_DAP_INVALID_VALUE_LOC; +}; +bool fromJSON(const llvm::json::Value &, LocationsArguments &, + llvm::json::Path); + +/// Response to 'locations' request. +struct LocationsResponseBody { + /// The source containing the location; either `source.path` or + /// `source.sourceReference` must be specified. + Source source; + + /// The line number of the location. The client capability `linesStartAt1` + /// determines whether it is 0- or 1-based. + uint32_t line = LLDB_INVALID_LINE_NUMBER; + + /// Position of the location within the `line`. 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. + uint32_t column = LLDB_INVALID_COLUMN_NUMBER; + + /// End line of the location, present if the location refers to a range. The + /// client capability `linesStartAt1` determines whether it is 0- or 1-based. + uint32_t endLine = LLDB_INVALID_LINE_NUMBER; + + /// End position of the location within `endLine`, present if the location + /// refers to a range. It is measured in UTF-16 code units and the client + /// capability `columnsStartAt1` determines whether it is 0- or 1-based. + uint32_t endColumn = LLDB_INVALID_COLUMN_NUMBER; +}; +llvm::json::Value toJSON(const LocationsResponseBody &); + } // namespace lldb_dap::protocol #endif diff --git a/lldb/unittests/DAP/ProtocolRequestsTest.cpp b/lldb/unittests/DAP/ProtocolRequestsTest.cpp index c830690a8e4fe..50b8335ba46cc 100644 --- a/lldb/unittests/DAP/ProtocolRequestsTest.cpp +++ b/lldb/unittests/DAP/ProtocolRequestsTest.cpp @@ -193,3 +193,53 @@ TEST(ProtocolRequestsTest, PauseRequestArguments) { EXPECT_THAT_EXPECTED(parse<PauseArguments>(R"({})"), FailedWithMessage("missing value at (root).threadId")); } + +TEST(ProtocolRequestsTest, LocationsArguments) { + llvm::Expected<LocationsArguments> expected = + parse<LocationsArguments>(R"({"locationReference": 123})"); + ASSERT_THAT_EXPECTED(expected, llvm::Succeeded()); + EXPECT_EQ(expected->locationReference, 123U); + + // Check required keys. + EXPECT_THAT_EXPECTED( + parse<LocationsArguments>(R"({})"), + FailedWithMessage("missing value at (root).locationReference")); +} + +TEST(ProtocolRequestsTest, LocationsResponseBody) { + LocationsResponseBody body; + body.source.sourceReference = 123; + body.source.name = "test.cpp"; + body.line = 42; + + // Check required keys. + Expected<json::Value> expected = parse(R"({ + "source": { + "sourceReference": 123, + "name": "test.cpp" + }, + "line": 42 + })"); + + ASSERT_THAT_EXPECTED(expected, llvm::Succeeded()); + EXPECT_EQ(PrettyPrint(*expected), PrettyPrint(body)); + + // Check optional keys. + body.column = 2; + body.endLine = 43; + body.endColumn = 4; + + expected = parse(R"({ + "source": { + "sourceReference": 123, + "name": "test.cpp" + }, + "line": 42, + "column": 2, + "endLine": 43, + "endColumn": 4 + })"); + + ASSERT_THAT_EXPECTED(expected, llvm::Succeeded()); + EXPECT_EQ(PrettyPrint(*expected), PrettyPrint(body)); +} _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
