https://github.com/da-viper created https://github.com/llvm/llvm-project/pull/137803
```cpp // The "id" is the unique integer ID that is unique within the enclosing // variablesReference. It is optionally added to any "interface Variable" // objects to uniquely identify a variable within an enclosing // variablesReference. It helps to disambiguate between two variables that // have the same name within the same scope since the "setVariables" request // only specifies the variable reference of the enclosing scope/variable, and // the name of the variable. We could have two shadowed variables with the // same name in "Locals" or "Globals". In our case the "id" absolute index // of the variable within the dap.variables list. const auto id_value = GetInteger<uint64_t>(arguments, "id").value_or(UINT64_MAX); if (id_value != UINT64_MAX) { ``` I dropped this part because. variables that have the same name has a ` @path` suffix on both of them. and the setVariableArguments does not have a field called `id`. >From ba68d13a307f7e7228de3e658b97743b153ba49b Mon Sep 17 00:00:00 2001 From: Ebuka Ezike <yerimy...@gmail.com> Date: Tue, 29 Apr 2025 11:09:21 +0100 Subject: [PATCH] [lldb][lldb-dap] use the new protocol for set variable. --- lldb/tools/lldb-dap/Handler/RequestHandler.h | 11 +- .../Handler/SetVariableRequestHandler.cpp | 206 +++++------------- .../lldb-dap/Protocol/ProtocolRequests.cpp | 31 +++ .../lldb-dap/Protocol/ProtocolRequests.h | 69 ++++++ .../tools/lldb-dap/Protocol/ProtocolTypes.cpp | 6 + lldb/tools/lldb-dap/Protocol/ProtocolTypes.h | 7 + 6 files changed, 172 insertions(+), 158 deletions(-) diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index fa3d76ed4a125..37cc902e1c98e 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -430,17 +430,20 @@ class ScopesRequestHandler : public LegacyRequestHandler { void operator()(const llvm::json::Object &request) const override; }; -class SetVariableRequestHandler : public LegacyRequestHandler { +class SetVariableRequestHandler final + : public RequestHandler<protocol::SetVariableArguments, + llvm::Expected<protocol::SetVariableResponseBody>> { public: - using LegacyRequestHandler::LegacyRequestHandler; + using RequestHandler::RequestHandler; static llvm::StringLiteral GetCommand() { return "setVariable"; } FeatureSet GetSupportedFeatures() const override { return {protocol::eAdapterFeatureSetVariable}; } - void operator()(const llvm::json::Object &request) const override; + llvm::Expected<protocol::SetVariableResponseBody> + Run(const protocol::SetVariableArguments &args) const override; }; -class SourceRequestHandler +class SourceRequestHandler final : public RequestHandler<protocol::SourceArguments, llvm::Expected<protocol::SourceResponseBody>> { public: diff --git a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp index c48bcd84c9ddc..eab5d69deb529 100644 --- a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp @@ -13,168 +13,66 @@ namespace lldb_dap { -// "SetVariableRequest": { -// "allOf": [ { "$ref": "#/definitions/Request" }, { -// "type": "object", -// "description": "setVariable request; value of command field is -// 'setVariable'. Set the variable with the given name in the variable -// container to a new value.", "properties": { -// "command": { -// "type": "string", -// "enum": [ "setVariable" ] -// }, -// "arguments": { -// "$ref": "#/definitions/SetVariableArguments" -// } -// }, -// "required": [ "command", "arguments" ] -// }] -// }, -// "SetVariableArguments": { -// "type": "object", -// "description": "Arguments for 'setVariable' request.", -// "properties": { -// "variablesReference": { -// "type": "integer", -// "description": "The reference of the variable container." -// }, -// "name": { -// "type": "string", -// "description": "The name of the variable." -// }, -// "value": { -// "type": "string", -// "description": "The value of the variable." -// }, -// "format": { -// "$ref": "#/definitions/ValueFormat", -// "description": "Specifies details on how to format the response value." -// } -// }, -// "required": [ "variablesReference", "name", "value" ] -// }, -// "SetVariableResponse": { -// "allOf": [ { "$ref": "#/definitions/Response" }, { -// "type": "object", -// "description": "Response to 'setVariable' request.", -// "properties": { -// "body": { -// "type": "object", -// "properties": { -// "value": { -// "type": "string", -// "description": "The new value of the variable." -// }, -// "type": { -// "type": "string", -// "description": "The type of the new value. Typically shown in the -// UI when hovering over the value." -// }, -// "variablesReference": { -// "type": "number", -// "description": "If variablesReference is > 0, the new value is -// structured and its children can be retrieved by passing -// variablesReference to the VariablesRequest." -// }, -// "namedVariables": { -// "type": "number", -// "description": "The number of named child variables. The client -// can use this optional information to present the variables in a -// paged UI and fetch them in chunks." -// }, -// "indexedVariables": { -// "type": "number", -// "description": "The number of indexed child variables. The client -// can use this optional information to present the variables in a -// paged UI and fetch them in chunks." -// }, -// "valueLocationReference": { -// "type": "integer", -// "description": "A reference that allows the client to request the -// location where the new value is declared. For example, if the new -// value is function pointer, the adapter may be able to look up the -// function's location. This should be present only if the adapter -// is likely to be able to resolve the location.\n\nThis reference -// shares the same lifetime as the `variablesReference`. See -// 'Lifetime of Object References' in the Overview section for -// details." -// } -// }, -// "required": [ "value" ] -// } -// }, -// "required": [ "body" ] -// }] -// } -void SetVariableRequestHandler::operator()( - const llvm::json::Object &request) const { - llvm::json::Object response; - FillResponse(request, response); - llvm::json::Array variables; - llvm::json::Object body; - const auto *arguments = request.getObject("arguments"); - // This is a reference to the containing variable/scope - const auto variablesReference = - GetInteger<uint64_t>(arguments, "variablesReference").value_or(0); - llvm::StringRef name = GetString(arguments, "name").value_or(""); +/// Set the variable with the given name in the variable container to a new +/// value. Clients should only call this request if the corresponding capability +/// `supportsSetVariable` is true. +/// +/// If a debug adapter implements both `setVariable` and `setExpression`, +/// a client will only use `setExpression` if the variable has an evaluateName +/// property. +llvm::Expected<protocol::SetVariableResponseBody> +SetVariableRequestHandler::Run( + const protocol::SetVariableArguments &args) const { + const auto args_name = llvm::StringRef(args.name); - const auto value = GetString(arguments, "value").value_or(""); - // Set success to false just in case we don't find the variable by name - response.try_emplace("success", false); + constexpr llvm::StringRef return_value_name = "(Return Value)"; + if (args_name == return_value_name) + return llvm::make_error<DAPError>( + "cannot change the value of the return value"); - lldb::SBValue variable; + lldb::SBValue variable = + dap.variables.FindVariable(args.variablesReference, args_name); - // The "id" is the unique integer ID that is unique within the enclosing - // variablesReference. It is optionally added to any "interface Variable" - // objects to uniquely identify a variable within an enclosing - // variablesReference. It helps to disambiguate between two variables that - // have the same name within the same scope since the "setVariables" request - // only specifies the variable reference of the enclosing scope/variable, and - // the name of the variable. We could have two shadowed variables with the - // same name in "Locals" or "Globals". In our case the "id" absolute index - // of the variable within the dap.variables list. - const auto id_value = - GetInteger<uint64_t>(arguments, "id").value_or(UINT64_MAX); - if (id_value != UINT64_MAX) { - variable = dap.variables.GetVariable(id_value); - } else { - variable = dap.variables.FindVariable(variablesReference, name); - } + if (!variable.IsValid()) + return llvm::make_error<DAPError>("could not find variable in scope"); + + lldb::SBError error; + const bool success = variable.SetValueFromCString(args.value.c_str(), error); + if (!success) + return llvm::make_error<DAPError>(error.GetCString()); + + VariableDescription desc(variable, + dap.configuration.enableAutoVariableSummaries); - if (variable.IsValid()) { - lldb::SBError error; - bool success = variable.SetValueFromCString(value.data(), error); - if (success) { - VariableDescription desc(variable, - dap.configuration.enableAutoVariableSummaries); - EmplaceSafeString(body, "value", desc.display_value); - EmplaceSafeString(body, "type", desc.display_type_name); + auto body = protocol::SetVariableResponseBody{}; + body.value = desc.display_value; + body.type = desc.display_type_name; + + // We don't know the index of the variable in our dap.variables + // so always insert a new one to get its variablesReference. + // is_permanent is false because debug console does not support + // setVariable request. + const int64_t new_var_ref = + dap.variables.InsertVariable(variable, /*is_permanent=*/false); + if (variable.MightHaveChildren()) { + body.variablesReference = new_var_ref; + if (desc.type_obj.IsArrayType()) + body.indexedVariables = variable.GetNumChildren(); + else + body.namedVariables = variable.GetNumChildren(); - // We don't know the index of the variable in our dap.variables - // so always insert a new one to get its variablesReference. - // is_permanent is false because debug console does not support - // setVariable request. - int64_t new_var_ref = - dap.variables.InsertVariable(variable, /*is_permanent=*/false); - if (variable.MightHaveChildren()) - body.try_emplace("variablesReference", new_var_ref); - else - body.try_emplace("variablesReference", 0); - if (lldb::addr_t addr = variable.GetLoadAddress(); - addr != LLDB_INVALID_ADDRESS) - body.try_emplace("memoryReference", EncodeMemoryReference(addr)); - if (ValuePointsToCode(variable)) - body.try_emplace("valueLocationReference", new_var_ref); - } else { - EmplaceSafeString(body, "message", std::string(error.GetCString())); - } - response["success"] = llvm::json::Value(success); } else { - response["success"] = llvm::json::Value(false); + body.variablesReference = 0; } - response.try_emplace("body", std::move(body)); - dap.SendJSON(llvm::json::Value(std::move(response))); + if (lldb::addr_t addr = variable.GetLoadAddress(); + addr != LLDB_INVALID_ADDRESS) + body.memoryReference = EncodeMemoryReference(addr); + + if (ValuePointsToCode(variable)) + body.valueLocationReference = new_var_ref; + + return body; } } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp index 61fea66490c30..d9ffc0c04e134 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -261,6 +261,37 @@ bool fromJSON(const json::Value &Params, LaunchRequestArguments &LRA, parseEnv(Params, LRA.env, P) && parseTimeout(Params, LRA.timeout, P); } +bool fromJSON(const llvm::json::Value &Params, SetVariableArguments &SVA, + llvm::json::Path P) { + json::ObjectMapper O(Params, P); + return O && O.map("variablesReference", SVA.variablesReference) && + O.map("name", SVA.name) && O.map("value", SVA.value) && + O.mapOptional("format", SVA.format); +} + +llvm::json::Value toJSON(const SetVariableResponseBody &SVR) { + json::Object Body{{"value", SVR.value}}; + if (SVR.type.has_value()) + Body.insert({"type", SVR.type}); + + if (SVR.variablesReference.has_value()) + Body.insert({"variablesReference", SVR.variablesReference}); + + if (SVR.namedVariables.has_value()) + Body.insert({"namedVariables", SVR.namedVariables}); + + if (SVR.indexedVariables.has_value()) + Body.insert({"indexedVariables", SVR.indexedVariables}); + + if (SVR.memoryReference.has_value()) + Body.insert({"memoryReference", SVR.memoryReference}); + + if (SVR.valueLocationReference.has_value()) + Body.insert({"valueLocationReference", SVR.valueLocationReference}); + + return llvm::json::Value(std::move(Body)); +} + bool fromJSON(const json::Value &Params, SourceArguments &SA, json::Path P) { json::ObjectMapper O(Params, P); return O && O.map("source", SA.source) && diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index 33f93cc38799a..3c39e9b278d5d 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -294,6 +294,75 @@ bool fromJSON(const llvm::json::Value &, LaunchRequestArguments &, /// field is required. using LaunchResponseBody = VoidResponse; +/// Arguments for `setVariable` request. +struct SetVariableArguments { + /// The reference of the variable container. The `variablesReference` must + /// have been obtained in the current suspended state. See 'Lifetime of Object + /// References' in the Overview section for details. + uint64_t variablesReference; + + /// The name of the variable in the container. + std::string name; + + /// The value of the variable. + std::string value; + + /// Specifies details on how to format the response value. + ValueFormat format; +}; +bool fromJSON(const llvm::json::Value &, SetVariableArguments &, + llvm::json::Path); + +/// Response to `setVariable` request. +struct SetVariableResponseBody { + + /// The new value of the variable. + std::string value; + + /// The type of the new value. Typically shown in the UI when hovering over + /// the value. + std::optional<std::string> type; + + /// If `variablesReference` is > 0, the new value is structured and its + /// children can be retrieved by passing `variablesReference` to the + /// `variables` request as long as execution remains suspended. See 'Lifetime + /// of Object References' in the Overview section for details. + /// + /// If this property is included in the response, any `variablesReference` + /// previously associated with the updated variable, and those of its + /// children, are no longer valid. + std::optional<uint64_t> variablesReference; + + /// The number of named child variables. + /// The client can use this information to present the variables in a paged + /// UI and fetch them in chunks. + /// The value should be less than or equal to 2147483647 (2^31-1). + std::optional<uint64_t> namedVariables; + + /// The number of indexed child variables. + /// The client can use this information to present the variables in a paged + /// UI and fetch them in chunks. + /// The value should be less than or equal to 2147483647 (2^31-1). + std::optional<uint64_t> indexedVariables; + + /// A memory reference to a location appropriate for this result. + /// For pointer type eval results, this is generally a reference to the + /// memory address contained in the pointer. + /// This attribute may be returned by a debug adapter if corresponding + /// capability `supportsMemoryReferences` is true. + std::optional<std::string> memoryReference; + + /// A reference that allows the client to request the location where the new + /// value is declared. For example, if the new value is function pointer, the + /// adapter may be able to look up the function's location. This should be + /// present only if the adapter is likely to be able to resolve the location. + /// + /// This reference shares the same lifetime as the `variablesReference`. See + /// 'Lifetime of Object References' in the Overview section for details. + std::optional<uint64_t> valueLocationReference; +}; +llvm::json::Value toJSON(const SetVariableResponseBody &); + /// 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..8f4defb6fd2ea 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp @@ -254,4 +254,10 @@ bool fromJSON(const llvm::json::Value &Params, SteppingGranularity &SG, return true; } +bool fromJSON(const llvm::json::Value &Params, ValueFormat &VF, + llvm::json::Path P) { + json::ObjectMapper O(Params, P); + return O && O.mapOptional("hex", VF.hex); +} + } // namespace lldb_dap::protocol diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h index 54941f24efbd9..ae1ba90d1666a 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h @@ -322,6 +322,13 @@ enum SteppingGranularity : unsigned { bool fromJSON(const llvm::json::Value &, SteppingGranularity &, llvm::json::Path); +/// Provides formatting information for a value. +struct ValueFormat { + /// Display the value in hex. + std::optional<bool> hex; +}; +bool fromJSON(const llvm::json::Value &, ValueFormat &, llvm::json::Path); + } // namespace lldb_dap::protocol #endif _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits