[lldb][lldb-dap] use the new protocol for setVariable requests. (llvm#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`.

Author: da-viper

lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py

@@ -722,6 +722,10 @@ def test_return_variables(self):
                 self.verify_variables(verify_locals, local_variables, varref_dict)
                 break
 
+        self.assertFalse(
+            self.dap_server.request_setVariable(1, "(Return Value)", 20)["success"]
+        )
+
     @skipIfWindows
     def test_indexedVariables(self):
         self.do_test_indexedVariables(enableSyntheticChildDebugging=False)

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:

lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp

@@ -11,170 +11,76 @@
 #include "JSONUtils.h"
 #include "RequestHandler.h"
 
+using namespace lldb_dap::protocol;
+
 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("");
-
-  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);
-
-  lldb::SBValue variable;
-
-  // 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);
+/// 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<SetVariableResponseBody>
+SetVariableRequestHandler::Run(const SetVariableArguments &args) const {
+  const auto args_name = llvm::StringRef(args.name);
+
+  if (args.variablesReference == UINT64_MAX) {
+    return llvm::make_error<DAPError>(
+        llvm::formatv("invalid reference {}", args.variablesReference).str(),
+        llvm::inconvertibleErrorCode(),
+        /*show_user=*/false);
   }
 
-  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);
-
-      // 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);
+  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 =
+      dap.variables.FindVariable(args.variablesReference, args_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);
+
+  SetVariableResponseBody body;
+  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();
+
   } 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 (const 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

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) &&

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 = UINT64_MAX;
+
+  /// 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<uint32_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<uint32_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

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