[lldb-dap] Migrate additional requests to stuctured types

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)));
+  return CompileUnitsResponseBody{std::move(units)};
 }
-
-} // namespace lldb_dap

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

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

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(

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

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

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

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

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

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));
+}