[Lldb-commits] [lldb] [lldb-vscode] Show addresses next to dereferenced summaries when using enableAutoVariableSummaries (PR #66551)

[Walter Erquinigo via lldb-commits]

https://github.com/walter-erquinigo created https://github.com/llvm/llvm-project/pull/66551

enableAutoVariableSummaries is a setting that enhances the summaries of variables in the IDE. A shortcoming of this feature is that it wasn't showing the addresses of pointers, which is valuable information. This fixes it by adding it whenever applicable.


>From 776f589a3fdfd160ce449461d0d2936e52849bd2 Mon Sep 17 00:00:00 2001
From: walter erquinigo <walter at modular.com>
Date: Fri, 15 Sep 2023 16:50:35 -0400
Subject: [PATCH] [lldb-vscode] Show addresses next to dereferenced summaries
 when using enableAutoVariableSummaries

enableAutoVariableSummaries is a setting that enhances the summaries of variables in the IDE. A shortcoming of this feature is that it wasn't showing the addresses of pointers, which is valuable information. This fixes it by adding it whenever applicable.
---
 .../evaluate/TestVSCode_evaluate.py           |  5 +-
 .../API/tools/lldb-vscode/evaluate/main.cpp   |  3 +-
 lldb/tools/lldb-vscode/JSONUtils.cpp          | 51 +++++++++++--------
 3 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py b/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py
index 3cfe02ef6aa1576..bf3b16067fed2fa 100644
--- a/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py
+++ b/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py
@@ -63,7 +63,10 @@ def run_test_evaluate_expressions(
             "struct1", "{foo:15}" if enableAutoVariableSummaries else "my_struct @ 0x"
         )
         self.assertEvaluate(
-            "struct2", "{foo:16}" if enableAutoVariableSummaries else "0x.*"
+            "struct2", "0x.* → {foo:16}" if enableAutoVariableSummaries else "0x.*"
+        )
+        self.assertEvaluate(
+            "struct3", "0x.*0"
         )
         self.assertEvaluate("struct1.foo", "15")
         self.assertEvaluate("struct2->foo", "16")
diff --git a/lldb/test/API/tools/lldb-vscode/evaluate/main.cpp b/lldb/test/API/tools/lldb-vscode/evaluate/main.cpp
index 3a541b21b220828..f09d00e6444bb79 100644
--- a/lldb/test/API/tools/lldb-vscode/evaluate/main.cpp
+++ b/lldb/test/API/tools/lldb-vscode/evaluate/main.cpp
@@ -18,6 +18,7 @@ struct my_struct {
 int main(int argc, char const *argv[]) {
   my_struct struct1 = {15};
   my_struct *struct2 = new my_struct{16};
+  my_struct *struct3 = nullptr;
   int var1 = 20;
   int var2 = 21;
   int var3 = static_int; // breakpoint 1
@@ -43,6 +44,6 @@ int main(int argc, char const *argv[]) {
   my_bool_vec.push_back(true);
   my_bool_vec.push_back(false); // breakpoint 6
   my_bool_vec.push_back(true); // breakpoint 7
-  
+
   return 0;
 }
diff --git a/lldb/tools/lldb-vscode/JSONUtils.cpp b/lldb/tools/lldb-vscode/JSONUtils.cpp
index c6b422e4d7a02e6..d475f45bec459bb 100644
--- a/lldb/tools/lldb-vscode/JSONUtils.cpp
+++ b/lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -203,10 +203,12 @@ static bool ShouldBeDereferencedForSummary(lldb::SBValue &v) {
   if (!v.GetType().IsPointerType() && !v.GetType().IsReferenceType())
     return false;
 
-  // If we are referencing a pointer, we don't dereference to avoid confusing
-  // the user with the addresses that could shown in the summary.
-  if (v.Dereference().GetType().IsPointerType())
-    return false;
+  // We don't want to dereference invalid data.
+  if (!v.IsSynthetic()) {
+    lldb::addr_t address = v.GetValueAsUnsigned(0);
+    if (address == 0 && address == LLDB_INVALID_ADDRESS)
+      return false;
+  }
 
   // If it's synthetic or a pointer to a basic type that provides a summary, we
   // don't dereference.
@@ -227,33 +229,40 @@ void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object,
   if (!error.Success()) {
     strm << "<error: " << error.GetCString() << ">";
   } else {
-    auto tryDumpSummaryAndValue = [&strm](lldb::SBValue value) {
+    auto tryDumpSummaryAndValue =
+        [](lldb::SBValue value) -> std::optional<std::string> {
       llvm::StringRef val = value.GetValue();
       llvm::StringRef summary = value.GetSummary();
       if (!val.empty()) {
-        strm << val;
+        std::string dump;
+        llvm::raw_string_ostream os(dump);
+        os << val;
         if (!summary.empty())
-          strm << ' ' << summary;
-        return true;
+          os << ' ' << summary;
+        return dump;
       }
-      if (!summary.empty()) {
-        strm << ' ' << summary;
-        return true;
-      }
-      if (auto container_summary = GetSyntheticSummaryForContainer(value)) {
-        strm << *container_summary;
-        return true;
-      }
-      return false;
+      if (!summary.empty())
+        return summary.str();
+      return GetSyntheticSummaryForContainer(value);
     };
 
+    bool done = false;
     // We first try to get the summary from its dereferenced value.
-    bool done = ShouldBeDereferencedForSummary(v) &&
-                tryDumpSummaryAndValue(v.Dereference());
+    if (ShouldBeDereferencedForSummary(v)) {
+      if (std::optional<std::string> text =
+              tryDumpSummaryAndValue(v.Dereference())) {
+        strm << v.GetValue() << " → " << *text;
+        done = true;
+      }
+    }
 
     // We then try to get it from the current value itself.
-    if (!done)
-      done = tryDumpSummaryAndValue(v);
+    if (!done) {
+      if (std::optional<std::string> text = tryDumpSummaryAndValue(v)) {
+        strm << *text;
+        done = true;
+      }
+    }
 
     // As last resort, we print its type and address if available.
     if (!done) {