[Lldb-commits] [lldb] 40a361a - [lldb-dap] Updating VariableDescription to use GetDescription() as a fallback. (#77026)

via lldb-commits lldb-commits at lists.llvm.org
Fri Jan 12 14:12:34 PST 2024


Author: John Harrison
Date: 2024-01-12T17:12:30-05:00
New Revision: 40a361acf5ce255054c5b2e5f67a24325bfe0398

URL: https://github.com/llvm/llvm-project/commit/40a361acf5ce255054c5b2e5f67a24325bfe0398
DIFF: https://github.com/llvm/llvm-project/commit/40a361acf5ce255054c5b2e5f67a24325bfe0398.diff

LOG: [lldb-dap] Updating VariableDescription to use GetDescription() as a fallback. (#77026)

When generating a `display_value` for a variable the current approach
calls `SBValue::GetValue()` and `SBValue::GetSummary()` to generate a
`display_value` for the `SBValue`. However, there are cases where both
of these return an empty string and the fallback is to print a pointer
and type name instead (e.g. `FooBarType @ 0x00321`).

For swift types, lldb includes a langauge runtime plugin that can
generate a description of the object but this is only used with
`SBValue::GetDescription()`.

For example:
```
$ lldb swift-binary
... stop at breakpoint ...
lldb> script
>>> event = lldb.frame.GetValueForVariablePath("event")
>>> print("Value", event.GetValue())
Value None
>>> print("Summary", event.GetSummary())
Summary None
>>> print("Description", event) # __str__ calls SBValue::GetDescription()
Description (main.Event) event = (name = "Greetings", time = 2024-01-04 23:38:06 UTC)
```

With this change, if GetValue and GetSummary return empty then we try
`SBValue::GetDescription()` as a fallback before using the previous
logic of printing `<type> @ <addr>`.

Added: 
    

Modified: 
    lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
    lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
    lldb/test/API/tools/lldb-dap/variables/main.cpp
    lldb/tools/lldb-dap/JSONUtils.cpp
    lldb/tools/lldb-dap/JSONUtils.h
    lldb/tools/lldb-dap/lldb-dap.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
index de9d2c93a1109d..7651a67b643094 100644
--- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
+++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
@@ -2,6 +2,7 @@
 Test lldb-dap completions request
 """
 
+import re
 
 import lldbdap_testcase
 import dap_server
@@ -10,7 +11,7 @@
 from lldbsuite.test.lldbtest import *
 
 
-class TestDAP_variables(lldbdap_testcase.DAPTestCaseBase):
+class TestDAP_evaluate(lldbdap_testcase.DAPTestCaseBase):
     def assertEvaluate(self, expression, regex):
         self.assertRegexpMatches(
             self.dap_server.request_evaluate(expression, context=self.context)["body"][
@@ -25,6 +26,9 @@ def assertEvaluateFailure(self, expression):
             self.dap_server.request_evaluate(expression, context=self.context)["body"],
         )
 
+    def isResultExpandedDescription(self):
+        return self.context == "repl" or self.context == "hover"
+
     def isExpressionParsedExpected(self):
         return self.context != "hover"
 
@@ -59,16 +63,30 @@ def run_test_evaluate_expressions(
         self.assertEvaluate("var2", "21")
         self.assertEvaluate("static_int", "42")
         self.assertEvaluate("non_static_int", "43")
-        self.assertEvaluate(
-            "struct1", "{foo:15}" if enableAutoVariableSummaries else "my_struct @ 0x"
-        )
-        self.assertEvaluate(
-            "struct2", "0x.* {foo:16}" if enableAutoVariableSummaries else "0x.*"
-        )
-        self.assertEvaluate("struct3", "0x.*0")
         self.assertEvaluate("struct1.foo", "15")
         self.assertEvaluate("struct2->foo", "16")
 
+        if self.isResultExpandedDescription():
+            self.assertEvaluate(
+                "struct1",
+                r"\(my_struct\) (struct1|\$\d+) = \(foo = 15\)",
+            )
+            self.assertEvaluate("struct2", r"\(my_struct \*\) (struct2|\$\d+) = 0x.*")
+            self.assertEvaluate(
+                "struct3", r"\(my_struct \*\) (struct3|\$\d+) = nullptr"
+            )
+        else:
+            self.assertEvaluate(
+                "struct1",
+                re.escape("{foo:15}")
+                if enableAutoVariableSummaries
+                else "my_struct @ 0x",
+            )
+            self.assertEvaluate(
+                "struct2", "0x.* {foo:16}" if enableAutoVariableSummaries else "0x.*"
+            )
+            self.assertEvaluate("struct3", "0x.*0")
+
         self.assertEvaluateFailure("var")  # local variable of a_function
         self.assertEvaluateFailure("my_struct")  # type name
         self.assertEvaluateFailure("int")  # type name
@@ -95,9 +113,18 @@ def run_test_evaluate_expressions(
         self.assertEvaluate(
             "non_static_int", "10"
         )  # 
diff erent variable with the same name
-        self.assertEvaluate(
-            "struct1", "{foo:15}" if enableAutoVariableSummaries else "my_struct @ 0x"
-        )
+        if self.isResultExpandedDescription():
+            self.assertEvaluate(
+                "struct1",
+                r"\(my_struct\) (struct1|\$\d+) = \(foo = 15\)",
+            )
+        else:
+            self.assertEvaluate(
+                "struct1",
+                re.escape("{foo:15}")
+                if enableAutoVariableSummaries
+                else "my_struct @ 0x",
+            )
         self.assertEvaluate("struct1.foo", "15")
         self.assertEvaluate("struct2->foo", "16")
 
@@ -164,16 +191,22 @@ def test_generic_evaluate_expressions(self):
     @skipIfRemote
     def test_repl_evaluate_expressions(self):
         # Tests expression evaluations that are triggered from the Debug Console
-        self.run_test_evaluate_expressions("repl", enableAutoVariableSummaries=True)
+        self.run_test_evaluate_expressions("repl", enableAutoVariableSummaries=False)
 
     @skipIfWindows
     @skipIfRemote
     def test_watch_evaluate_expressions(self):
         # Tests expression evaluations that are triggered from a watch expression
-        self.run_test_evaluate_expressions("watch", enableAutoVariableSummaries=False)
+        self.run_test_evaluate_expressions("watch", enableAutoVariableSummaries=True)
 
     @skipIfWindows
     @skipIfRemote
     def test_hover_evaluate_expressions(self):
         # Tests expression evaluations that are triggered when hovering on the editor
-        self.run_test_evaluate_expressions("hover", enableAutoVariableSummaries=True)
+        self.run_test_evaluate_expressions("hover", enableAutoVariableSummaries=False)
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_variable_evaluate_expressions(self):
+        # Tests expression evaluations that are triggered in the variable explorer
+        self.run_test_evaluate_expressions("variable", enableAutoVariableSummaries=True)

diff  --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
index 58d6b941ac81a7..b6d9b865bb8b6f 100644
--- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
+++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
@@ -39,7 +39,18 @@ def verify_values(self, verify_dict, actual, varref_dict=None, expression=None):
                 startswith = actual_value.startswith(verify_value)
                 self.assertTrue(
                     startswith,
-                    ('"%s" value "%s" doesn\'t start with' ' "%s")')
+                    ('"%s" value "%s" doesn\'t start with "%s")')
+                    % (key, actual_value, verify_value),
+                )
+        if "matches" in verify_dict:
+            verify = verify_dict["matches"]
+            for key in verify:
+                verify_value = verify[key]
+                actual_value = actual[key]
+                self.assertRegex(
+                    actual_value,
+                    verify_value,
+                    ('"%s" value "%s" doesn\'t match pattern "%s")')
                     % (key, actual_value, verify_value),
                 )
         if "contains" in verify_dict:
@@ -150,7 +161,7 @@ def do_test_scopes_variables_setVariable_evaluate(
         self.continue_to_breakpoints(breakpoint_ids)
         locals = self.dap_server.get_local_variables()
         globals = self.dap_server.get_global_variables()
-        buffer_children = make_buffer_verify_dict(0, 32)
+        buffer_children = make_buffer_verify_dict(0, 16)
         verify_locals = {
             "argc": {
                 "equals": {
@@ -243,18 +254,18 @@ def do_test_scopes_variables_setVariable_evaluate(
             "pt": {
                 "equals": {"type": "PointType"},
                 "startswith": {
-                    "result": "{x:11, y:22}"
+                    "result": "{x:11, y:22, buffer:{...}}"
                     if enableAutoVariableSummaries
                     else "PointType @ 0x"
                 },
                 "hasVariablesReference": True,
             },
             "pt.buffer": {
-                "equals": {"type": "int[32]"},
+                "equals": {"type": "int[16]"},
                 "startswith": {
                     "result": "{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...}"
                     if enableAutoVariableSummaries
-                    else "int[32] @ 0x"
+                    else "int[16] @ 0x"
                 },
                 "hasVariablesReference": True,
             },
@@ -440,7 +451,7 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo
                     },
                     "buffer": {
                         "children": buffer_children,
-                        "equals": {"indexedVariables": 32},
+                        "equals": {"indexedVariables": 16},
                     },
                 },
             },
@@ -455,15 +466,85 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo
         # the other temporary (from other UI).
         expandable_expression = {
             "name": "pt",
-            "response": {
-                "equals": {"type": "PointType"},
-                "startswith": {
-                    "result": "{x:11, y:22}"
-                    if enableAutoVariableSummaries
-                    else "PointType @ 0x"
+            "context": {
+                "repl": {
+                    "equals": {"type": "PointType"},
+                    "equals": {
+                        "result": """(PointType) $0 = {
+  x = 11
+  y = 22
+  buffer = {
+    [0] = 0
+    [1] = 1
+    [2] = 2
+    [3] = 3
+    [4] = 4
+    [5] = 5
+    [6] = 6
+    [7] = 7
+    [8] = 8
+    [9] = 9
+    [10] = 10
+    [11] = 11
+    [12] = 12
+    [13] = 13
+    [14] = 14
+    [15] = 15
+  }
+}"""
+                    },
+                    "missing": ["indexedVariables"],
+                    "hasVariablesReference": True,
+                },
+                "hover": {
+                    "equals": {"type": "PointType"},
+                    "equals": {
+                        "result": """(PointType) pt = {
+  x = 11
+  y = 22
+  buffer = {
+    [0] = 0
+    [1] = 1
+    [2] = 2
+    [3] = 3
+    [4] = 4
+    [5] = 5
+    [6] = 6
+    [7] = 7
+    [8] = 8
+    [9] = 9
+    [10] = 10
+    [11] = 11
+    [12] = 12
+    [13] = 13
+    [14] = 14
+    [15] = 15
+  }
+}"""
+                    },
+                    "missing": ["indexedVariables"],
+                    "hasVariablesReference": True,
+                },
+                "watch": {
+                    "equals": {"type": "PointType"},
+                    "startswith": {
+                        "result": "{x:11, y:22, buffer:{...}}"
+                        if enableAutoVariableSummaries
+                        else "PointType @ 0x"
+                    },
+                    "missing": ["indexedVariables"],
+                    "hasVariablesReference": True,
+                },
+                "variables": {
+                    "equals": {"type": "PointType"},
+                    "startswith": {
+                        "result": "{x:11, y:22, buffer:{...}}"
+                        if enableAutoVariableSummaries
+                        else "PointType @ 0x"
+                    },
+                    "missing": ["indexedVariables"],
+                    "hasVariablesReference": True,
                 },
-                "missing": ["indexedVariables"],
-                "hasVariablesReference": True,
             },
             "children": {
                 "x": {"equals": {"type": "int", "value": "11"}},
@@ -472,27 +553,21 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo
             },
         }
 
-        # Evaluate from permanent UI.
-        permanent_expr_varref_dict = {}
-        response = self.dap_server.request_evaluate(
-            expandable_expression["name"], frameIndex=0, threadId=None, context="repl"
-        )
-        self.verify_values(
-            expandable_expression["response"],
-            response["body"],
-            permanent_expr_varref_dict,
-            expandable_expression["name"],
-        )
-
-        # Evaluate from temporary UI.
-        temporary_expr_varref_dict = {}
-        response = self.dap_server.request_evaluate(expandable_expression["name"])
-        self.verify_values(
-            expandable_expression["response"],
-            response["body"],
-            temporary_expr_varref_dict,
-            expandable_expression["name"],
-        )
+        # Evaluate from known contexts.
+        expr_varref_dict = {}
+        for context, verify_dict in expandable_expression["context"].items():
+            response = self.dap_server.request_evaluate(
+                expandable_expression["name"],
+                frameIndex=0,
+                threadId=None,
+                context=context,
+            )
+            self.verify_values(
+                verify_dict,
+                response["body"],
+                expr_varref_dict,
+                expandable_expression["name"],
+            )
 
         # Evaluate locals again.
         locals = self.dap_server.get_local_variables()
@@ -500,7 +575,7 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo
 
         # Verify the evaluated expressions before second locals evaluation
         # can be expanded.
-        var_ref = temporary_expr_varref_dict[expandable_expression["name"]]
+        var_ref = expr_varref_dict[expandable_expression["name"]]
         response = self.dap_server.request_variables(var_ref)
         self.verify_variables(
             expandable_expression["children"], response["body"]["variables"]
@@ -516,7 +591,7 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo
         )
         self.continue_to_breakpoints(breakpoint_ids)
 
-        var_ref = permanent_expr_varref_dict[expandable_expression["name"]]
+        var_ref = expr_varref_dict[expandable_expression["name"]]
         response = self.dap_server.request_variables(var_ref)
         self.verify_variables(
             expandable_expression["children"], response["body"]["variables"]

diff  --git a/lldb/test/API/tools/lldb-dap/variables/main.cpp b/lldb/test/API/tools/lldb-dap/variables/main.cpp
index da09ecb474f3b2..3557067ed9ce13 100644
--- a/lldb/test/API/tools/lldb-dap/variables/main.cpp
+++ b/lldb/test/API/tools/lldb-dap/variables/main.cpp
@@ -1,5 +1,5 @@
 
-#define BUFFER_SIZE 32
+#define BUFFER_SIZE 16
 struct PointType {
   int x;
   int y;

diff  --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index df17ac9d849176..a8b438d9d6df39 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -135,6 +135,12 @@ std::vector<std::string> GetStrings(const llvm::json::Object *obj,
   return strs;
 }
 
+static bool IsClassStructOrUnionType(lldb::SBType t) {
+  return (t.GetTypeClass() & (lldb::eTypeClassUnion | lldb::eTypeClassStruct |
+                              lldb::eTypeClassUnion | lldb::eTypeClassArray)) !=
+         0;
+}
+
 /// 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.
@@ -173,21 +179,23 @@ TryCreateAutoSummaryForContainer(lldb::SBValue &v) {
     lldb::SBValue child = v.GetChildAtIndex(i);
 
     if (llvm::StringRef name = child.GetName(); !name.empty()) {
-      llvm::StringRef value;
+      llvm::StringRef desc;
       if (llvm::StringRef summary = child.GetSummary(); !summary.empty())
-        value = summary;
+        desc = summary;
+      else if (llvm::StringRef value = child.GetValue(); !value.empty())
+        desc = value;
+      else if (IsClassStructOrUnionType(child.GetType()))
+        desc = "{...}";
       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 = ", ";
-      }
+        continue;
+
+      // If the child is an indexed entry, we don't show its index to save
+      // characters.
+      if (name.starts_with("["))
+        os << separator << desc;
+      else
+        os << separator << name << ":" << desc;
+      separator = ", ";
     }
   }
   os << "}";
@@ -1091,6 +1099,23 @@ llvm::json::Object VariableDescription::GetVariableExtensionsJSON() {
   return extensions;
 }
 
+std::string VariableDescription::GetResult(llvm::StringRef context) {
+  // In repl and hover context, the results can be displayed as multiple lines
+  // so more detailed descriptions can be returned.
+  if (context != "repl" && context != "hover")
+    return display_value;
+
+  if (!v.IsValid())
+    return display_value;
+
+  // Try the SBValue::GetDescription(), which may call into language runtime
+  // specific formatters (see ValueObjectPrinter).
+  lldb::SBStream stream;
+  v.GetDescription(stream);
+  llvm::StringRef description = stream.GetData();
+  return description.trim().str();
+}
+
 // "Variable": {
 //   "type": "object",
 //   "description": "A Variable is a name/value pair. Optionally a variable

diff  --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index 7f9fc7037c63d6..62338548890c0c 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -405,6 +405,9 @@ struct VariableDescription {
   /// Create a JSON object that represents these extensions to the DAP variable
   /// response.
   llvm::json::Object GetVariableExtensionsJSON();
+
+  /// Returns a description of the value appropriate for the specified context.
+  std::string GetResult(llvm::StringRef context);
 };
 
 /// Create a "Variable" object for a LLDB thread object.

diff  --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 0d45b0379c8526..e91b4115156b73 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1317,7 +1317,7 @@ void request_evaluate(const llvm::json::Object &request) {
         EmplaceSafeString(response, "message", "evaluate failed");
     } else {
       VariableDescription desc(value);
-      EmplaceSafeString(body, "result", desc.display_value);
+      EmplaceSafeString(body, "result", desc.GetResult(context));
       EmplaceSafeString(body, "type", desc.display_type_name);
       if (value.MightHaveChildren()) {
         auto variableReference = g_dap.variables.InsertExpandableVariable(


        


More information about the lldb-commits mailing list