Skip to content

Commit 4ba8f4e

Browse files
da-viperda-viper
authored
[lldb][lldb-dap] Migrate ScopesRequest to structured types (#138116)
Migrate ScopesRequest To use the Protocol Types
1 parent 156985e commit 4ba8f4e

File tree

10 files changed

+319
-129
lines changed

10 files changed

+319
-129
lines changed

‎lldb/tools/lldb-dap/DAP.cpp

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -559,17 +559,6 @@ lldb::SBFrame DAP::GetLLDBFrame(const llvm::json::Object &arguments) {
559559
return GetLLDBFrame(frame_id);
560560
}
561561

562-
llvm::json::Value DAP::CreateTopLevelScopes() {
563-
llvm::json::Array scopes;
564-
scopes.emplace_back(
565-
CreateScope("Locals", VARREF_LOCALS, variables.locals.GetSize(), false));
566-
scopes.emplace_back(CreateScope("Globals", VARREF_GLOBALS,
567-
variables.globals.GetSize(), false));
568-
scopes.emplace_back(CreateScope("Registers", VARREF_REGS,
569-
variables.registers.GetSize(), false));
570-
return llvm::json::Value(std::move(scopes));
571-
}
572-
573562
ReplMode DAP::DetectReplMode(lldb::SBFrame frame, std::string &expression,
574563
bool partial_expression) {
575564
// Check for the escape hatch prefix.

‎lldb/tools/lldb-dap/DAP.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,10 +283,10 @@ struct DAP {
283283
lldb::SBThread GetLLDBThread(const llvm::json::Object &arguments);
284284

285285
lldb::SBFrame GetLLDBFrame(uint64_t frame_id);
286+
/// TODO: remove this function when we finish migrating to the
287+
/// new protocol types.
286288
lldb::SBFrame GetLLDBFrame(const llvm::json::Object &arguments);
287289

288-
llvm::json::Value CreateTopLevelScopes();
289-
290290
void PopulateExceptionBreakpoints();
291291

292292
/// Attempt to determine if an expression is a variable expression or

‎lldb/tools/lldb-dap/Handler/RequestHandler.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -452,11 +452,15 @@ class PauseRequestHandler : public LegacyRequestHandler {
452452
void operator()(const llvm::json::Object &request) const override;
453453
};
454454

455-
class ScopesRequestHandler : public LegacyRequestHandler {
455+
class ScopesRequestHandler final
456+
: public RequestHandler<protocol::ScopesArguments,
457+
llvm::Expected<protocol::ScopesResponseBody>> {
456458
public:
457-
using LegacyRequestHandler::LegacyRequestHandler;
459+
using RequestHandler::RequestHandler;
458460
static llvm::StringLiteral GetCommand() { return "scopes"; }
459-
void operator()(const llvm::json::Object &request) const override;
461+
462+
llvm::Expected<protocol::ScopesResponseBody>
463+
Run(const protocol::ScopesArguments &args) const override;
460464
};
461465

462466
class SetVariableRequestHandler final

‎lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp

Lines changed: 57 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -7,69 +7,56 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "DAP.h"
10-
#include "EventHelper.h"
11-
#include "JSONUtils.h"
1210
#include "RequestHandler.h"
1311

12+
using namespace lldb_dap::protocol;
1413
namespace lldb_dap {
1514

16-
// "ScopesRequest": {
17-
// "allOf": [ { "$ref": "#/definitions/Request" }, {
18-
// "type": "object",
19-
// "description": "Scopes request; value of command field is 'scopes'. The
20-
// request returns the variable scopes for a given stackframe ID.",
21-
// "properties": {
22-
// "command": {
23-
// "type": "string",
24-
// "enum": [ "scopes" ]
25-
// },
26-
// "arguments": {
27-
// "$ref": "#/definitions/ScopesArguments"
28-
// }
29-
// },
30-
// "required": [ "command", "arguments" ]
31-
// }]
32-
// },
33-
// "ScopesArguments": {
34-
// "type": "object",
35-
// "description": "Arguments for 'scopes' request.",
36-
// "properties": {
37-
// "frameId": {
38-
// "type": "integer",
39-
// "description": "Retrieve the scopes for this stackframe."
40-
// }
41-
// },
42-
// "required": [ "frameId" ]
43-
// },
44-
// "ScopesResponse": {
45-
// "allOf": [ { "$ref": "#/definitions/Response" }, {
46-
// "type": "object",
47-
// "description": "Response to 'scopes' request.",
48-
// "properties": {
49-
// "body": {
50-
// "type": "object",
51-
// "properties": {
52-
// "scopes": {
53-
// "type": "array",
54-
// "items": {
55-
// "$ref": "#/definitions/Scope"
56-
// },
57-
// "description": "The scopes of the stackframe. If the array has
58-
// length zero, there are no scopes available."
59-
// }
60-
// },
61-
// "required": [ "scopes" ]
62-
// }
63-
// },
64-
// "required": [ "body" ]
65-
// }]
66-
// }
67-
void ScopesRequestHandler::operator()(const llvm::json::Object &request) const {
68-
llvm::json::Object response;
69-
FillResponse(request, response);
70-
llvm::json::Object body;
71-
const auto *arguments = request.getObject("arguments");
72-
lldb::SBFrame frame = dap.GetLLDBFrame(*arguments);
15+
/// Creates a `protocol::Scope` struct.
16+
///
17+
///
18+
/// \param[in] name
19+
/// The value to place into the "name" key
20+
///
21+
/// \param[in] variablesReference
22+
/// The value to place into the "variablesReference" key
23+
///
24+
/// \param[in] namedVariables
25+
/// The value to place into the "namedVariables" key
26+
///
27+
/// \param[in] expensive
28+
/// The value to place into the "expensive" key
29+
///
30+
/// \return
31+
/// A `protocol::Scope`
32+
static Scope CreateScope(const llvm::StringRef name, int64_t variablesReference,
33+
int64_t namedVariables, bool expensive) {
34+
Scope scope;
35+
scope.name = name;
36+
37+
// TODO: Support "arguments" and "return value" scope.
38+
// At the moment lldb-dap includes the arguments and return_value into the
39+
// "locals" scope.
40+
// vscode only expands the first non-expensive scope, this causes friction
41+
// if we add the arguments above the local scope as the locals scope will not
42+
// be expanded if we enter a function with arguments. It becomes more
43+
// annoying when the scope has arguments, return_value and locals.
44+
if (variablesReference == VARREF_LOCALS)
45+
scope.presentationHint = Scope::eScopePresentationHintLocals;
46+
else if (variablesReference == VARREF_REGS)
47+
scope.presentationHint = Scope::eScopePresentationHintRegisters;
48+
49+
scope.variablesReference = variablesReference;
50+
scope.namedVariables = namedVariables;
51+
scope.expensive = expensive;
52+
53+
return scope;
54+
}
55+
56+
llvm::Expected<ScopesResponseBody>
57+
ScopesRequestHandler::Run(const ScopesArguments &args) const {
58+
lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId);
59+
7360
// As the user selects different stack frames in the GUI, a "scopes" request
7461
// will be sent to the DAP. This is the only way we know that the user has
7562
// selected a frame in a thread. There are no other notifications that are
@@ -78,17 +65,16 @@ void ScopesRequestHandler::operator()(const llvm::json::Object &request) const {
7865
// are sent, this allows users to type commands in the debugger console
7966
// with a backtick character to run lldb commands and these lldb commands
8067
// will now have the right context selected as they are run. If the user
81-
// types "`bt" into the debugger console and we had another thread selected
68+
// types "`bt" into the debugger console, and we had another thread selected
8269
// in the LLDB library, we would show the wrong thing to the user. If the
83-
// users switches threads with a lldb command like "`thread select 14", the
70+
// users switch threads with a lldb command like "`thread select 14", the
8471
// GUI will not update as there are no "event" notification packets that
8572
// allow us to change the currently selected thread or frame in the GUI that
8673
// I am aware of.
8774
if (frame.IsValid()) {
8875
frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
8976
frame.GetThread().SetSelectedFrame(frame.GetFrameID());
9077
}
91-
9278
dap.variables.locals = frame.GetVariables(/*arguments=*/true,
9379
/*locals=*/true,
9480
/*statics=*/false,
@@ -98,9 +84,15 @@ void ScopesRequestHandler::operator()(const llvm::json::Object &request) const {
9884
/*statics=*/true,
9985
/*in_scope_only=*/true);
10086
dap.variables.registers = frame.GetRegisters();
101-
body.try_emplace("scopes", dap.CreateTopLevelScopes());
102-
response.try_emplace("body", std::move(body));
103-
dap.SendJSON(llvm::json::Value(std::move(response)));
87+
88+
std::vector scopes = {CreateScope("Locals", VARREF_LOCALS,
89+
dap.variables.locals.GetSize(), false),
90+
CreateScope("Globals", VARREF_GLOBALS,
91+
dap.variables.globals.GetSize(), false),
92+
CreateScope("Registers", VARREF_REGS,
93+
dap.variables.registers.GetSize(), false)};
94+
95+
return ScopesResponseBody{std::move(scopes)};
10496
}
10597

10698
} // namespace lldb_dap

‎lldb/tools/lldb-dap/JSONUtils.h

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -238,27 +238,6 @@ llvm::json::Object CreateEventObject(const llvm::StringRef event_name);
238238
protocol::ExceptionBreakpointsFilter
239239
CreateExceptionBreakpointFilter(const ExceptionBreakpoint &bp);
240240

241-
/// Create a "Scope" JSON object as described in the debug adapter definition.
242-
///
243-
/// \param[in] name
244-
/// The value to place into the "name" key
245-
//
246-
/// \param[in] variablesReference
247-
/// The value to place into the "variablesReference" key
248-
//
249-
/// \param[in] namedVariables
250-
/// The value to place into the "namedVariables" key
251-
//
252-
/// \param[in] expensive
253-
/// The value to place into the "expensive" key
254-
///
255-
/// \return
256-
/// A "Scope" JSON object with that follows the formal JSON
257-
/// definition outlined by Microsoft.
258-
llvm::json::Value CreateScope(const llvm::StringRef name,
259-
int64_t variablesReference,
260-
int64_t namedVariables, bool expensive);
261-
262241
/// Create a "Source" JSON object as described in the debug adapter definition.
263242
///
264243
/// \param[in] file

‎lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,20 @@ llvm::json::Value toJSON(const SetVariableResponseBody &SVR) {
335335

336336
return llvm::json::Value(std::move(Body));
337337
}
338+
bool fromJSON(const llvm::json::Value &Params, ScopesArguments &SCA,
339+
llvm::json::Path P) {
340+
json::ObjectMapper O(Params, P);
341+
return O && O.map("frameId", SCA.frameId);
342+
}
343+
344+
llvm::json::Value toJSON(const ScopesResponseBody &SCR) {
345+
llvm::json::Array scopes;
346+
for (const Scope &scope : SCR.scopes) {
347+
scopes.emplace_back(toJSON(scope));
348+
}
349+
350+
return llvm::json::Object{{"scopes", std::move(scopes)}};
351+
}
338352

339353
bool fromJSON(const json::Value &Params, SourceArguments &SA, json::Path P) {
340354
json::ObjectMapper O(Params, P);

‎lldb/tools/lldb-dap/Protocol/ProtocolRequests.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,19 @@ struct SetVariableResponseBody {
439439
};
440440
llvm::json::Value toJSON(const SetVariableResponseBody &);
441441

442+
struct ScopesArguments {
443+
/// Retrieve the scopes for the stack frame identified by `frameId`. The
444+
/// `frameId` must have been obtained in the current suspended state. See
445+
/// 'Lifetime of Object References' in the Overview section for details.
446+
uint64_t frameId = LLDB_INVALID_FRAME_ID;
447+
};
448+
bool fromJSON(const llvm::json::Value &, ScopesArguments &, llvm::json::Path);
449+
450+
struct ScopesResponseBody {
451+
std::vector<Scope> scopes;
452+
};
453+
llvm::json::Value toJSON(const ScopesResponseBody &);
454+
442455
/// Arguments for `source` request.
443456
struct SourceArguments {
444457
/// Specifies the source content to load. Either `source.path` or

0 commit comments

Comments
 (0)