[lldb-dap] Migrate 'stepIn' request to well structured types.

[ashgti]

Migrates the 'stepIn' request handler to have well structured types instead of raw json values.

I also noticed in the 'next' request handler we were not passing the 'RunMode' flag. Updated the 'next' request handler as well.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: John Harrison (ashgti)

Changes

Migrates the 'stepIn' request handler to have well structured types instead of raw json values.

I also noticed in the 'next' request handler we were not passing the 'RunMode' flag. Updated the 'next' request handler as well.

---

Full diff: https://github.com/llvm/llvm-project/pull/137071.diff

5 Files Affected:

• (modified) lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp (+2-1)
• (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+4-3)
• (modified) lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp (+32-75)
• (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+9)
• (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+22)

diff --git a/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
index 1603563841005..3fa167686d2f9 100644
--- a/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
@@ -13,6 +13,7 @@
 #include "llvm/Support/Error.h"
 
 using namespace llvm;
+using namespace lldb;
 using namespace lldb_dap::protocol;
 
 namespace lldb_dap {
@@ -35,7 +36,7 @@ Error NextRequestHandler::Run(const NextArguments &args) const {
   if (args.granularity == eSteppingGranularityInstruction) {
     thread.StepInstruction(/*step_over=*/true);
   } else {
-    thread.StepOver();
+    thread.StepOver(args.singleThread ? eOnlyThisThread : eOnlyDuringStepping);
   }
 
   return Error::success();
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index edb9de7d0dc20..e13f7a3749e00 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -298,11 +298,12 @@ class NextRequestHandler
   llvm::Error Run(const protocol::NextArguments &args) const override;
 };
 
-class StepInRequestHandler : public LegacyRequestHandler {
+class StepInRequestHandler : public RequestHandler<protocol::StepInArguments,
+                                                   protocol::StepInResponse> {
 public:
-  using LegacyRequestHandler::LegacyRequestHandler;
+  using RequestHandler::RequestHandler;
   static llvm::StringLiteral GetCommand() { return "stepIn"; }
-  void operator()(const llvm::json::Object &request) const override;
+  llvm::Error Run(const protocol::StepInArguments &args) const override;
 };
 
 class StepInTargetsRequestHandler : public LegacyRequestHandler {
diff --git a/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
index 9d8d75b359447..00b68abb48677 100644
--- a/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
@@ -8,91 +8,48 @@
 
 #include "DAP.h"
 #include "EventHelper.h"
-#include "JSONUtils.h"
+#include "Protocol/ProtocolRequests.h"
+#include "Protocol/ProtocolTypes.h"
 #include "RequestHandler.h"
 
-namespace lldb_dap {
+using namespace llvm;
+using namespace lldb;
+using namespace lldb_dap::protocol;
 
-// "StepInRequest": {
-//   "allOf": [ { "$ref": "#/definitions/Request" }, {
-//     "type": "object",
-//     "description": "StepIn request; value of command field is 'stepIn'. The
-//     request starts the debuggee to step into a function/method if possible.
-//     If it cannot step into a target, 'stepIn' behaves like 'next'. The debug
-//     adapter first sends the StepInResponse and then a StoppedEvent (event
-//     type 'step') after the step has completed. If there are multiple
-//     function/method calls (or other targets) on the source line, the optional
-//     argument 'targetId' can be used to control into which target the 'stepIn'
-//     should occur. The list of possible targets for a given source line can be
-//     retrieved via the 'stepInTargets' request.", "properties": {
-//       "command": {
-//         "type": "string",
-//         "enum": [ "stepIn" ]
-//       },
-//       "arguments": {
-//         "$ref": "#/definitions/StepInArguments"
-//       }
-//     },
-//     "required": [ "command", "arguments"  ]
-//   }]
-// },
-// "StepInArguments": {
-//   "type": "object",
-//   "description": "Arguments for 'stepIn' request.",
-//   "properties": {
-//     "threadId": {
-//       "type": "integer",
-//       "description": "Execute 'stepIn' for this thread."
-//     },
-//     "targetId": {
-//       "type": "integer",
-//       "description": "Optional id of the target to step into."
-//     },
-//     "granularity": {
-//       "$ref": "#/definitions/SteppingGranularity",
-//       "description": "Stepping granularity. If no granularity is specified, a
-//                       granularity of `statement` is assumed."
-//     }
-//   },
-//   "required": [ "threadId" ]
-// },
-// "StepInResponse": {
-//   "allOf": [ { "$ref": "#/definitions/Response" }, {
-//     "type": "object",
-//     "description": "Response to 'stepIn' request. This is just an
-//     acknowledgement, so no body field is required."
-//   }]
-// }
-void StepInRequestHandler::operator()(const llvm::json::Object &request) const {
-  llvm::json::Object response;
-  FillResponse(request, response);
-  const auto *arguments = request.getObject("arguments");
+namespace lldb_dap {
 
+// The request resumes the given thread to step into a function/method and
+// allows all other threads to run freely by resuming them. If the debug adapter
+// supports single thread execution (see capability
+// `supportsSingleThreadExecutionRequests`), setting the `singleThread` argument
+// to true prevents other suspended threads from resuming. If the request cannot
+// step into a target, `stepIn` behaves like the `next` request. The debug
+// adapter first sends the response and then a `stopped` event (with reason
+// `step`) after the step has completed. If there are multiple function/method
+// calls (or other targets) on the source line, the argument `targetId` can be
+// used to control into which target the `stepIn` should occur. The list of
+// possible targets for a given source line can be retrieved via the
+// `stepInTargets` request.
+Error StepInRequestHandler::Run(const StepInArguments &args) const {
   std::string step_in_target;
-  const auto target_id =
-      GetInteger<uint64_t>(arguments, "targetId").value_or(0);
-  auto it = dap.step_in_targets.find(target_id);
+  auto it = dap.step_in_targets.find(args.targetId.value_or(0));
   if (it != dap.step_in_targets.end())
     step_in_target = it->second;
 
-  const bool single_thread =
-      GetBoolean(arguments, "singleThread").value_or(false);
-  lldb::RunMode run_mode =
-      single_thread ? lldb::eOnlyThisThread : lldb::eOnlyDuringStepping;
-  lldb::SBThread thread = dap.GetLLDBThread(*arguments);
-  if (thread.IsValid()) {
-    // Remember the thread ID that caused the resume so we can set the
-    // "threadCausedFocus" boolean value in the "stopped" events.
-    dap.focus_tid = thread.GetThreadID();
-    if (HasInstructionGranularity(*arguments)) {
-      thread.StepInstruction(/*step_over=*/false);
-    } else {
-      thread.StepInto(step_in_target.c_str(), run_mode);
-    }
+  RunMode run_mode = args.singleThread ? eOnlyThisThread : eOnlyDuringStepping;
+  SBThread thread = dap.GetLLDBThread(args.threadId);
+  if (!thread.IsValid())
+    return make_error<DAPError>("invalid thread");
+
+  // Remember the thread ID that caused the resume so we can set the
+  // "threadCausedFocus" boolean value in the "stopped" events.
+  dap.focus_tid = thread.GetThreadID();
+  if (args.granularity == eSteppingGranularityInstruction) {
+    thread.StepInstruction(/*step_over=*/false);
   } else {
-    response["success"] = llvm::json::Value(false);
+    thread.StepInto(step_in_target.c_str(), run_mode);
   }
-  dap.SendJSON(llvm::json::Value(std::move(response)));
+  return Error::success();
 }
 
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index b113299affb0f..ee7c653ee9f1b 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -121,4 +121,13 @@ bool fromJSON(const llvm::json::Value &Params, NextArguments &NA,
          OM.mapOptional("granularity", NA.granularity);
 }
 
+bool fromJSON(const llvm::json::Value &Params, StepInArguments &SIA,
+              llvm::json::Path P) {
+  json::ObjectMapper OM(Params, P);
+  return OM && OM.map("threadId", SIA.threadId) &&
+         OM.map("targetId", SIA.targetId) &&
+         OM.mapOptional("singleThread", SIA.singleThread) &&
+         OM.mapOptional("granularity", SIA.granularity);
+}
+
 } // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index 6e3e2c6a9e2c8..50c16c15cef32 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -256,6 +256,28 @@ bool fromJSON(const llvm::json::Value &, NextArguments &, llvm::json::Path);
 /// body field is required.
 using NextResponse = VoidResponse;
 
+/// Arguments for `stepIn` request.
+struct StepInArguments {
+  /// Specifies the thread for which to resume execution for one step-into (of
+  /// the given granularity).
+  uint64_t threadId = LLDB_INVALID_THREAD_ID;
+
+  /// If this flag is true, all other suspended threads are not resumed.
+  bool singleThread = false;
+
+  /// Id of the target to step into.
+  std::optional<uint64_t> targetId;
+
+  /// Stepping granularity. If no granularity is specified, a granularity of
+  /// `statement` is assumed.
+  SteppingGranularity granularity = eSteppingGranularityStatement;
+};
+bool fromJSON(const llvm::json::Value &, StepInArguments &, llvm::json::Path);
+
+/// Response to `stepIn` request. This is just an acknowledgement, so no
+/// body field is required.
+using StepInResponse = VoidResponse;
+
 } // namespace lldb_dap::protocol
 
 #endif