[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 11:57:04 PST 2025
https://github.com/DrSergei updated https://github.com/llvm/llvm-project/pull/171099
>From c2d2546e4631f47150db7a0957d61f108d633760 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 1/3] [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 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 95ecc7e4e7e40..354e1b5ef8210 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -701,4 +701,23 @@ 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) {
+ 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 dc84e90ae03b4..ff63c160ba917 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 = 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 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));
+}
>From 127434c8fd2df35850889e9ed9cc39e3fe143d10 Mon Sep 17 00:00:00 2001
From: Druzhkov Sergei <serzhdruzhok at gmail.com>
Date: Mon, 8 Dec 2025 14:52:48 +0300
Subject: [PATCH 2/3] Fix review comments
---
.../Handler/LocationsRequestHandler.cpp | 17 ++++++-----------
.../lldb-dap/Protocol/ProtocolRequests.cpp | 6 +++---
lldb/tools/lldb-dap/Protocol/ProtocolRequests.h | 8 ++++----
3 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp
index 10a6dcf4d8305..923c8b1aefa34 100644
--- a/lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp
@@ -46,34 +46,29 @@ LocationsRequestHandler::Run(const protocol::LocationsArguments &args) const {
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)
return llvm::make_error<DAPError>(
"Failed to resolve file path for location");
response.source = std::move(*source);
- if (int line = line_entry.GetLine())
- response.line = line;
- if (int column = line_entry.GetColumn())
- response.column = column;
+ response.line = line_entry.GetLine();
+ response.column = line_entry.GetColumn();
} else {
// Get the declaration location
lldb::SBDeclaration decl = variable.GetDeclaration();
if (!decl.IsValid())
return llvm::make_error<DAPError>("No declaration location available");
- const std::optional<protocol::Source> source =
- CreateSource(decl.GetFileSpec());
+ std::optional<protocol::Source> source = CreateSource(decl.GetFileSpec());
if (!source)
return llvm::make_error<DAPError>(
"Failed to resolve file path for location");
response.source = std::move(*source);
- if (int line = decl.GetLine())
- response.line = line;
- if (int column = decl.GetColumn())
- response.column = column;
+ response.line = decl.GetLine();
+ response.column = decl.GetColumn();
}
return response;
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index 354e1b5ef8210..6fb0cf182d222 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -710,11 +710,11 @@ bool fromJSON(const llvm::json::Value &Params, LocationsArguments &Args,
llvm::json::Value toJSON(const LocationsResponseBody &Body) {
json::Object result{{"source", Body.source}, {"line", Body.line}};
- if (Body.column)
+ if (Body.column != 0 && Body.column != LLDB_INVALID_COLUMN_NUMBER)
result.insert({"column", Body.column});
- if (Body.endLine)
+ if (Body.endLine != 0 && Body.endLine != LLDB_INVALID_LINE_NUMBER)
result.insert({"endLine", Body.endLine});
- if (Body.endColumn)
+ if (Body.endColumn != 0 && Body.endColumn != LLDB_INVALID_COLUMN_NUMBER)
result.insert({"endColumn", Body.endColumn});
return result;
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index ff63c160ba917..104520f2c24c2 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -1211,22 +1211,22 @@ struct LocationsResponseBody {
/// The line number of the location. The client capability `linesStartAt1`
/// determines whether it is 0- or 1-based.
- uint32_t line = 0;
+ 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.
- std::optional<uint32_t> column;
+ 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.
- std::optional<uint32_t> endLine;
+ 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.
- std::optional<uint32_t> endColumn;
+ uint32_t endColumn = LLDB_INVALID_COLUMN_NUMBER;
};
llvm::json::Value toJSON(const LocationsResponseBody &);
>From abee0f4aa36365cc1e4f8f6e6eef7a67d80a6ba4 Mon Sep 17 00:00:00 2001
From: Druzhkov Sergei <serzhdruzhok at gmail.com>
Date: Mon, 8 Dec 2025 22:56:32 +0300
Subject: [PATCH 3/3] Add assert
---
lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index 6fb0cf182d222..9e980141b5dc7 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -708,6 +708,7 @@ bool fromJSON(const llvm::json::Value &Params, LocationsArguments &Args,
}
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)
More information about the lldb-commits
mailing list