llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Sergei Druzhkov (DrSergei) <details> <summary>Changes</summary> These patches update `compileUnits` and `testGetTargetBreakpoints` requests, which are not a part of DAP. I combine two commits into one MR to save maintainers time and because it is mostly NFC. --- Full diff: https://github.com/llvm/llvm-project/pull/172283.diff 10 Files Affected: - (modified) lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp (+18-58) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+15-10) - (modified) lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp (+13-14) - (modified) lldb/tools/lldb-dap/JSONUtils.cpp (-9) - (modified) lldb/tools/lldb-dap/JSONUtils.h (-2) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+16) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+25) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp (+5) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.h (+6) - (modified) lldb/unittests/DAP/ProtocolRequestsTest.cpp (+61) ``````````diff diff --git a/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp index 0e5c2b23d8d67..e79000230de1f 100644 --- a/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp @@ -8,75 +8,35 @@ #include "DAP.h" #include "EventHelper.h" -#include "JSONUtils.h" +#include "Protocol/ProtocolRequests.h" #include "RequestHandler.h" -namespace lldb_dap { +using namespace lldb_dap; +using namespace lldb_dap::protocol; -// "compileUnitsRequest": { -// "allOf": [ { "$ref": "#/definitions/Request" }, { -// "type": "object", -// "description": "Compile Unit request; value of command field is -// 'compileUnits'.", -// "properties": { -// "command": { -// "type": "string", -// "enum": [ "compileUnits" ] -// }, -// "arguments": { -// "$ref": "#/definitions/compileUnitRequestArguments" -// } -// }, -// "required": [ "command", "arguments" ] -// }] -// }, -// "compileUnitsRequestArguments": { -// "type": "object", -// "description": "Arguments for 'compileUnits' request.", -// "properties": { -// "moduleId": { -// "type": "string", -// "description": "The ID of the module." -// } -// }, -// "required": [ "moduleId" ] -// }, -// "compileUnitsResponse": { -// "allOf": [ { "$ref": "#/definitions/Response" }, { -// "type": "object", -// "description": "Response to 'compileUnits' request.", -// "properties": { -// "body": { -// "description": "Response to 'compileUnits' request. Array of -// paths of compile units." -// } -// } -// }] -// } -void CompileUnitsRequestHandler::operator()( - const llvm::json::Object &request) const { - llvm::json::Object response; - FillResponse(request, response); - llvm::json::Object body; - llvm::json::Array units; - const auto *arguments = request.getObject("arguments"); - const llvm::StringRef module_id = - GetString(arguments, "moduleId").value_or(""); +static CompileUnit CreateCompileUnit(lldb::SBCompileUnit &unit) { + char unit_path_arr[PATH_MAX]; + unit.GetFileSpec().GetPath(unit_path_arr, sizeof(unit_path_arr)); + std::string unit_path(unit_path_arr); + return {std::move(unit_path)}; +} + +/// The `compileUnits` request returns an array of path of compile units for +/// given module specified by `moduleId`. +llvm::Expected<CompileUnitsResponseBody> CompileUnitsRequestHandler::Run( + const std::optional<CompileUnitsArguments> &args) const { + std::vector<CompileUnit> units; int num_modules = dap.target.GetNumModules(); for (int i = 0; i < num_modules; i++) { auto curr_module = dap.target.GetModuleAtIndex(i); - if (module_id == llvm::StringRef(curr_module.GetUUIDString())) { + if (args->moduleId == llvm::StringRef(curr_module.GetUUIDString())) { int num_units = curr_module.GetNumCompileUnits(); for (int j = 0; j < num_units; j++) { auto curr_unit = curr_module.GetCompileUnitAtIndex(j); units.emplace_back(CreateCompileUnit(curr_unit)); } - body.try_emplace("compileUnits", std::move(units)); break; } } - response.try_emplace("body", std::move(body)); - dap.SendJSON(llvm::json::Value(std::move(response))); -} - -} // namespace lldb_dap + return CompileUnitsResponseBody{std::move(units)}; +}; diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index 8f42a28160751..0669be50597e9 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -472,11 +472,16 @@ class SetInstructionBreakpointsRequestHandler Run(const protocol::SetInstructionBreakpointsArguments &args) const override; }; -class CompileUnitsRequestHandler : public LegacyRequestHandler { +class CompileUnitsRequestHandler + : public RequestHandler< + std::optional<protocol::CompileUnitsArguments>, + llvm::Expected<protocol::CompileUnitsResponseBody>> { public: - using LegacyRequestHandler::LegacyRequestHandler; + using RequestHandler::RequestHandler; static llvm::StringLiteral GetCommand() { return "compileUnits"; } - void operator()(const llvm::json::Object &request) const override; + llvm::Expected<protocol::CompileUnitsResponseBody> + Run(const std::optional<protocol::CompileUnitsArguments> &args) + const override; }; class ModulesRequestHandler final @@ -625,17 +630,17 @@ class ModuleSymbolsRequestHandler Run(const protocol::ModuleSymbolsArguments &args) const override; }; -/// A request used in testing to get the details on all breakpoints that are -/// currently set in the target. This helps us to test "setBreakpoints" and -/// "setFunctionBreakpoints" requests to verify we have the correct set of -/// breakpoints currently set in LLDB. -class TestGetTargetBreakpointsRequestHandler : public LegacyRequestHandler { +class TestGetTargetBreakpointsRequestHandler + : public RequestHandler< + protocol::TestGetTargetBreakpointsArguments, + llvm::Expected<protocol::TestGetTargetBreakpointsResponseBody>> { public: - using LegacyRequestHandler::LegacyRequestHandler; + using RequestHandler::RequestHandler; static llvm::StringLiteral GetCommand() { return "_testGetTargetBreakpoints"; } - void operator()(const llvm::json::Object &request) const override; + llvm::Expected<protocol::TestGetTargetBreakpointsResponseBody> + Run(const protocol::TestGetTargetBreakpointsArguments &args) const override; }; class WriteMemoryRequestHandler final diff --git a/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp index 5f4f016f6a1ef..d1b6a8e189032 100644 --- a/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp @@ -8,24 +8,23 @@ #include "DAP.h" #include "EventHelper.h" -#include "JSONUtils.h" +#include "Protocol/ProtocolRequests.h" #include "RequestHandler.h" -namespace lldb_dap { +using namespace lldb_dap; +using namespace lldb_dap::protocol; -void TestGetTargetBreakpointsRequestHandler::operator()( - const llvm::json::Object &request) const { - llvm::json::Object response; - FillResponse(request, response); - llvm::json::Array response_breakpoints; +/// A request used in testing to get the details on all breakpoints that are +/// currently set in the target. This helps us to test "setBreakpoints" and +/// "setFunctionBreakpoints" requests to verify we have the correct set of +/// breakpoints currently set in LLDB. +llvm::Expected<TestGetTargetBreakpointsResponseBody> +TestGetTargetBreakpointsRequestHandler::Run( + const TestGetTargetBreakpointsArguments &args) const { + std::vector<protocol::Breakpoint> breakpoints; for (uint32_t i = 0; dap.target.GetBreakpointAtIndex(i).IsValid(); ++i) { auto bp = Breakpoint(dap, dap.target.GetBreakpointAtIndex(i)); - response_breakpoints.push_back(bp.ToProtocolBreakpoint()); + breakpoints.push_back(bp.ToProtocolBreakpoint()); } - llvm::json::Object body; - body.try_emplace("breakpoints", std::move(response_breakpoints)); - response.try_emplace("body", std::move(body)); - dap.SendJSON(llvm::json::Value(std::move(response))); + return TestGetTargetBreakpointsResponseBody{std::move(breakpoints)}; } - -} // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index bb3f8aaaded89..1f9719110cedb 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -865,15 +865,6 @@ std::pair<int64_t, bool> UnpackLocation(int64_t location_id) { return std::pair{location_id >> 1, location_id & 1}; } -llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit) { - llvm::json::Object object; - char unit_path_arr[PATH_MAX]; - unit.GetFileSpec().GetPath(unit_path_arr, sizeof(unit_path_arr)); - std::string unit_path(unit_path_arr); - object.try_emplace("compileUnitPath", unit_path); - return llvm::json::Value(std::move(object)); -} - /// See /// https://microsoft.github.io/debug-adapter-protocol/specification#Reverse_Requests_RunInTerminal llvm::json::Object CreateRunInTerminalReverseRequest( diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h index 7fad7409b11fe..0a907599fc9ee 100644 --- a/lldb/tools/lldb-dap/JSONUtils.h +++ b/lldb/tools/lldb-dap/JSONUtils.h @@ -368,8 +368,6 @@ int64_t PackLocation(int64_t var_ref, bool is_value_location); /// Reverse of `PackLocation` std::pair<int64_t, bool> UnpackLocation(int64_t location_id); -llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit); - /// Create a runInTerminal reverse request object /// /// \param[in] program diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp index 42acc70333a7b..bf470b78077df 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -721,4 +721,20 @@ llvm::json::Value toJSON(const LocationsResponseBody &Body) { return result; } +bool fromJSON(const llvm::json::Value &Params, CompileUnitsArguments &Args, + llvm::json::Path Path) { + json::ObjectMapper O(Params, Path); + return O && O.map("moduleId", Args.moduleId); +} + +llvm::json::Value toJSON(const CompileUnitsResponseBody &Body) { + json::Object result{{"compileUnits", Body.compileUnits}}; + return result; +} + +llvm::json::Value toJSON(const TestGetTargetBreakpointsResponseBody &Body) { + json::Object result{{"breakpoints", Body.breakpoints}}; + 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 104520f2c24c2..cc123943ec0b6 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -1230,6 +1230,31 @@ struct LocationsResponseBody { }; llvm::json::Value toJSON(const LocationsResponseBody &); +/// Arguments for `compileUnits` request. +struct CompileUnitsArguments { + /// The ID of the module. + std::string moduleId; +}; +bool fromJSON(const llvm::json::Value &, CompileUnitsArguments &, + llvm::json::Path); + +/// Response to `compileUnits` request. +struct CompileUnitsResponseBody { + /// Array of compile units. + std::vector<CompileUnit> compileUnits; +}; +llvm::json::Value toJSON(const CompileUnitsResponseBody &); + +/// Arguments for `testGetTargetBreakpoints` request. +using TestGetTargetBreakpointsArguments = EmptyArguments; + +/// Response to `testGetTargetBreakpoints` request. +struct TestGetTargetBreakpointsResponseBody { + /// Array of all breakpoints that are currently set in the target. + std::vector<Breakpoint> breakpoints; +}; +llvm::json::Value toJSON(const TestGetTargetBreakpointsResponseBody &); + } // namespace lldb_dap::protocol #endif diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp index 95007013742a0..c7f7c447b5b6f 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp @@ -1169,4 +1169,9 @@ json::Value toJSON(const ExceptionDetails &ED) { return result; } +llvm::json::Value toJSON(const CompileUnit &CU) { + json::Object result{{"compileUnitPath", CU.compileUnitPath}}; + 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 ee103ddf0b7a2..4ead4786bc661 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h @@ -1038,6 +1038,12 @@ struct ExceptionDetails { }; llvm::json::Value toJSON(const ExceptionDetails &); +struct CompileUnit { + /// Path of compile unit. + std::string compileUnitPath; +}; +llvm::json::Value toJSON(const CompileUnit &); + } // namespace lldb_dap::protocol #endif diff --git a/lldb/unittests/DAP/ProtocolRequestsTest.cpp b/lldb/unittests/DAP/ProtocolRequestsTest.cpp index 50b8335ba46cc..710b78960ef09 100644 --- a/lldb/unittests/DAP/ProtocolRequestsTest.cpp +++ b/lldb/unittests/DAP/ProtocolRequestsTest.cpp @@ -243,3 +243,64 @@ TEST(ProtocolRequestsTest, LocationsResponseBody) { ASSERT_THAT_EXPECTED(expected, llvm::Succeeded()); EXPECT_EQ(PrettyPrint(*expected), PrettyPrint(body)); } + +TEST(ProtocolRequestsTest, CompileUnitsArguments) { + llvm::Expected<CompileUnitsArguments> expected = + parse<CompileUnitsArguments>(R"({"moduleId": "42"})"); + ASSERT_THAT_EXPECTED(expected, llvm::Succeeded()); + EXPECT_EQ(expected->moduleId, "42"); + + // Check required keys. + EXPECT_THAT_EXPECTED(parse<CompileUnitsArguments>(R"({})"), + FailedWithMessage("missing value at (root).moduleId")); +} + +TEST(ProtocolRequestsTest, CompileUnitsResponseBody) { + CompileUnitsResponseBody body; + body.compileUnits = {{"main.cpp"}, {"util.cpp"}}; + + // Check required keys. + Expected<json::Value> expected = parse(R"({ + "compileUnits": [ + { + "compileUnitPath": "main.cpp" + }, + { + "compileUnitPath": "util.cpp" + } + ] + })"); + + ASSERT_THAT_EXPECTED(expected, llvm::Succeeded()); + EXPECT_EQ(PrettyPrint(*expected), PrettyPrint(body)); +} + +TEST(ProtocolRequestsTest, TestGetTargetBreakpointsResponseBody) { + Breakpoint breakpoint1; + breakpoint1.id = 1; + breakpoint1.verified = true; + Breakpoint breakpoint2; + breakpoint2.id = 2; + breakpoint2.verified = false; + breakpoint2.message = "Failed to set breakpoint"; + TestGetTargetBreakpointsResponseBody body; + body.breakpoints = {breakpoint1, breakpoint2}; + + // Check required keys. + Expected<json::Value> expected = parse(R"({ + "breakpoints": [ + { + "id": 1, + "verified": true + }, + { + "id": 2, + "verified": false, + "message": "Failed to set breakpoint" + } + ] + })"); + + ASSERT_THAT_EXPECTED(expected, llvm::Succeeded()); + EXPECT_EQ(PrettyPrint(*expected), PrettyPrint(body)); +} `````````` </details> https://github.com/llvm/llvm-project/pull/172283 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
