llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Ebuka Ezike (da-viper) <details> <summary>Changes</summary> --- Patch is 27.92 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/138116.diff 14 Files Affected: - (modified) lldb/tools/lldb-dap/DAP.cpp (+1-14) - (modified) lldb/tools/lldb-dap/DAP.h (+1-3) - (modified) lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp (+3-1) - (modified) lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp (+4-1) - (modified) lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp (+4-1) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+7-3) - (modified) lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp (+56-65) - (modified) lldb/tools/lldb-dap/Handler/StepInTargetsRequestHandler.cpp (+4-1) - (modified) lldb/tools/lldb-dap/JSONUtils.cpp (-85) - (modified) lldb/tools/lldb-dap/JSONUtils.h (-21) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+14) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+13) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp (+82-6) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.h (+74-6) ``````````diff diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 4cb0d8e49004c..cad120ddd0621 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -514,9 +514,7 @@ lldb::SBThread DAP::GetLLDBThread(const llvm::json::Object &arguments) { return target.GetProcess().GetThreadByID(tid); } -lldb::SBFrame DAP::GetLLDBFrame(const llvm::json::Object &arguments) { - const uint64_t frame_id = - GetInteger<uint64_t>(arguments, "frameId").value_or(UINT64_MAX); +lldb::SBFrame DAP::GetLLDBFrame(uint64_t frame_id) { lldb::SBProcess process = target.GetProcess(); // Upper 32 bits is the thread index ID lldb::SBThread thread = @@ -525,17 +523,6 @@ lldb::SBFrame DAP::GetLLDBFrame(const llvm::json::Object &arguments) { return thread.GetFrameAtIndex(GetLLDBFrameID(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. diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 88eedb0860cf1..d0b4e3987b88c 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -275,9 +275,7 @@ struct DAP { lldb::SBThread GetLLDBThread(lldb::tid_t id); lldb::SBThread GetLLDBThread(const llvm::json::Object &arguments); - lldb::SBFrame GetLLDBFrame(const llvm::json::Object &arguments); - - llvm::json::Value CreateTopLevelScopes(); + lldb::SBFrame GetLLDBFrame(uint64_t frame_id); void PopulateExceptionBreakpoints(); diff --git a/lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp b/lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp index c72fc5686cd5b..65686cd23b243 100644 --- a/lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp @@ -136,7 +136,9 @@ void CompletionsRequestHandler::operator()( const auto *arguments = request.getObject("arguments"); // If we have a frame, try to set the context for variable completions. - lldb::SBFrame frame = dap.GetLLDBFrame(*arguments); + const uint64_t frame_id = + GetInteger<uint64_t>(*arguments, "frameId").value_or(UINT64_MAX); + lldb::SBFrame frame = dap.GetLLDBFrame(frame_id); if (frame.IsValid()) { frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread()); frame.GetThread().SetSelectedFrame(frame.GetFrameID()); diff --git a/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp index 4d920f8556254..76407d230438d 100644 --- a/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp @@ -118,7 +118,10 @@ void DataBreakpointInfoRequestHandler::operator()( const auto variablesReference = GetInteger<uint64_t>(arguments, "variablesReference").value_or(0); llvm::StringRef name = GetString(arguments, "name").value_or(""); - lldb::SBFrame frame = dap.GetLLDBFrame(*arguments); + + const uint64_t frame_id = + GetInteger<uint64_t>(arguments, "frameId").value_or(UINT64_MAX); + lldb::SBFrame frame = dap.GetLLDBFrame(frame_id); lldb::SBValue variable = dap.variables.FindVariable(variablesReference, name); std::string addr, size; diff --git a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp index 5ce133c33b7e1..01b5f956ba953 100644 --- a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp @@ -144,7 +144,10 @@ void EvaluateRequestHandler::operator()( FillResponse(request, response); llvm::json::Object body; const auto *arguments = request.getObject("arguments"); - lldb::SBFrame frame = dap.GetLLDBFrame(*arguments); + + const uint64_t frame_id = + GetInteger<uint64_t>(arguments, "frameId").value_or(UINT64_MAX); + lldb::SBFrame frame = dap.GetLLDBFrame(frame_id); std::string expression = GetString(arguments, "expression").value_or("").str(); const llvm::StringRef context = GetString(arguments, "context").value_or(""); diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index fa3d76ed4a125..00eeb66f10670 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -423,11 +423,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 : public LegacyRequestHandler { diff --git a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp index 7d1608f59f9a4..d9dd29f7269f2 100644 --- a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp @@ -7,69 +7,55 @@ //===----------------------------------------------------------------------===// #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. add presentation hint; + // vscode only expands the first non-expensive scope, this causes friction + // as the locals scope will not be expanded. It becomes more annoying when + // the scope has arguments, return_value and locals. + if (variablesReference == VARREF_LOCALS) + scope.presentationHint = Scope::ePresentationHintLocals; + else if (variablesReference == VARREF_REGS) + scope.presentationHint = Scope::ePresentationHintRegisters; + + 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 @@ -78,9 +64,9 @@ 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. @@ -88,7 +74,6 @@ void ScopesRequestHandler::operator()(const llvm::json::Object &request) const { frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread()); frame.GetThread().SetSelectedFrame(frame.GetFrameID()); } - dap.variables.locals = frame.GetVariables(/*arguments=*/true, /*locals=*/true, /*statics=*/false, @@ -98,9 +83,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)}; } } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/StepInTargetsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/StepInTargetsRequestHandler.cpp index 9b99791599f82..e7c568e9e9e60 100644 --- a/lldb/tools/lldb-dap/Handler/StepInTargetsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/StepInTargetsRequestHandler.cpp @@ -74,7 +74,10 @@ void StepInTargetsRequestHandler::operator()( const auto *arguments = request.getObject("arguments"); dap.step_in_targets.clear(); - lldb::SBFrame frame = dap.GetLLDBFrame(*arguments); + + const uint64_t frame_id = + GetInteger<uint64_t>(arguments, "frameId").value_or(UINT64_MAX); + lldb::SBFrame frame = dap.GetLLDBFrame(frame_id); if (frame.IsValid()) { lldb::SBAddress pc_addr = frame.GetPCAddress(); lldb::SBAddress line_end_addr = diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 4409cf5b27e5b..5e6f1f3942dc5 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -269,91 +269,6 @@ void FillResponse(const llvm::json::Object &request, response.try_emplace("success", true); } -// "Scope": { -// "type": "object", -// "description": "A Scope is a named container for variables. Optionally -// a scope can map to a source or a range within a source.", -// "properties": { -// "name": { -// "type": "string", -// "description": "Name of the scope such as 'Arguments', 'Locals'." -// }, -// "presentationHint": { -// "type": "string", -// "description": "An optional hint for how to present this scope in the -// UI. If this attribute is missing, the scope is shown -// with a generic UI.", -// "_enum": [ "arguments", "locals", "registers" ], -// }, -// "variablesReference": { -// "type": "integer", -// "description": "The variables of this scope can be retrieved by -// passing the value of variablesReference to the -// VariablesRequest." -// }, -// "namedVariables": { -// "type": "integer", -// "description": "The number of named variables in this scope. The -// client can use this optional information to present -// the variables in a paged UI and fetch them in chunks." -// }, -// "indexedVariables": { -// "type": "integer", -// "description": "The number of indexed variables in this scope. The -// client can use this optional information to present -// the variables in a paged UI and fetch them in chunks." -// }, -// "expensive": { -// "type": "boolean", -// "description": "If true, the number of variables in this scope is -// large or expensive to retrieve." -// }, -// "source": { -// "$ref": "#/definitions/Source", -// "description": "Optional source for this scope." -// }, -// "line": { -// "type": "integer", -// "description": "Optional start line of the range covered by this -// scope." -// }, -// "column": { -// "type": "integer", -// "description": "Optional start column of the range covered by this -// scope." -// }, -// "endLine": { -// "type": "integer", -// "description": "Optional end line of the range covered by this scope." -// }, -// "endColumn": { -// "type": "integer", -// "description": "Optional end column of the range covered by this -// scope." -// } -// }, -// "required": [ "name", "variablesReference", "expensive" ] -// } -llvm::json::Value CreateScope(const llvm::StringRef name, - int64_t variablesReference, - int64_t namedVariables, bool expensive) { - llvm::json::Object object; - EmplaceSafeString(object, "name", name.str()); - - // TODO: Support "arguments" scope. At the moment lldb-dap includes the - // arguments into the "locals" scope. - if (variablesReference == VARREF_LOCALS) { - object.try_emplace("presentationHint", "locals"); - } else if (variablesReference == VARREF_REGS) { - object.try_emplace("presentationHint", "registers"); - } - - object.try_emplace("variablesReference", variablesReference); - object.try_emplace("expensive", expensive); - object.try_emplace("namedVariables", namedVariables); - return llvm::json::Value(std::move(object)); -} - // "Breakpoint": { // "type": "object", // "description": "Information about a Breakpoint created in setBreakpoints diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h index d0e20729f4ed9..1d9f72c3763c9 100644 --- a/lldb/tools/lldb-dap/JSONUtils.h +++ b/lldb/tools/lldb-dap/JSONUtils.h @@ -294,27 +294,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 diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp index 61fea66490c30..aa8f193140d7a 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -260,6 +260,20 @@ bool fromJSON(const json::Value &Params, LaunchRequestArguments &LRA, O.mapOptional("runInTerminal", LRA.runInTerminal) && parseEnv(Params, LRA.env, P) && parseTimeout(Params, LRA.timeout, P); } +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); diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index 33f93cc38799a..9ac5468608e1e 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -294,6 +294,19 @@ bool fromJSON(const llvm::json::Value &, LaunchRequestArguments &, /// field is required. using LaunchResponseBody = VoidResponse; +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; +}; +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 diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp index e64998c4ca488..0f382a8dc66c3 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp @@ -16,17 +16,18 @@ using name... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/138116 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
