[lldb-vscode] Use auto summaries whenever variables don't have a summary (#66551)

Auto summaries were only being used when non-pointer/reference variables
didn't have values nor summaries. Greg pointed out that it should be
better to simply use auto summaries when the variable doesn't have a
summary of its own, regardless of other conditions.

This led to code simplification and correct visualization of auto
summaries for pointer/reference types, as seen in this screenshot.

<img width="310" alt="Screenshot 2023-09-19 at 7 04 55 PM"
src="https://github.com/llvm/llvm-project/assets/1613874/d356d579-13f2-487b-ae3a-f3443dce778f">

Author: walter-erquinigo

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

@@ -63,8 +63,9 @@ 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")
 

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;
 }

lldb/tools/lldb-vscode/JSONUtils.cpp

@@ -136,14 +136,14 @@ std::vector<std::string> GetStrings(const llvm::json::Object *obj,
 /// 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) {
+TryCreateAutoSummaryForContainer(lldb::SBValue &v) {
   // We gate this feature because it performs GetNumChildren(), which can
   // cause performance issues because LLDB needs to complete possibly huge
   // types.
   if (!g_vsc.enable_auto_variable_summaries)
     return std::nullopt;
 
-  if (v.TypeIsPointerType() || !v.MightHaveChildren())
+  if (!v.MightHaveChildren())
     return std::nullopt;
   /// As this operation can be potentially slow, we limit the total time spent
   /// fetching children to a few ms.
@@ -194,28 +194,18 @@ GetSyntheticSummaryForContainer(lldb::SBValue &v) {
   return summary;
 }
 
-/// Return whether we should dereference an SBValue in order to generate a more
-/// meaningful summary string.
-static bool ShouldBeDereferencedForSummary(lldb::SBValue &v) {
+/// Try to create a summary string for the given value that doesn't have a
+/// summary of its own.
+static std::optional<std::string> TryCreateAutoSummary(lldb::SBValue value) {
   if (!g_vsc.enable_auto_variable_summaries)
-    return false;
-
-  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;
-
-  // 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;
+    return std::nullopt;
+
+  // We use the dereferenced value for generating the summary.
+  if (value.GetType().IsPointerType() || value.GetType().IsReferenceType())
+    value = value.Dereference();
+
+  // We only support auto summaries for containers.
+  return TryCreateAutoSummaryForContainer(value);
 }
 
 void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object,
@@ -227,36 +217,20 @@ void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object,
   if (!error.Success()) {
     strm << "<error: " << error.GetCString() << ">";
   } else {
-    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;
-        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) {
+    llvm::StringRef value = v.GetValue();
+    llvm::StringRef nonAutoSummary = v.GetSummary();
+    std::optional<std::string> summary = !nonAutoSummary.empty()
+                                             ? nonAutoSummary.str()
+                                             : TryCreateAutoSummary(v);
+    if (!value.empty()) {
+      strm << value;
+      if (summary)
+        strm << ' ' << *summary;
+    } else if (summary) {
+      strm << *summary;
+
+      // As last resort, we print its type and address if available.
+    } else {
       if (llvm::StringRef type_name = v.GetType().GetDisplayTypeName();
           !type_name.empty()) {
         strm << type_name;