Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[lldb][lldb-dap] Migrate ScopesRequest to structured types #138116

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.

Sign up for GitHub

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 7 commits into from
May 15, 2025
Merged

[lldb][lldb-dap] Migrate ScopesRequest to structured types #138116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cef462d
[lldb][lldb-dap] Migrate 'Scopes' to structured types.
da-viper Apr 29, 2025
2f2a46d
[lldb][lldb-dap] review changes
da-viper May 12, 2025
1815d6b
[lldb][lldb-dap] review changes
da-viper May 12, 2025
7f7b3f6
[lldb][lldb-dap] add review changes
da-viper May 13, 2025
78e50c5
[lldb][lldb-dap] clarify the todo.
da-viper May 13, 2025
0b5db4e
[lldb][lldb-dap] update merge changes
da-viper May 15, 2025
6519da9
[lldb][lldb-dap] update merge changes
da-viper May 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions 11 lldb/tools/lldb-dap/DAP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,17 +559,6 @@ lldb::SBFrame DAP::GetLLDBFrame(const llvm::json::Object &arguments) {
return GetLLDBFrame(frame_id);
}

llvm::json::Value DAP::CreateTopLevelScopes() {
llvm::json::Array scopes;
scopes.emplace_back(
CreateScope("Locals", VARREF_LOCALS, variables.locals.GetSize(), false));
scopes.emplace_back(CreateScope("Globals", VARREF_GLOBALS,
variables.globals.GetSize(), false));
scopes.emplace_back(CreateScope("Registers", VARREF_REGS,
variables.registers.GetSize(), false));
return llvm::json::Value(std::move(scopes));
}

ReplMode DAP::DetectReplMode(lldb::SBFrame frame, std::string &expression,
bool partial_expression) {
// Check for the escape hatch prefix.
Expand Down
4 changes: 2 additions & 2 deletions 4 lldb/tools/lldb-dap/DAP.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,10 +283,10 @@ struct DAP {
lldb::SBThread GetLLDBThread(const llvm::json::Object &arguments);

lldb::SBFrame GetLLDBFrame(uint64_t frame_id);
/// TODO: remove this function when we finish migrating to the
/// new protocol types.
lldb::SBFrame GetLLDBFrame(const llvm::json::Object &arguments);
da-viper marked this conversation as resolved.
Show resolved Hide resolved

llvm::json::Value CreateTopLevelScopes();

void PopulateExceptionBreakpoints();

/// Attempt to determine if an expression is a variable expression or
Expand Down
10 changes: 7 additions & 3 deletions 10 lldb/tools/lldb-dap/Handler/RequestHandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,11 +452,15 @@ class PauseRequestHandler : public LegacyRequestHandler {
void operator()(const llvm::json::Object &request) const override;
};

class ScopesRequestHandler : public LegacyRequestHandler {
class ScopesRequestHandler final
: public RequestHandler<protocol::ScopesArguments,
llvm::Expected<protocol::ScopesResponseBody>> {
public:
using LegacyRequestHandler::LegacyRequestHandler;
using RequestHandler::RequestHandler;
static llvm::StringLiteral GetCommand() { return "scopes"; }
void operator()(const llvm::json::Object &request) const override;

llvm::Expected<protocol::ScopesResponseBody>
Run(const protocol::ScopesArguments &args) const override;
};

class SetVariableRequestHandler final
Expand Down
122 changes: 57 additions & 65 deletions 122 lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,69 +7,56 @@
//===----------------------------------------------------------------------===//

#include "DAP.h"
#include "EventHelper.h"
#include "JSONUtils.h"
#include "RequestHandler.h"

using namespace lldb_dap::protocol;
namespace lldb_dap {

// "ScopesRequest": {
// "allOf": [ { "$ref": "#/definitions/Request" }, {
// "type": "object",
// "description": "Scopes request; value of command field is 'scopes'. The
// request returns the variable scopes for a given stackframe ID.",
// "properties": {
// "command": {
// "type": "string",
// "enum": [ "scopes" ]
// },
// "arguments": {
// "$ref": "#/definitions/ScopesArguments"
// }
// },
// "required": [ "command", "arguments" ]
// }]
// },
// "ScopesArguments": {
// "type": "object",
// "description": "Arguments for 'scopes' request.",
// "properties": {
// "frameId": {
// "type": "integer",
// "description": "Retrieve the scopes for this stackframe."
// }
// },
// "required": [ "frameId" ]
// },
// "ScopesResponse": {
// "allOf": [ { "$ref": "#/definitions/Response" }, {
// "type": "object",
// "description": "Response to 'scopes' request.",
// "properties": {
// "body": {
// "type": "object",
// "properties": {
// "scopes": {
// "type": "array",
// "items": {
// "$ref": "#/definitions/Scope"
// },
// "description": "The scopes of the stackframe. If the array has
// length zero, there are no scopes available."
// }
// },
// "required": [ "scopes" ]
// }
// },
// "required": [ "body" ]
// }]
// }
void ScopesRequestHandler::operator()(const llvm::json::Object &request) const {
llvm::json::Object response;
FillResponse(request, response);
llvm::json::Object body;
const auto *arguments = request.getObject("arguments");
lldb::SBFrame frame = dap.GetLLDBFrame(*arguments);
/// Creates a `protocol::Scope` struct.
///
///
/// \param[in] name
/// The value to place into the "name" key
///
/// \param[in] variablesReference
/// The value to place into the "variablesReference" key
///
/// \param[in] namedVariables
/// The value to place into the "namedVariables" key
///
/// \param[in] expensive
/// The value to place into the "expensive" key
///
/// \return
/// A `protocol::Scope`
static Scope CreateScope(const llvm::StringRef name, int64_t variablesReference,
int64_t namedVariables, bool expensive) {
Scope scope;
scope.name = name;

// TODO: Support "arguments" and "return value" scope.
// At the moment lldb-dap includes the arguments and return_value into the
// "locals" scope.
// vscode only expands the first non-expensive scope, this causes friction
// if we add the arguments above the local scope as the locals scope will not
// be expanded if we enter a function with arguments. It becomes more
// annoying when the scope has arguments, return_value and locals.
if (variablesReference == VARREF_LOCALS)
scope.presentationHint = Scope::eScopePresentationHintLocals;
else if (variablesReference == VARREF_REGS)
scope.presentationHint = Scope::eScopePresentationHintRegisters;

scope.variablesReference = variablesReference;
scope.namedVariables = namedVariables;
scope.expensive = expensive;

return scope;
}

llvm::Expected<ScopesResponseBody>
ScopesRequestHandler::Run(const ScopesArguments &args) const {
lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId);

// As the user selects different stack frames in the GUI, a "scopes" request
// will be sent to the DAP. This is the only way we know that the user has
// selected a frame in a thread. There are no other notifications that are
Expand All @@ -78,17 +65,16 @@ void ScopesRequestHandler::operator()(const llvm::json::Object &request) const {
// are sent, this allows users to type commands in the debugger console
// with a backtick character to run lldb commands and these lldb commands
// will now have the right context selected as they are run. If the user
// types "`bt" into the debugger console and we had another thread selected
// types "`bt" into the debugger console, and we had another thread selected
// in the LLDB library, we would show the wrong thing to the user. If the
// users switches threads with a lldb command like "`thread select 14", the
// users switch threads with a lldb command like "`thread select 14", the
// GUI will not update as there are no "event" notification packets that
// allow us to change the currently selected thread or frame in the GUI that
// I am aware of.
if (frame.IsValid()) {
frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
frame.GetThread().SetSelectedFrame(frame.GetFrameID());
}

dap.variables.locals = frame.GetVariables(/*arguments=*/true,
/*locals=*/true,
/*statics=*/false,
Expand All @@ -98,9 +84,15 @@ void ScopesRequestHandler::operator()(const llvm::json::Object &request) const {
/*statics=*/true,
/*in_scope_only=*/true);
dap.variables.registers = frame.GetRegisters();
body.try_emplace("scopes", dap.CreateTopLevelScopes());
response.try_emplace("body", std::move(body));
dap.SendJSON(llvm::json::Value(std::move(response)));

std::vector scopes = {CreateScope("Locals", VARREF_LOCALS,
dap.variables.locals.GetSize(), false),
CreateScope("Globals", VARREF_GLOBALS,
dap.variables.globals.GetSize(), false),
CreateScope("Registers", VARREF_REGS,
dap.variables.registers.GetSize(), false)};

return ScopesResponseBody{std::move(scopes)};
Copy link
Contributor

Choose a reason for hiding this comment

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

I almost wonder if it would make more sense for some of this to move into the Variables helper?

Maybe llvm::Expected<std::vector<protocol::Scope>> Variables::SetFrame(SBFrame frame); or a SetFrame and GetScopes helper separately.

I think the intention is for the Variables helper to be able to handle caching and managing variable lookups. We can make it a bit more aware of its own state by moving this over there.

How does that sound to you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it should be only in the scoped request since nothing outside is using the functionality.

would be worth it to move it to the variables if there was another request that depends on getting all the scopes.

}

} // namespace lldb_dap
21 changes: 0 additions & 21 deletions 21 lldb/tools/lldb-dap/JSONUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,27 +238,6 @@ llvm::json::Object CreateEventObject(const llvm::StringRef event_name);
protocol::ExceptionBreakpointsFilter
CreateExceptionBreakpointFilter(const ExceptionBreakpoint &bp);

/// Create a "Scope" JSON object as described in the debug adapter definition.
///
/// \param[in] name
/// The value to place into the "name" key
//
/// \param[in] variablesReference
/// The value to place into the "variablesReference" key
//
/// \param[in] namedVariables
/// The value to place into the "namedVariables" key
//
/// \param[in] expensive
/// The value to place into the "expensive" key
///
/// \return
/// A "Scope" JSON object with that follows the formal JSON
/// definition outlined by Microsoft.
llvm::json::Value CreateScope(const llvm::StringRef name,
int64_t variablesReference,
int64_t namedVariables, bool expensive);

/// Create a "Source" JSON object as described in the debug adapter definition.
///
/// \param[in] file
Expand Down
14 changes: 14 additions & 0 deletions 14 lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,20 @@ llvm::json::Value toJSON(const SetVariableResponseBody &SVR) {

return llvm::json::Value(std::move(Body));
}
bool fromJSON(const llvm::json::Value &Params, ScopesArguments &SCA,
llvm::json::Path P) {
json::ObjectMapper O(Params, P);
return O && O.map("frameId", SCA.frameId);
}

llvm::json::Value toJSON(const ScopesResponseBody &SCR) {
llvm::json::Array scopes;
for (const Scope &scope : SCR.scopes) {
scopes.emplace_back(toJSON(scope));
}

return llvm::json::Object{{"scopes", std::move(scopes)}};
}

bool fromJSON(const json::Value &Params, SourceArguments &SA, json::Path P) {
json::ObjectMapper O(Params, P);
Expand Down
13 changes: 13 additions & 0 deletions 13 lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,19 @@ struct SetVariableResponseBody {
};
llvm::json::Value toJSON(const SetVariableResponseBody &);

struct ScopesArguments {
/// Retrieve the scopes for the stack frame identified by `frameId`. The
/// `frameId` must have been obtained in the current suspended state. See
/// 'Lifetime of Object References' in the Overview section for details.
uint64_t frameId = LLDB_INVALID_FRAME_ID;
};
bool fromJSON(const llvm::json::Value &, ScopesArguments &, llvm::json::Path);

struct ScopesResponseBody {
std::vector<Scope> scopes;
};
llvm::json::Value toJSON(const ScopesResponseBody &);

/// Arguments for `source` request.
struct SourceArguments {
/// Specifies the source content to load. Either `source.path` or
Expand Down
Loading
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.