[binutils-gdb] Put pretty-printers to_string output in varobj result

[binutils-gdb] Put pretty-printers to_string output in varobj result

[Tom Tromey]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3e8154778baf4df222d95e43a2c8fefdb17305f7

commit 3e8154778baf4df222d95e43a2c8fefdb17305f7
Author: Tom Tromey <tromey@adacore.com>
Date:   Thu May 26 08:33:05 2022 -0600

    Put pretty-printers to_string output in varobj result
    
    PR mi/11335 points out that an MI varobj will not display the result
    of a pretty-printer's "to_string" method.  Instead, it always shows
    "{...}".
    
    This does not seem very useful, and there have been multiple
    complaints about it over the years.  This patch changes varobj to emit
    this string when possible, and updates the test suite.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=11335

Diff:
---
 gdb/doc/gdb.texinfo                |  5 +++--
 gdb/testsuite/gdb.python/py-mi.exp | 19 +++++++++++--------
 gdb/testsuite/lib/mi-support.exp   |  8 ++++----
 gdb/varobj.c                       | 29 ++++++++---------------------
 4 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 6c811b8be2e..e8e2c718e45 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -34523,8 +34523,9 @@ reliable for a dynamic varobj.  Instead, you must examine the
 
 @item value
 The varobj's scalar value.  For a varobj whose type is some sort of
-aggregate (e.g., a @code{struct}), or for a dynamic varobj, this value
-will not be interesting.
+aggregate (e.g., a @code{struct}), this value will not be interesting.
+For a dynamic varobj, this value comes directly from the Python
+pretty-printer object's @code{to_string} method.
 
 @item type
 The varobj's type.  This is a string representation of the type, as
diff --git a/gdb/testsuite/gdb.python/py-mi.exp b/gdb/testsuite/gdb.python/py-mi.exp
index a7b7a5ffcd3..97b8a42f715 100644
--- a/gdb/testsuite/gdb.python/py-mi.exp
+++ b/gdb/testsuite/gdb.python/py-mi.exp
@@ -91,7 +91,7 @@ mi_gdb_test "-var-update string" \
 with_test_prefix "varobj container" {
     # The "elements" field of "c" is still empty, so the attribute
     # "has_more" is expected to be zero.
-    mi_create_dynamic_varobj container c 0 \
+    mi_create_dynamic_varobj container c "container .* with 0 elements" 0 \
 	"create container varobj"
 
     mi_list_varobj_children container {
@@ -207,7 +207,7 @@ mi_next "next over update 5"
 
 # Regression test: examine an object that has no children, then update
 # it to ensure that we don't print the children.
-mi_create_dynamic_varobj container2 c2 0 \
+mi_create_dynamic_varobj container2 c2 "container .* with 0 elements" 0 \
   "create second container varobj"
 
 mi_gdb_test "-var-update container2" \
@@ -228,7 +228,7 @@ mi_continue_to_line \
     [gdb_get_line_number {MI outer breakpoint here} ${srcfile}] \
     "step to first outer breakpoint"
 
-mi_create_dynamic_varobj outer outer 1 \
+mi_create_dynamic_varobj outer outer "x = 0" 1 \
   "create outer varobj"
 
 mi_list_varobj_children outer {
@@ -259,7 +259,7 @@ mi_continue_to_line \
     "step to second outer breakpoint"
 
 with_test_prefix "varobj nscont" {
-    mi_create_dynamic_varobj nscont nstype 1 \
+    mi_create_dynamic_varobj nscont nstype ".*" 1 \
 	"create nstype varobj"
 
     mi_list_varobj_children nscont {
@@ -284,7 +284,7 @@ set cmd "python exception_flag = True"
 set re [string_list_to_regexp & {"} $cmd \\ n {"} \r\n ^ done]
 mi_gdb_test $cmd $re
 
-mi_create_dynamic_varobj nstype2 nstype2 1 \
+mi_create_dynamic_varobj nstype2 nstype2 ".*" 1 \
   "create nstype2 varobj"
 
 mi_list_varobj_children nstype2 {
@@ -299,7 +299,8 @@ mi_gdb_test "-var-evaluate-expression me" \
 	"evaluate me varobj"
 
 # Regression test for python/14836.
-mi_create_dynamic_varobj children_as_list children_as_list 1 \
+mi_create_dynamic_varobj children_as_list children_as_list \
+    children_as_list_val 1 \
     "printer whose children are returned as a list"
 
 # Test that when a pretty-printer returns a gdb.Value in its to_string, we call
@@ -317,7 +318,7 @@ mi_continue_to_line \
     [gdb_get_line_number {breakpoint bug 14741} ${srcfile}] \
     "step to breakpoint for bug 14741"
 
-mi_create_dynamic_varobj c c 1 \
+mi_create_dynamic_varobj c c "container .* with 1 elements" 1 \
   "create varobj for c"
 
 mi_gdb_test "-var-set-visualizer c ArrayPrinter" \
@@ -330,8 +331,10 @@ mi_list_varobj_children c {
 
 mi_next "next over change of array element"
 
+# 'c' is noticed as changing here due to an artifact of the
+# -var-update implementation.  However, it seems harmless.
 mi_gdb_test "-var-update c" \
-    "\\^done,changelist=\\\[{name=\"c.\\\[0\\\]\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"}\\\]" \
+    "\\^done,changelist=\\\[{name=\"c\",in_scope=\"true\",type_changed=\"false\",displayhint=\"array\",dynamic=\"1\",has_more=\"0\"},{name=\"c.\\\[0\\\]\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"}\\\]" \
     "update varobj after element change"
 
 # C++ MI tests
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index c738b7be505..c914ed49150 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -1426,11 +1426,11 @@ proc mi_create_varobj_checked { name expression type testname } {
 }
 
 # Same as mi_create_floating_varobj, but assumes the test is creating
-# a dynamic varobj that has children, so the value must be "{...}".
-# The "has_more" attribute is checked.
-proc mi_create_dynamic_varobj {name expression has_more testname} {
+# a dynamic varobj that has children.  The "value" and "has_more"
+# attributes are checked.
+proc mi_create_dynamic_varobj {name expression value has_more testname} {
     mi_gdb_test "-var-create $name @ $expression" \
-	"\\^done,name=\"$name\",numchild=\"0\",value=\"{\\.\\.\\.}\",type=.*,has_more=\"${has_more}\"" \
+	"\\^done,name=\"$name\",numchild=\"0\",value=\"$value\",type=.*,has_more=\"${has_more}\"" \
 	$testname
 }
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 1d545382f5e..2c35b911bad 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -652,21 +652,6 @@ install_dynamic_child (struct varobj *var,
     }
 }
 
-#if HAVE_PYTHON
-
-static bool
-dynamic_varobj_has_child_method (const struct varobj *var)
-{
-  PyObject *printer = var->dynamic->pretty_printer;
-
-  if (!gdb_python_initialized)
-    return false;
-
-  gdbpy_enter_varobj enter_py (var);
-  return PyObject_HasAttr (printer, gdbpy_children_cst);
-}
-#endif
-
 /* A factory for creating dynamic varobj's iterators.  Returns an
    iterator object suitable for iterating over VAR's children.  */
 
@@ -2178,11 +2163,6 @@ varobj_value_get_print_value (struct value *value,
 
       if (value_formatter)
 	{
-	  /* First check to see if we have any children at all.  If so,
-	     we simply return {...}.  */
-	  if (dynamic_varobj_has_child_method (var))
-	    return "{...}";
-
 	  if (PyObject_HasAttr (value_formatter, gdbpy_to_string_cst))
 	    {
 	      struct value *replacement;
@@ -2193,7 +2173,7 @@ varobj_value_get_print_value (struct value *value,
 								&opts);
 
 	      /* If we have string like output ...  */
-	      if (output != NULL)
+	      if (output != nullptr && output != Py_None)
 		{
 		  /* If this is a lazy string, extract it.  For lazy
 		     strings we always print as a string, so set
@@ -2245,6 +2225,13 @@ varobj_value_get_print_value (struct value *value,
 	      if (replacement)
 		value = replacement;
 	    }
+	  else
+	    {
+	      /* No to_string method, so if there is a 'children'
+		 method, return the default.  */
+	      if (PyObject_HasAttr (value_formatter, gdbpy_children_cst))
+		return "{...}";
+	    }
 	}
     }
 #endif