[lldb][lldb-dap] Add review changes.

Author: da-viper

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

@@ -721,6 +721,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/SetVariableRequestHandler.cpp

@@ -11,6 +11,8 @@
 #include "JSONUtils.h"
 #include "RequestHandler.h"
 
+using namespace lldb_dap::protocol;
+
 namespace lldb_dap {
 
 /// Set the variable with the given name in the variable container to a new
@@ -20,11 +22,17 @@ namespace lldb_dap {
 /// 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 {
+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);
+  }
+
   constexpr llvm::StringRef return_value_name = "(Return Value)";
   if (args_name == return_value_name)
     return llvm::make_error<DAPError>(
@@ -44,7 +52,7 @@ SetVariableRequestHandler::Run(
   VariableDescription desc(variable,
                            dap.configuration.enableAutoVariableSummaries);
 
-  auto body = protocol::SetVariableResponseBody{};
+  SetVariableResponseBody body;
   body.value = desc.display_value;
   body.type = desc.display_type_name;
 
@@ -65,7 +73,7 @@ SetVariableRequestHandler::Run(
     body.variablesReference = 0;
   }
 
-  if (lldb::addr_t addr = variable.GetLoadAddress();
+  if (const lldb::addr_t addr = variable.GetLoadAddress();
       addr != LLDB_INVALID_ADDRESS)
     body.memoryReference = EncodeMemoryReference(addr);
 

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

@@ -299,7 +299,7 @@ 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_t variablesReference = UINT64_MAX;
 
   /// The name of the variable in the container.
   std::string name;
@@ -337,13 +337,13 @@ struct SetVariableResponseBody {
   /// 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;
+  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<uint64_t> indexedVariables;
+  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