[lldb-vscode] Display a more descriptive summary for containers and pointers (#65514)

We've been displaying types and addresses for containers, but that's not
very useful information. A better approach is to compose the summary of
containers with the summary of a few of its children.

Not only that, we can dereference simple pointers and references to get
the summary of the pointer variable, which is also better than just
showing an anddress.

And in the rare case where the user wants to inspect the raw address,
they can always use the debug console for that.

For the record, this is very similar to what the CodeLLDB extension
does, and it seems to give a better experience.

An example of the new output:
<img width="494" alt="Screenshot 2023-09-06 at 2 24 27 PM"
src="https://github.com/llvm/llvm-project/assets/1613874/588659b8-421a-4865-8d67-ce4b6182c4f9">

And this is the 
<img width="476" alt="Screenshot 2023-09-06 at 2 46 30 PM"
src="https://github.com/llvm/llvm-project/assets/1613874/5768a52e-a773-449d-9aab-1b2fb2a98035">
old output:

Author: walter-erquinigo

lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py

@@ -55,7 +55,7 @@ def run_test_evaluate_expressions(self, context=None):
         self.assertEvaluate("var2", "21")
         self.assertEvaluate("static_int", "42")
         self.assertEvaluate("non_static_int", "43")
-        self.assertEvaluate("struct1", "my_struct @ 0x.*")
+        self.assertEvaluate("struct1", "{foo:15}")
         self.assertEvaluate("struct1.foo", "15")
         self.assertEvaluate("struct2->foo", "16")
 
@@ -85,7 +85,7 @@ def run_test_evaluate_expressions(self, context=None):
         self.assertEvaluate(
             "non_static_int", "10"
         )  # different variable with the same name
-        self.assertEvaluate("struct1", "my_struct @ 0x.*")
+        self.assertEvaluate("struct1", "{foo:15}")
         self.assertEvaluate("struct1.foo", "15")
         self.assertEvaluate("struct2->foo", "16")
 

lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py

@@ -2,12 +2,13 @@
 Test lldb-vscode setBreakpoints request
 """
 
+import os
+
+import lldbvscode_testcase
 import vscode
+from lldbsuite.test import lldbutil
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-import lldbvscode_testcase
-import os
 
 
 def make_buffer_verify_dict(start_idx, count, offset=0):
@@ -218,12 +219,12 @@ def test_scopes_variables_setVariable_evaluate(self):
             },
             "pt": {
                 "equals": {"type": "PointType"},
-                "startswith": {"result": "PointType @ 0x"},
+                "startswith": {"result": "{x:11, y:22}"},
                 "hasVariablesReference": True,
             },
             "pt.buffer": {
                 "equals": {"type": "int[32]"},
-                "startswith": {"result": "int[32] @ 0x"},
+                "startswith": {"result": "{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...}"},
                 "hasVariablesReference": True,
             },
             "argv": {
@@ -409,7 +410,7 @@ def test_scopes_and_evaluate_expansion(self):
             "name": "pt",
             "response": {
                 "equals": {"type": "PointType"},
-                "startswith": {"result": "PointType @ 0x"},
+                "startswith": {"result": "{x:11, y:22}"},
                 "missing": ["indexedVariables"],
                 "hasVariablesReference": True,
             },

lldb/tools/lldb-vscode/JSONUtils.cpp

@@ -132,6 +132,84 @@ std::vector<std::string> GetStrings(const llvm::json::Object *obj,
   return strs;
 }
 
+/// Create a short summary for a container that contains the summary of its
+/// first children, so that the user can get a glimpse of its contents at a
+/// glance.
+static std::optional<std::string>
+GetSyntheticSummaryForContainer(lldb::SBValue &v) {
+  if (v.TypeIsPointerType() || !v.MightHaveChildren())
+    return std::nullopt;
+  /// As this operation can be potentially slow, we limit the total time spent
+  /// fetching children to a few ms.
+  const auto max_evaluation_time = std::chrono::milliseconds(10);
+  /// We don't want to generate a extremely long summary string, so we limit its
+  /// length.
+  const size_t max_length = 32;
+
+  auto start = std::chrono::steady_clock::now();
+  std::string summary;
+  llvm::raw_string_ostream os(summary);
+  os << "{";
+
+  llvm::StringRef separator = "";
+
+  for (size_t i = 0, e = v.GetNumChildren(); i < e; ++i) {
+    // If we reached the time limit or exceeded the number of characters, we
+    // dump `...` to signal that there are more elements in the collection.
+    if (summary.size() > max_length ||
+        (std::chrono::steady_clock::now() - start) > max_evaluation_time) {
+      os << separator << "...";
+      break;
+    }
+    lldb::SBValue child = v.GetChildAtIndex(i);
+
+    if (llvm::StringRef name = child.GetName(); !name.empty()) {
+      llvm::StringRef value;
+      if (llvm::StringRef summary = child.GetSummary(); !summary.empty())
+        value = summary;
+      else
+        value = child.GetValue();
+
+      if (!value.empty()) {
+        // If the child is an indexed entry, we don't show its index to save
+        // characters.
+        if (name.starts_with("["))
+          os << separator << value;
+        else
+          os << separator << name << ":" << value;
+        separator = ", ";
+      }
+    }
+  }
+  os << "}";
+
+  if (summary == "{...}" || summary == "{}")
+    return std::nullopt;
+  return summary;
+}
+
+/// Return whether we should dereference an SBValue in order to generate a more
+/// meaningful summary string.
+static bool ShouldBeDereferencedForSummary(lldb::SBValue &v) {
+  if (!v.GetType().IsPointerType() && !v.GetType().IsReferenceType())
+    return false;
+
+  // If it's a pointer to a pointer, we don't dereference to avoid confusing
+  // the user with the addresses that could shown in the summary.
+  if (v.GetType().IsPointerType() &&
+      v.GetType().GetDereferencedType().IsPointerType())
+    return false;
+
+  // If it's synthetic or a pointer to a basic type that provides a summary, we
+  // don't dereference.
+  if ((v.IsSynthetic() || v.GetType().GetPointeeType().GetBasicType() !=
+                              lldb::eBasicTypeInvalid) &&
+      !llvm::StringRef(v.GetSummary()).empty()) {
+    return false;
+  }
+  return true;
+}
+
 void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object,
                     llvm::StringRef key) {
   std::string result;
@@ -141,23 +219,45 @@ void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object,
   if (!error.Success()) {
     strm << "<error: " << error.GetCString() << ">";
   } else {
-    llvm::StringRef value = v.GetValue();
-    llvm::StringRef summary = v.GetSummary();
-    llvm::StringRef type_name = v.GetType().GetDisplayTypeName();
-    if (!value.empty()) {
-      strm << value;
-      if (!summary.empty())
+    auto tryDumpSummaryAndValue = [&strm](lldb::SBValue value) {
+      llvm::StringRef val = value.GetValue();
+      llvm::StringRef summary = value.GetSummary();
+      if (!val.empty()) {
+        strm << val;
+        if (!summary.empty())
+          strm << ' ' << summary;
+        return true;
+      }
+      if (!summary.empty()) {
         strm << ' ' << summary;
-    } else if (!summary.empty()) {
-      strm << ' ' << summary;
-    } else if (!type_name.empty()) {
-      strm << type_name;
-      lldb::addr_t address = v.GetLoadAddress();
-      if (address != LLDB_INVALID_ADDRESS)
-        strm << " @ " << llvm::format_hex(address, 0);
+        return true;
+      }
+      if (auto container_summary = GetSyntheticSummaryForContainer(value)) {
+        strm << *container_summary;
+        return true;
+      }
+      return false;
+    };
+
+    // We first try to get the summary from its dereferenced value.
+    bool done = ShouldBeDereferencedForSummary(v) &&
+                tryDumpSummaryAndValue(v.Dereference());
+
+    // We then try to get it from the current value itself.
+    if (!done)
+      done = tryDumpSummaryAndValue(v);
+
+    // As last resort, we print its type and address if available.
+    if (!done) {
+      if (llvm::StringRef type_name = v.GetType().GetDisplayTypeName();
+          !type_name.empty()) {
+        strm << type_name;
+        lldb::addr_t address = v.GetLoadAddress();
+        if (address != LLDB_INVALID_ADDRESS)
+          strm << " @ " << llvm::format_hex(address, 0);
+      }
     }
   }
-  strm.flush();
   EmplaceSafeString(object, key, result);
 }