[PATCH 0/2] Add pretty-printer base class and new methods

[PATCH 0/2] Add pretty-printer base class and new methods

[Tom Tromey]

This adds a pretty-printer base class, to make it possible for gdb to
extend the pretty-printer API over time.

Then, a couple of new methods are added and the no-op array printer is
changed to use these, rather than the current approach of adding an
attribute and hoping it works ok.

Regression tested on x86-64 Fedora 36.

---
Tom Tromey (2):
      Introduce gdb.ValuePrinter
      Add two new pretty-printer methods

 gdb/NEWS                                      |  5 ++
 gdb/doc/python.texi                           | 68 ++++++++++++++++++++-------
 gdb/python/lib/gdb/dap/varref.py              | 22 ++++++---
 gdb/python/lib/gdb/printer/bound_registers.py |  9 ++--
 gdb/python/lib/gdb/printing.py                | 49 ++++++++++---------
 gdb/python/py-prettyprint.c                   | 65 +++++++++++++++++++++++++
 6 files changed, 168 insertions(+), 50 deletions(-)
---
base-commit: 14432bde076fdee66e6163993eadf592742935da
change-id: 20230911-pp-v2-9efa40abcc4e

Best regards,
-- 
Tom Tromey <tromey@adacore.com>

[PATCH 1/2] Introduce gdb.ValuePrinter

[Tom Tromey]

There was an earlier thread about adding new methods to
pretty-printers:

https://sourceware.org/pipermail/gdb-patches/2023-June/200503.html

We've known about the need for printer extensibility for a while, but
have been hampered by backward-compatibilty concerns: gdb never
documented that printers might acquire new methods, and so existing
printers may have attribute name clashes.

To solve this problem, this patch adds a new pretty-printer tag class
that signals to gdb that the printer follows new extensibility rules.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30816
---
 gdb/NEWS                                      |  5 +++
 gdb/doc/python.texi                           | 52 ++++++++++++++-------
 gdb/python/lib/gdb/printer/bound_registers.py |  9 ++--
 gdb/python/lib/gdb/printing.py                | 40 ++++++++---------
 gdb/python/py-prettyprint.c                   | 65 +++++++++++++++++++++++++++
 5 files changed, 131 insertions(+), 40 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 98ff00d5efc..cca86d82de8 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -286,6 +286,11 @@ info main
      might be array- or string-like, even if they do not have the
      corresponding type code.
 
+  ** gdb.ValuePrinter is a new class that can be used as the base
+     class for the result of applying a pretty-printer.  As a base
+     class, it signals to gdb that the printer may implement new
+     pretty-printer methods.
+
 *** Changes in GDB 13
 
 * MI version 1 is deprecated, and will be removed in GDB 14.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 5b13958aeaf..d09f5e03997 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -1722,6 +1722,22 @@ A pretty-printer is just an object that holds a value and implements a
 specific interface, defined here.  An example output is provided
 (@pxref{Pretty Printing}).
 
+Because @value{GDBN} did not document extensibility for
+pretty-printers, by default @value{GDBN} will assume that only the
+basic pretty-printer methods may be available.  The basic methods are
+marked as such, below.
+
+To allow extensibility, @value{GDBN} provides the
+@code{gdb.ValuePrinter} base class.  This class does not provide any
+attributes or behavior, but instead serves as a tag that can be
+recognized by @value{GDBN}.  For such printers, @value{GDBN} reserves
+all attributes of starting with a lower-case letter.  That is, in the
+future, @value{GDBN} may add a new method or attribute to the
+pretty-printer protocol, and @code{gdb.ValuePrinter}-based printers
+are expected to handle this gracefully.  A simple way to do this would
+be to use a leading underscore (or two, following the Python
+name-mangling scheme) to any attributes local to the implementation.
+
 @defun pretty_printer.children (self)
 @value{GDBN} will call this method on a pretty-printer to compute the
 children of the pretty-printer's value.
@@ -1732,8 +1748,8 @@ two elements.  The first element is the ``name'' of the child; the
 second element is the child's value.  The value can be any Python
 object which is convertible to a @value{GDBN} value.
 
-This method is optional.  If it does not exist, @value{GDBN} will act
-as though the value has no children.
+This is a basic method, and is optional.  If it does not exist,
+@value{GDBN} will act as though the value has no children.
 
 For efficiency, the @code{children} method should lazily compute its
 results.  This will let @value{GDBN} read as few elements as
@@ -1751,8 +1767,8 @@ formatting of a value.  The result will also be supplied to an MI
 consumer as a @samp{displayhint} attribute of the variable being
 printed.
 
-This method is optional.  If it does exist, this method must return a
-string or the special value @code{None}.
+This is a basic method, and is optional.  If it does exist, this
+method must return a string or the special value @code{None}.
 
 Some display hints are predefined by @value{GDBN}:
 
@@ -1784,6 +1800,8 @@ display rules.
 @value{GDBN} will call this method to display the string
 representation of the value passed to the object's constructor.
 
+This is a basic method, and is optional.
+
 When printing from the CLI, if the @code{to_string} method exists,
 then @value{GDBN} will prepend its result to the values returned by
 @code{children}.  Exactly how this formatting is done is dependent on
@@ -1904,17 +1922,19 @@ if the type is supported, and the printer itself.
 
 Here is an example showing how a @code{std::string} printer might be
 written.  @xref{Pretty Printing API}, for details on the API this class
-must provide.
+must provide.  Note that this example uses the @code{gdb.ValuePrinter}
+base class, and is careful to use a leading underscore for its local
+state.
 
 @smallexample
-class StdStringPrinter(object):
+class StdStringPrinter(gdb.ValuePrinter):
     "Print a std::string"
 
     def __init__(self, val):
-        self.val = val
+        self.__val = val
 
     def to_string(self):
-        return self.val['_M_dataplus']['_M_p']
+        return self.__val['_M_dataplus']['_M_p']
 
     def display_hint(self):
         return 'string'
@@ -2005,25 +2025,25 @@ struct bar @{ struct foo x, y; @};
 Here are the printers:
 
 @smallexample
-class fooPrinter:
+class fooPrinter(gdb.ValuePrinter):
     """Print a foo object."""
 
     def __init__(self, val):
-        self.val = val
+        self.__val = val
 
     def to_string(self):
-        return ("a=<" + str(self.val["a"]) +
-                "> b=<" + str(self.val["b"]) + ">")
+        return ("a=<" + str(self.__val["a"]) +
+                "> b=<" + str(self.__val["b"]) + ">")
 
-class barPrinter:
+class barPrinter(gdb.ValuePrinter):
     """Print a bar object."""
 
     def __init__(self, val):
-        self.val = val
+        self.__val = val
 
     def to_string(self):
-        return ("x=<" + str(self.val["x"]) +
-                "> y=<" + str(self.val["y"]) + ">")
+        return ("x=<" + str(self.__val["x"]) +
+                "> y=<" + str(self.__val["y"]) + ">")
 @end smallexample
 
 This example doesn't need a lookup function, that is handled by the
diff --git a/gdb/python/lib/gdb/printer/bound_registers.py b/gdb/python/lib/gdb/printer/bound_registers.py
index 08f30cbf286..b5298b9ee28 100644
--- a/gdb/python/lib/gdb/printer/bound_registers.py
+++ b/gdb/python/lib/gdb/printer/bound_registers.py
@@ -14,18 +14,19 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import gdb
 import gdb.printing
 
 
-class MpxBound128Printer:
+class MpxBound128Printer(gdb.ValuePrinter):
     """Adds size field to a mpx __gdb_builtin_type_bound128 type."""
 
     def __init__(self, val):
-        self.val = val
+        self.__val = val
 
     def to_string(self):
-        upper = self.val["ubound"]
-        lower = self.val["lbound"]
+        upper = self.__val["ubound"]
+        lower = self.__val["lbound"]
         size = upper - lower
         if size > -1:
             size = size + 1
diff --git a/gdb/python/lib/gdb/printing.py b/gdb/python/lib/gdb/printing.py
index 1a761a6aa57..80851590134 100644
--- a/gdb/python/lib/gdb/printing.py
+++ b/gdb/python/lib/gdb/printing.py
@@ -220,16 +220,16 @@ class RegexpCollectionPrettyPrinter(PrettyPrinter):
 
 # A helper class for printing enum types.  This class is instantiated
 # with a list of enumerators to print a particular Value.
-class _EnumInstance:
+class _EnumInstance(gdb.ValuePrinter):
     def __init__(self, enumerators, val):
-        self.enumerators = enumerators
-        self.val = val
+        self.__enumerators = enumerators
+        self.__val = val
 
     def to_string(self):
         flag_list = []
-        v = int(self.val)
+        v = int(self.__val)
         any_found = False
-        for e_name, e_value in self.enumerators:
+        for e_name, e_value in self.__enumerators:
             if v & e_value != 0:
                 flag_list.append(e_name)
                 v = v & ~e_value
@@ -237,7 +237,7 @@ class _EnumInstance:
         if not any_found or v != 0:
             # Leftover value.
             flag_list.append("<unknown: 0x%x>" % v)
-        return "0x%x [%s]" % (int(self.val), " | ".join(flag_list))
+        return "0x%x [%s]" % (int(self.__val), " | ".join(flag_list))
 
 
 class FlagEnumerationPrinter(PrettyPrinter):
@@ -270,21 +270,21 @@ class FlagEnumerationPrinter(PrettyPrinter):
             return None
 
 
-class NoOpScalarPrinter:
+class NoOpScalarPrinter(gdb.ValuePrinter):
     """A no-op pretty printer that wraps a scalar value."""
 
     def __init__(self, value):
-        self.value = value
+        self.__value = value
 
     def to_string(self):
-        return self.value.format_string(raw=True)
+        return self.__value.format_string(raw=True)
 
 
-class NoOpArrayPrinter:
+class NoOpArrayPrinter(gdb.ValuePrinter):
     """A no-op pretty printer that wraps an array value."""
 
     def __init__(self, ty, value):
-        self.value = value
+        self.__value = value
         (low, high) = ty.range()
         # In Ada, an array can have an index type that is a
         # non-contiguous enum.  In this case the indexing must be done
@@ -302,8 +302,8 @@ class NoOpArrayPrinter:
         # This is a convenience to the DAP code and perhaps other
         # users.
         self.num_children = high - low + 1
-        self.low = low
-        self.high = high
+        self.__low = low
+        self.__high = high
 
     def to_string(self):
         return ""
@@ -312,24 +312,24 @@ class NoOpArrayPrinter:
         return "array"
 
     def children(self):
-        for i in range(self.low, self.high + 1):
-            yield (i, self.value[i])
+        for i in range(self.__low, self.__high + 1):
+            yield (i, self.__value[i])
 
 
-class NoOpStructPrinter:
+class NoOpStructPrinter(gdb.ValuePrinter):
     """A no-op pretty printer that wraps a struct or union value."""
 
     def __init__(self, ty, value):
-        self.ty = ty
-        self.value = value
+        self.__ty = ty
+        self.__value = value
 
     def to_string(self):
         return ""
 
     def children(self):
-        for field in self.ty.fields():
+        for field in self.__ty.fields():
             if field.name is not None:
-                yield (field.name, self.value[field])
+                yield (field.name, self.__value[field])
 
 
 def make_visualizer(value):
diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index cccc94e319b..7a43d9d7881 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -27,6 +27,8 @@
 #include "python-internal.h"
 #include "cli/cli-style.h"
 
+extern PyTypeObject printer_object_type;
+
 /* Return type of print_string_repr.  */
 
 enum gdbpy_string_repr_result
@@ -779,3 +781,66 @@ gdbpy_get_print_options (value_print_options *opts)
   else
     get_user_print_options (opts);
 }
+
+/* A ValuePrinter is just a "tag", so it has no state other than that
+   required by Python.  */
+struct printer_object
+{
+  PyObject_HEAD
+};
+
+/* The ValuePrinter type object.  */
+PyTypeObject printer_object_type =
+{
+  PyVarObject_HEAD_INIT (NULL, 0)
+  "gdb.ValuePrinter",		/*tp_name*/
+  sizeof (printer_object),	  /*tp_basicsize*/
+  0,				  /*tp_itemsize*/
+  0,				  /*tp_dealloc*/
+  0,				  /*tp_print*/
+  0,				  /*tp_getattr*/
+  0,				  /*tp_setattr*/
+  0,				  /*tp_compare*/
+  0,				  /*tp_repr*/
+  0,				  /*tp_as_number*/
+  0,				  /*tp_as_sequence*/
+  0,				  /*tp_as_mapping*/
+  0,				  /*tp_hash*/
+  0,				  /*tp_call*/
+  0,				  /*tp_str*/
+  0,				  /*tp_getattro*/
+  0,				  /*tp_setattro*/
+  0,				  /*tp_as_buffer*/
+  Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /*tp_flags*/
+  "GDB value printer object",	  /* tp_doc */
+  0,				  /* tp_traverse */
+  0,				  /* tp_clear */
+  0,				  /* tp_richcompare */
+  0,				  /* tp_weaklistoffset */
+  0,				  /* tp_iter */
+  0,				  /* tp_iternext */
+  0,				  /* tp_methods */
+  0,				  /* tp_members */
+  0,				  /* tp_getset */
+  0,				  /* tp_base */
+  0,				  /* tp_dict */
+  0,				  /* tp_descr_get */
+  0,				  /* tp_descr_set */
+  0,				  /* tp_dictoffset */
+  0,				  /* tp_init */
+  0,				  /* tp_alloc */
+  PyType_GenericNew,		  /* tp_new */
+};
+
+/* Set up the ValuePrinter type.  */
+
+static int
+gdbpy_initialize_prettyprint ()
+{
+  if (PyType_Ready (&printer_object_type) < 0)
+    return -1;
+  return gdb_pymodule_addobject (gdb_module, "ValuePrinter",
+				 (PyObject *) &printer_object_type);
+}
+
+GDBPY_INITIALIZE_FILE (gdbpy_initialize_prettyprint);

-- 
2.40.1

[PATCH 2/2] Add two new pretty-printer methods

[Tom Tromey]

This adds two new pretty-printer methods, to support random access to
children.  The methods are implemented for the no-op array printer,
and DAP is updated to use this.
---
 gdb/doc/python.texi              | 16 ++++++++++++++++
 gdb/python/lib/gdb/dap/varref.py | 22 +++++++++++++++-------
 gdb/python/lib/gdb/printing.py   |  9 ++++++---
 3 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index d09f5e03997..79438baf477 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -1828,6 +1828,22 @@ are peformed in this method and nothing is printed.
 If the result is not one of these types, an exception is raised.
 @end defun
 
+@defun pretty_printer.num_children ()
+This is not a basic method, so @value{GDBN} will only ever call it for
+objects derived from @code{gdb.ValuePrinter}.
+
+If available, this method should return the number of children.
+@code{None} may be returned if the number can't readily be computed.
+@end defun
+
+@defun pretty_printer.child (n)
+This is not a basic method, so @value{GDBN} will only ever call it for
+objects derived from @code{gdb.ValuePrinter}.
+
+If available, this method should return the child value indicated by
+@var{n}.  Indices start at zero.
+@end defun
+
 @value{GDBN} provides a function which can be used to look up the
 default pretty-printer for a @code{gdb.Value}:
 
diff --git a/gdb/python/lib/gdb/dap/varref.py b/gdb/python/lib/gdb/dap/varref.py
index c1e5ba8686d..768d768c7b3 100644
--- a/gdb/python/lib/gdb/dap/varref.py
+++ b/gdb/python/lib/gdb/dap/varref.py
@@ -151,9 +151,9 @@ class VariableReference(BaseReference):
             # This discards all laziness.  This could be improved
             # slightly by lazily evaluating children, but because this
             # code also generally needs to know the number of
-            # children, it probably wouldn't help much.  A real fix
-            # would require an update to gdb's pretty-printer protocol
-            # (though of course that is probably also inadvisable).
+            # children, it probably wouldn't help much.  Note that
+            # this is only needed with legacy (non-ValuePrinter)
+            # printers.
             self.child_cache = list(self.printer.children())
         return self.child_cache
 
@@ -161,9 +161,12 @@ class VariableReference(BaseReference):
         if self.count is None:
             return None
         if self.count == -1:
-            if hasattr(self.printer, "num_children"):
-                num_children = self.printer.num_children
-            else:
+            num_children = None
+            if isinstance(self.printer, gdb.ValuePrinter) and hasattr(
+                self.printer, "num_children"
+            ):
+                num_children = self.printer.num_children()
+            if num_children is None:
                 num_children = len(self.cache_children())
             self.count = num_children
         return self.count
@@ -193,7 +196,12 @@ class VariableReference(BaseReference):
 
     @in_gdb_thread
     def fetch_one_child(self, idx):
-        return self.cache_children()[idx]
+        if isinstance(self.printer, gdb.ValuePrinter) and hasattr(
+                self.printer, "child"
+        ):
+            return self.printer.child(idx)
+        else:
+            return self.cache_children()[idx]
 
 
 @in_gdb_thread
diff --git a/gdb/python/lib/gdb/printing.py b/gdb/python/lib/gdb/printing.py
index 80851590134..4982abc3f77 100644
--- a/gdb/python/lib/gdb/printing.py
+++ b/gdb/python/lib/gdb/printing.py
@@ -299,9 +299,6 @@ class NoOpArrayPrinter(gdb.ValuePrinter):
             e_values = itertools.takewhile(lambda x: x.enumval <= high, e_values)
             low = 0
             high = len(list(e_values)) - 1
-        # This is a convenience to the DAP code and perhaps other
-        # users.
-        self.num_children = high - low + 1
         self.__low = low
         self.__high = high
 
@@ -311,6 +308,12 @@ class NoOpArrayPrinter(gdb.ValuePrinter):
     def display_hint(self):
         return "array"
 
+    def num_children(self):
+        return self.__high - self.__low + 1
+
+    def child(self, i):
+        return (self.__low + i, self.__value[self.__low + i])
+
     def children(self):
         for i in range(self.__low, self.__high + 1):
             yield (i, self.__value[i])

-- 
2.40.1

Re: [PATCH 2/2] Add two new pretty-printer methods

[Eli Zaretskii]

> Date: Mon, 11 Sep 2023 11:28:26 -0600
> From: Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
> 
> This adds two new pretty-printer methods, to support random access to
> children.  The methods are implemented for the no-op array printer,
> and DAP is updated to use this.
> ---
>  gdb/doc/python.texi              | 16 ++++++++++++++++
>  gdb/python/lib/gdb/dap/varref.py | 22 +++++++++++++++-------
>  gdb/python/lib/gdb/printing.py   |  9 ++++++---
>  3 files changed, 37 insertions(+), 10 deletions(-)
> 
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index d09f5e03997..79438baf477 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -1828,6 +1828,22 @@ are peformed in this method and nothing is printed.
>  If the result is not one of these types, an exception is raised.
>  @end defun
>  
> +@defun pretty_printer.num_children ()
> +This is not a basic method, so @value{GDBN} will only ever call it for
> +objects derived from @code{gdb.ValuePrinter}.
> +
> +If available, this method should return the number of children.
> +@code{None} may be returned if the number can't readily be computed.
> +@end defun
> +
> +@defun pretty_printer.child (n)
> +This is not a basic method, so @value{GDBN} will only ever call it for
> +objects derived from @code{gdb.ValuePrinter}.
> +
> +If available, this method should return the child value indicated by
> +@var{n}.  Indices start at zero.
> +@end defun
> +
>  @value{GDBN} provides a function which can be used to look up the
>  default pretty-printer for a @code{gdb.Value}:

The python.texi part is OK, thanks.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

Re: [PATCH 1/2] Introduce gdb.ValuePrinter

[Eli Zaretskii]

> Date: Mon, 11 Sep 2023 11:28:25 -0600
> From: Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
> 
> There was an earlier thread about adding new methods to
> pretty-printers:

Thanks.

> diff --git a/gdb/NEWS b/gdb/NEWS
> index 98ff00d5efc..cca86d82de8 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -286,6 +286,11 @@ info main
>       might be array- or string-like, even if they do not have the
>       corresponding type code.
>  
> +  ** gdb.ValuePrinter is a new class that can be used as the base
> +     class for the result of applying a pretty-printer.  As a base
> +     class, it signals to gdb that the printer may implement new
> +     pretty-printer methods.
> +
>  *** Changes in GDB 13

This part is OK.

> +To allow extensibility, @value{GDBN} provides the
> +@code{gdb.ValuePrinter} base class.  This class does not provide any
> +attributes or behavior, but instead serves as a tag that can be
> +recognized by @value{GDBN}.  For such printers, @value{GDBN} reserves
> +all attributes of starting with a lower-case letter.  That is, in the

I think that "of" should be removed.

The python.texi part is OK otherwise.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

Re: [PATCH 1/2] Introduce gdb.ValuePrinter

[Tom Tromey]

>> +To allow extensibility, @value{GDBN} provides the
>> +@code{gdb.ValuePrinter} base class.  This class does not provide any
>> +attributes or behavior, but instead serves as a tag that can be
>> +recognized by @value{GDBN}.  For such printers, @value{GDBN} reserves
>> +all attributes of starting with a lower-case letter.  That is, in the

Eli> I think that "of" should be removed.

I fixed this.

Tom

Re: [PATCH 0/2] Add pretty-printer base class and new methods

[Tom Tromey]

>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> This adds a pretty-printer base class, to make it possible for gdb to
Tom> extend the pretty-printer API over time.

Tom> Then, a couple of new methods are added and the no-op array printer is
Tom> changed to use these, rather than the current approach of adding an
Tom> attribute and hoping it works ok.

I've rebased this and applied the obvious fixes to NoOpPointerReferencePrinter.
I'm going to check it in now.

Tom