Address comments

Author: vogelsgesang

lldb/tools/lldb-dap/JSONUtils.cpp

@@ -1210,6 +1210,14 @@ bool HasValueLocation(lldb::SBValue v) {
   return line_entry.IsValid();
 }
 
+int64_t PackLocation(int64_t var_ref, bool is_value_location) {
+   return var_ref << 1 | is_value_location;
+}
+
+std::pair<int64_t, bool> UnpackLocation(int64_t location_id) {
+   return std::pair{ location_id >> 1, location_id & 1};
+}
+
 // "Variable": {
 //   "type": "object",
 //   "description": "A Variable is a name/value pair. Optionally a variable
@@ -1414,10 +1422,10 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t var_ref,
     object.try_emplace("variablesReference", 0);
 
   if (v.GetDeclaration().IsValid())
-    object.try_emplace("declarationLocationReference", var_ref << 1);
+    object.try_emplace("declarationLocationReference", PackLocation(var_ref, false));
 
   if (HasValueLocation(v))
-    object.try_emplace("valueLocationReference", (var_ref << 1) | 1);
+    object.try_emplace("valueLocationReference", PackLocation(var_ref, true));
 
   if (lldb::addr_t addr = v.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS)
     object.try_emplace("memoryReference", EncodeMemoryReference(addr));

lldb/tools/lldb-dap/JSONUtils.h

@@ -461,6 +461,13 @@ struct VariableDescription {
 /// Does the given variable have an associated value location?
 bool HasValueLocation(lldb::SBValue v);
 
+/// Pack a location into a single integer which we can send via
+/// the debug adapter protocol.
+int64_t PackLocation(int64_t var_ref, bool is_value_location);
+
+/// Reverse of `PackLocation`
+std::pair<int64_t, bool> UnpackLocation(int64_t location_id);
+
 /// Create a "Variable" object for a LLDB thread object.
 ///
 /// This function will fill in the following keys in the returned

lldb/tools/lldb-dap/lldb-dap.cpp

@@ -1624,7 +1624,9 @@ void request_evaluate(const llvm::json::Object &request) {
       VariableDescription desc(value);
       EmplaceSafeString(body, "result", desc.GetResult(context));
       EmplaceSafeString(body, "type", desc.display_type_name);
-      auto var_ref = g_dap.variables.InsertVariable(
+      int64_t var_ref = 0;
+      if (value.MightHaveChildren() || HasValueLocation(value))
+         var_ref = g_dap.variables.InsertVariable(
           value, /*is_permanent=*/context == "repl");
       if (value.MightHaveChildren())
         body.try_emplace("variablesReference", var_ref);
@@ -4116,11 +4118,11 @@ void request_locations(const llvm::json::Object &request) {
   FillResponse(request, response);
   auto *arguments = request.getObject("arguments");
 
-  uint64_t reference_id = GetUnsigned(arguments, "locationReference", 0);
+  uint64_t location_id = GetUnsigned(arguments, "locationReference", 0);
   // We use the lowest bit to distinguish between value location and declaration
   // location
-  bool isValueLocation = reference_id & 1;
-  lldb::SBValue variable = g_dap.variables.GetVariable(reference_id >> 1);
+  auto [var_ref, is_value_location] = UnpackLocation(location_id);
+  lldb::SBValue variable = g_dap.variables.GetVariable(var_ref);
   if (!variable.IsValid()) {
     response["success"] = false;
     response["message"] = "Invalid variable reference";
@@ -4129,7 +4131,7 @@ void request_locations(const llvm::json::Object &request) {
   }
 
   llvm::json::Object body;
-  if (isValueLocation) {
+  if (is_value_location) {
     // Get the value location
     if (!variable.GetType().IsPointerType() &&
         !variable.GetType().IsReferenceType()) {