Skip to content

[lldb-dap] Migrating the 'stepOut' request to use typed RequestHandler. #137362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 25, 2025

Conversation

ashgti
Copy link
Contributor

@ashgti ashgti commented Apr 25, 2025

This moves the 'stepOut' request to the typed RequestHandler.

This moves the 'stepOut' request to the typed RequestHandler.
@llvmbot
Copy link
Member

llvmbot commented Apr 25, 2025

@llvm/pr-subscribers-lldb

Author: John Harrison (ashgti)

Changes

This moves the 'stepOut' request to the typed RequestHandler.


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

4 Files Affected:

  • (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+4-3)
  • (modified) lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp (+26-52)
  • (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+8)
  • (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+19)
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index 6a641c14397d6..fa3d76ed4a125 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -331,11 +331,12 @@ class StepInTargetsRequestHandler : public LegacyRequestHandler {
   void operator()(const llvm::json::Object &request) const override;
 };
 
-class StepOutRequestHandler : public LegacyRequestHandler {
+class StepOutRequestHandler : public RequestHandler<protocol::StepOutArguments,
+                                                    protocol::StepOutResponse> {
 public:
-  using LegacyRequestHandler::LegacyRequestHandler;
+  using RequestHandler::RequestHandler;
   static llvm::StringLiteral GetCommand() { return "stepOut"; }
-  void operator()(const llvm::json::Object &request) const override;
+  llvm::Error Run(const protocol::StepOutArguments &args) const override;
 };
 
 class SetBreakpointsRequestHandler : public LegacyRequestHandler {
diff --git a/lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp
index 39d68d21a23d4..6b98582262a68 100644
--- a/lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp
@@ -8,62 +8,36 @@
 
 #include "DAP.h"
 #include "EventHelper.h"
-#include "JSONUtils.h"
+#include "Protocol/ProtocolRequests.h"
 #include "RequestHandler.h"
+#include "llvm/Support/Error.h"
+
+using namespace llvm;
+using namespace lldb_dap::protocol;
 
 namespace lldb_dap {
 
-// "StepOutRequest": {
-//   "allOf": [ { "$ref": "#/definitions/Request" }, {
-//     "type": "object",
-//     "description": "StepOut request; value of command field is 'stepOut'. The
-//     request starts the debuggee to run again for one step. The debug adapter
-//     first sends the StepOutResponse and then a StoppedEvent (event type
-//     'step') after the step has completed.", "properties": {
-//       "command": {
-//         "type": "string",
-//         "enum": [ "stepOut" ]
-//       },
-//       "arguments": {
-//         "$ref": "#/definitions/StepOutArguments"
-//       }
-//     },
-//     "required": [ "command", "arguments"  ]
-//   }]
-// },
-// "StepOutArguments": {
-//   "type": "object",
-//   "description": "Arguments for 'stepOut' request.",
-//   "properties": {
-//     "threadId": {
-//       "type": "integer",
-//       "description": "Execute 'stepOut' for this thread."
-//     }
-//   },
-//   "required": [ "threadId" ]
-// },
-// "StepOutResponse": {
-//   "allOf": [ { "$ref": "#/definitions/Response" }, {
-//     "type": "object",
-//     "description": "Response to 'stepOut' request. This is just an
-//     acknowledgement, so no body field is required."
-//   }]
-// }
-void StepOutRequestHandler::operator()(
-    const llvm::json::Object &request) const {
-  llvm::json::Object response;
-  FillResponse(request, response);
-  const auto *arguments = request.getObject("arguments");
-  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();
-    thread.StepOut();
-  } else {
-    response["success"] = llvm::json::Value(false);
-  }
-  dap.SendJSON(llvm::json::Value(std::move(response)));
+/// The request resumes the given thread to step out (return) from 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.
+///
+/// The debug adapter first sends the response and then a `stopped` event (with
+/// reason `step`) after the step has completed."
+Error StepOutRequestHandler::Run(const StepOutArguments &arguments) const {
+  lldb::SBThread thread = dap.GetLLDBThread(arguments.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();
+  thread.StepOut();
+
+  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 a49efde390871..61fea66490c30 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -293,4 +293,12 @@ bool fromJSON(const llvm::json::Value &Params, StepInArguments &SIA,
          OM.mapOptional("granularity", SIA.granularity);
 }
 
+bool fromJSON(const llvm::json::Value &Params, StepOutArguments &SOA,
+              llvm::json::Path P) {
+  json::ObjectMapper OM(Params, P);
+  return OM && OM.map("threadId", SOA.threadId) &&
+         OM.mapOptional("singleThread", SOA.singleThread) &&
+         OM.mapOptional("granularity", SOA.granularity);
+}
+
 } // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index fcb5dbddb2e28..33f93cc38799a 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -358,6 +358,25 @@ bool fromJSON(const llvm::json::Value &, StepInArguments &, llvm::json::Path);
 /// body field is required.
 using StepInResponse = VoidResponse;
 
+/// Arguments for `stepOut` request.
+struct StepOutArguments {
+  /// Specifies the thread for which to resume execution for one step-out (of
+  /// the given granularity).
+  uint64_t threadId = LLDB_INVALID_THREAD_ID;
+
+  /// If this flag is true, all other suspended threads are not resumed.
+  std::optional<bool> singleThread;
+
+  /// Stepping granularity. If no granularity is specified, a granularity of
+  /// `statement` is assumed.
+  SteppingGranularity granularity = eSteppingGranularityStatement;
+};
+bool fromJSON(const llvm::json::Value &, StepOutArguments &, llvm::json::Path);
+
+/// Response to `stepOut` request. This is just an acknowledgement, so no
+/// body field is required.
+using StepOutResponse = VoidResponse;
+
 } // namespace lldb_dap::protocol
 
 #endif

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty straightforward. LGTM.

@ashgti ashgti merged commit 6957699 into llvm:main Apr 25, 2025
11 of 13 checks passed
jyli0116 pushed a commit to jyli0116/llvm-project that referenced this pull request Apr 28, 2025
…r. (llvm#137362)

This moves the 'stepOut' request to the typed RequestHandler.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…r. (llvm#137362)

This moves the 'stepOut' request to the typed RequestHandler.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…r. (llvm#137362)

This moves the 'stepOut' request to the typed RequestHandler.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…r. (llvm#137362)

This moves the 'stepOut' request to the typed RequestHandler.
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request May 9, 2025
…r. (llvm#137362)

This moves the 'stepOut' request to the typed RequestHandler.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants