[Lldb-commits] [lldb] [lldb-dap] Migrate locations request to structured types (PR #171099)
Sergei Druzhkov via lldb-commits
lldb-commits at lists.llvm.org
Mon Dec 8 01:46:49 PST 2025
https://github.com/DrSergei created https://github.com/llvm/llvm-project/pull/171099
This patch migrates `locations` request into structured types and adds test for it.
>From 5a8427e3a9e515a47d68588d05d6147660c98b2f Mon Sep 17 00:00:00 2001
From: Druzhkov Sergei <serzhdruzhok at gmail.com>
Date: Sun, 7 Dec 2025 17:23:49 +0300
Subject: [PATCH] [lldb-dap] Migrate locations request to structured types
---
.../Handler/LocationsRequestHandler.cpp | 162 ++++--------------
lldb/tools/lldb-dap/Handler/RequestHandler.h | 9 +-
.../lldb-dap/Protocol/ProtocolRequests.cpp | 19 ++
.../lldb-dap/Protocol/ProtocolRequests.h | 35 ++++
lldb/unittests/DAP/ProtocolRequestsTest.cpp | 50 ++++++
5 files changed, 140 insertions(+), 135 deletions(-)
diff --git a/lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp
index cf9b5a3dbd06b..10a6dcf4d8305 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,64 @@
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 =
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);
+ response.source = std::move(*source);
if (int line = line_entry.GetLine())
- body.try_emplace("line", line);
+ response.line = line;
if (int column = line_entry.GetColumn())
- body.try_emplace("column", column);
+ response.column = column;
} 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;
- }
+ if (!source)
+ return llvm::make_error<DAPError>(
+ "Failed to resolve file path for location");
- body.try_emplace("source", *source);
+ response.source = std::move(*source);
if (int line = decl.GetLine())
- body.try_emplace("line", line);
+ response.line = line;
if (int column = decl.GetColumn())
- body.try_emplace("column", column);
+ response.column = column;
}
- 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 5d235352b7738..28e79933a191a 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -563,11 +563,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 0a1d580bffd68..d503baffe491b 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -695,4 +695,23 @@ llvm::json::Value toJSON(const EvaluateResponseBody &Body) {
return result;
}
+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) {
+ json::Object result{{"source", Body.source}, {"line", Body.line}};
+
+ if (Body.column)
+ result.insert({"column", Body.column});
+ if (Body.endLine)
+ result.insert({"endLine", Body.endLine});
+ if (Body.endColumn)
+ 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 6a85033ae7ef2..c9ac5c575abe3 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -1184,6 +1184,41 @@ struct EvaluateResponseBody {
};
llvm::json::Value toJSON(const EvaluateResponseBody &);
+/// 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 = 0;
+
+ /// 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.
+ std::optional<uint32_t> column;
+
+ /// 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.
+ std::optional<uint32_t> endLine;
+
+ /// 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.
+ std::optional<uint32_t> endColumn;
+};
+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 a74c369924b8e..e64aa5bf8179b 100644
--- a/lldb/unittests/DAP/ProtocolRequestsTest.cpp
+++ b/lldb/unittests/DAP/ProtocolRequestsTest.cpp
@@ -182,3 +182,53 @@ TEST(ProtocolRequestsTest, InitializeRequestArguments) {
EXPECT_THAT_EXPECTED(parse<InitializeRequestArguments>(R"({})"),
FailedWithMessage("missing value at (root).adapterID"));
}
+
+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));
+}
More information about the lldb-commits
mailing list