public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Add __repr__() implementation to a few Python types
       [not found] <3Cc9d2dc49-45c7-d6b9-c567-4ec78dd870a0>
@ 2023-01-11  0:35 ` Matheus Branco Borella
  2023-01-18 17:05   ` Bruno Larsen
  2023-01-18 18:02   ` Andrew Burgess
  0 siblings, 2 replies; 15+ messages in thread
From: Matheus Branco Borella @ 2023-01-11  0:35 UTC (permalink / raw)
  To: gdb-patches; +Cc: Matheus Branco Borella

Only a few types in the Python API currently have __repr__() implementations.
This patch adds a few more of them. specifically: it adds __repr__()
implementations to gdb.Symbol, gdb.Architecture, gdb.Block, gdb.Breakpoint,
and gdb.Type.

This makes it easier to play around the GDB Python API in the Python interpreter
session invoked with the 'pi' command in GDB, giving more easily accessible tipe
information to users.

An example of how this would look like:
```
(gdb) pi
>> gdb.lookup_type("char")
<gdb.Type code=TYPE_CODE_INT name=char>
>> gdb.lookup_global_symbol("main")
<gdb.Symbol print_name=main>
```

One thing to note about this patch is that it makes use of u8 string literals,
so as to make sure we meet python's expectations of strings passed to it using
PyUnicode_FromFormat being encoded in utf8. This should remove the chance of
odd compilation environments spitting out strings Python would consider invalid
for the function we're calling.
---
 gdb/python/py-arch.c                       | 18 +++++++-
 gdb/python/py-block.c                      | 30 ++++++++++++-
 gdb/python/py-breakpoint.c                 | 49 +++++++++++++++++++++-
 gdb/python/py-symbol.c                     | 16 ++++++-
 gdb/python/py-type.c                       | 31 +++++++++++++-
 gdb/testsuite/gdb.python/py-arch.exp       |  4 ++
 gdb/testsuite/gdb.python/py-block.exp      |  4 +-
 gdb/testsuite/gdb.python/py-breakpoint.exp | 26 +++++++-----
 gdb/testsuite/gdb.python/py-symbol.exp     |  1 +
 gdb/testsuite/gdb.python/py-type.exp       |  4 ++
 10 files changed, 165 insertions(+), 18 deletions(-)

diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index cf0978560f9..5384a0d0d0c 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -319,6 +319,22 @@ archpy_integer_type (PyObject *self, PyObject *args, PyObject *kw)
   return type_to_type_object (type);
 }
 
+/* __repr__ implementation for gdb.Architecture.  */
+
+static PyObject *
+archpy_repr (PyObject *self)
+{
+  const auto gdbarch = arch_object_to_gdbarch (self);
+  if (gdbarch == nullptr)
+    return PyUnicode_FromFormat
+      ("<gdb.Architecture (invalid)>");
+
+  return PyUnicode_FromFormat
+    ("<gdb.Architecture arch_name=%s printable_name=%s>",
+     gdbarch_bfd_arch_info (gdbarch)->arch_name,
+     gdbarch_bfd_arch_info (gdbarch)->printable_name);
+}
+
 /* Implementation of gdb.architecture_names().  Return a list of all the
    BFD architecture names that GDB understands.  */
 
@@ -391,7 +407,7 @@ PyTypeObject arch_object_type = {
   0,                                  /* tp_getattr */
   0,                                  /* tp_setattr */
   0,                                  /* tp_compare */
-  0,                                  /* tp_repr */
+  archpy_repr,                        /* tp_repr */
   0,                                  /* tp_as_number */
   0,                                  /* tp_as_sequence */
   0,                                  /* tp_as_mapping */
diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index b9aea3aca69..1b8433d41e7 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -23,6 +23,7 @@
 #include "symtab.h"
 #include "python-internal.h"
 #include "objfiles.h"
+#include <sstream>
 
 struct block_object {
   PyObject_HEAD
@@ -424,6 +425,33 @@ blpy_iter_is_valid (PyObject *self, PyObject *args)
   Py_RETURN_TRUE;
 }
 
+/* __repr__ implementation for gdb.Block.  */
+
+static PyObject *
+blpy_repr (PyObject *self)
+{
+  const auto block = block_object_to_block (self);
+  if (block == nullptr)
+    return PyUnicode_FromFormat("<gdb.Block (invalid)>");
+
+  const auto name = block->function () ? block->function ()->print_name () : "<anonymous>";
+
+  block_iterator iter;
+  block_iterator_first (block, &iter);
+
+  std::stringstream ss;
+  const struct symbol *symbol;
+  while ((symbol = block_iterator_next (&iter)) != nullptr)
+  {
+    ss << std::endl;
+    ss << symbol->print_name () << ",";
+  }
+  if(!ss.str ().empty ())
+    ss << std::endl;
+
+  return PyUnicode_FromFormat("<gdb.Block %s {%s}>", name, ss.str ().c_str ());
+}
+
 int
 gdbpy_initialize_blocks (void)
 {
@@ -486,7 +514,7 @@ PyTypeObject block_object_type = {
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  blpy_repr,                     /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   &block_object_as_mapping,	  /*tp_as_mapping*/
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index de7b9f4266b..ce307f7be21 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -33,6 +33,7 @@
 #include "location.h"
 #include "py-event.h"
 #include "linespec.h"
+#include <sstream>
 
 extern PyTypeObject breakpoint_location_object_type
     CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("breakpoint_location_object");
@@ -967,6 +968,23 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
   return 0;
 }
 
+/* __repr__ implementation for gdb.Breakpoint.  */
+
+static PyObject *
+bppy_repr(PyObject *self)
+{
+  const auto bp = (struct gdbpy_breakpoint_object*) self;
+  if (bp->bp == nullptr)
+    return PyUnicode_FromFormat ("<gdb.Breakpoint (invalid)>");
+
+  return PyUnicode_FromFormat
+    ("<gdb.Breakpoint number=%d thread=%d hits=%d enable_count=%d>",
+     bp->bp->number,
+     bp->bp->thread,
+     bp->bp->hit_count,
+     bp->bp->enable_count);
+}
+
 /* Append to LIST the breakpoint Python object associated to B.
 
    Return true on success.  Return false on failure, with the Python error
@@ -1389,7 +1407,7 @@ PyTypeObject breakpoint_object_type =
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  bppy_repr,                     /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   0,				  /*tp_as_mapping*/
@@ -1604,6 +1622,33 @@ bplocpy_dealloc (PyObject *py_self)
   Py_TYPE (py_self)->tp_free (py_self);
 }
 
+/* __repr__ implementation for gdb.BreakpointLocation.  */
+
+static PyObject *
+bplocpy_repr (PyObject *py_self)
+{
+  const auto self = (gdbpy_breakpoint_location_object *) py_self;
+  if (self->owner == nullptr || self->owner->bp == nullptr || self->owner->bp != self->bp_loc->owner)
+    return PyUnicode_FromFormat ("<gdb.BreakpointLocation (invalid)>");
+
+  const auto enabled = self->bp_loc->enabled ? "enabled" : "disabled";
+
+  std::stringstream ss;
+  ss << std::endl << enabled << std::endl;
+  ss << "requested_address=0x" << std::hex << self->bp_loc->requested_address << " ";
+  ss << "address=0x" << self->bp_loc->address << " " << std::dec << std::endl;
+  if (self->bp_loc->symtab != nullptr)
+  {
+    ss << self->bp_loc->symtab->filename << ":" << self->bp_loc->line_number << " " << std::endl;
+  }
+
+  const auto fn_name = self->bp_loc->function_name.get ();
+  if (fn_name != nullptr)
+    ss << "in " << fn_name << " " << std::endl;
+
+  return PyUnicode_FromFormat ("<gdb.BreakpointLocation %s>", ss.str ().c_str ());
+}
+
 /* Attribute get/set Python definitions. */
 
 static gdb_PyGetSetDef bp_location_object_getset[] = {
@@ -1635,7 +1680,7 @@ PyTypeObject breakpoint_location_object_type =
   0,					/*tp_getattr*/
   0,					/*tp_setattr*/
   0,					/*tp_compare*/
-  0,					/*tp_repr*/
+  bplocpy_repr,                        /*tp_repr*/
   0,					/*tp_as_number*/
   0,					/*tp_as_sequence*/
   0,					/*tp_as_mapping*/
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index 93c86964f3e..5a8149bbe66 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -375,6 +375,20 @@ sympy_dealloc (PyObject *obj)
   Py_TYPE (obj)->tp_free (obj);
 }
 
+/* __repr__ implementation for gdb.Symbol.  */
+
+static PyObject *
+sympy_repr (PyObject *self)
+{
+  const auto symbol = symbol_object_to_symbol (self);
+  if (symbol == nullptr)
+    return PyUnicode_FromFormat ("<gdb.Symbol (invalid)>");
+
+  return PyUnicode_FromFormat
+    ("<gdb.Symbol print_name=%s>",
+     symbol->print_name ());
+}
+
 /* Implementation of
    gdb.lookup_symbol (name [, block] [, domain]) -> (symbol, is_field_of_this)
    A tuple with 2 elements is always returned.  The first is the symbol
@@ -732,7 +746,7 @@ PyTypeObject symbol_object_type = {
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  sympy_repr,                    /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   0,				  /*tp_as_mapping*/
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 928efacfe8a..abe127eca76 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -442,6 +442,7 @@ typy_is_signed (PyObject *self, void *closure)
     Py_RETURN_TRUE;
 }
 
+
 /* Return the type, stripped of typedefs. */
 static PyObject *
 typy_strip_typedefs (PyObject *self, PyObject *args)
@@ -1026,6 +1027,34 @@ typy_template_argument (PyObject *self, PyObject *args)
   return value_to_value_object (val);
 }
 
+/* __repr__ implementation for gdb.Type.  */
+
+static PyObject *
+typy_repr (PyObject *self)
+{
+  const auto type = type_object_to_type (self);
+  if (type == nullptr)
+    return PyUnicode_FromFormat ("<gdb.Type (invalid)>");
+
+  const char *code = pyty_codes[type->code ()].name;
+  string_file type_name;
+  try
+    {
+      current_language->print_type (type, "",
+				    &type_name, -1, 0,
+				    &type_print_raw_options);
+    }
+  catch (const gdb_exception &except)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+    }
+  auto py_typename = PyUnicode_Decode
+    (type_name.c_str (), type_name.size (),
+		 host_charset (), NULL);
+	
+	return PyUnicode_FromFormat ("<gdb.Type code=%s name=%U>", code, py_typename);
+}
+
 static PyObject *
 typy_str (PyObject *self)
 {
@@ -1612,7 +1641,7 @@ PyTypeObject type_object_type =
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  typy_repr,                     /*tp_repr*/
   &type_object_as_number,	  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   &typy_mapping,		  /*tp_as_mapping*/
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
index 1fbbc47c872..a60b4a25cbb 100644
--- a/gdb/testsuite/gdb.python/py-arch.exp
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -29,6 +29,8 @@ if ![runto_main] {
 # Test python/15461.  Invalid architectures should not trigger an
 # internal GDB assert.
 gdb_py_test_silent_cmd "python empty = gdb.Architecture()" "get empty arch" 0
+gdb_test "python print(repr (empty))" "<gdb\\.Architecture \\(invalid\\)>" \
+    "Test empty achitecture __repr__ does not trigger an assert"
 gdb_test "python print(empty.name())" ".*Architecture is invalid.*" \
     "Test empty architecture.name does not trigger an assert"
 gdb_test "python print(empty.disassemble())" ".*Architecture is invalid.*" \
@@ -46,6 +48,8 @@ gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
 gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(gdb.Value(pc))" \
   "disassemble no end no count" 0
 
+gdb_test "python print (repr (arch))" "<gdb.Architecture arch_name=.* printable_name=.*>" "test __repr__ for architecture"
+
 gdb_test "python print (len(insn_list1))" "1" "test number of instructions 1"
 gdb_test "python print (len(insn_list2))" "1" "test number of instructions 2"
 gdb_test "python print (len(insn_list3))" "1" "test number of instructions 3"
diff --git a/gdb/testsuite/gdb.python/py-block.exp b/gdb/testsuite/gdb.python/py-block.exp
index 0a88aec56a0..5e3d1c72d5e 100644
--- a/gdb/testsuite/gdb.python/py-block.exp
+++ b/gdb/testsuite/gdb.python/py-block.exp
@@ -39,7 +39,7 @@ gdb_continue_to_breakpoint "Block break here."
 gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
 gdb_py_test_silent_cmd "python block = frame.block()" \
     "Get block, initial innermost block" 0
-gdb_test "python print (block)" "<gdb.Block object at $hex>" "check block not None"
+gdb_test "python print (block)" "<gdb.Block .* \{.*\}>" "check block not None"
 gdb_test "python print (block.function)" "None" "first anonymous block"
 gdb_test "python print (block.start)" "${decimal}" "check start not None"
 gdb_test "python print (block.end)" "${decimal}" "check end not None"
@@ -73,7 +73,7 @@ gdb_test "python print (block.function)" "block_func" \
 gdb_test "up" ".*"
 gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
 gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
-gdb_test "python print (block)" "<gdb.Block object at $hex>" \
+gdb_test "python print (repr (block))" "<gdb.Block .* \{.*\}>" \
          "Check Frame 2's block not None"
 gdb_test "python print (block.function)" "main" "main block"
 
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index e36e87dc291..0c904a12c90 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -50,11 +50,14 @@ proc_with_prefix test_bkpt_basic { } {
 	return 0
     }
 
+    set num_exp "-?\[0-9\]+"
+    set repr_pattern "<gdb.Breakpoint number=$num_exp thread=$num_exp hits=$num_exp enable_count=$num_exp>"
+
     # Now there should be one breakpoint: main.
     gdb_py_test_silent_cmd "python blist = gdb.breakpoints()" \
 	"Get Breakpoint List" 0
-    gdb_test "python print (blist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check obj exists @main"
+    gdb_test "python print (repr (blist\[0\]))" \
+	"$repr_pattern" "Check obj exists @main"
     gdb_test "python print (blist\[0\].location)" \
 	"main." "Check breakpoint location @main"
     gdb_test "python print (blist\[0\].pending)" "False" \
@@ -71,12 +74,12 @@ proc_with_prefix test_bkpt_basic { } {
 	"Get Breakpoint List" 0
     gdb_test "python print (len(blist))" \
 	"2" "Check for two breakpoints"
-    gdb_test "python print (blist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check obj exists @main 2"
+    gdb_test "python print (repr (blist\[0\]))" \
+	"$repr_pattern" "Check obj exists @main 2"
     gdb_test "python print (blist\[0\].location)" \
 	"main." "Check breakpoint location @main 2"
-    gdb_test "python print (blist\[1\])" \
-	"<gdb.Breakpoint object at $hex>" "Check obj exists @mult_line"
+    gdb_test "python print (repr (blist\[1\]))" \
+	"$repr_pattern" "Check obj exists @mult_line"
 
     gdb_test "python print (blist\[1\].location)" \
 	"py-breakpoint\.c:${mult_line}*" \
@@ -224,14 +227,17 @@ proc_with_prefix test_bkpt_invisible { } {
 	return 0
     }
 
+    set num_exp "-?\[0-9\]+"
+    set repr_pattern "<gdb.Breakpoint number=$num_exp thread=$num_exp hits=$num_exp enable_count=$num_exp>"
+
     delete_breakpoints
     set ibp_location [gdb_get_line_number "Break at multiply."]
     gdb_py_test_silent_cmd  "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=False)" \
 	"Set invisible breakpoint" 0
     gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
 	"Get Breakpoint List" 0
-    gdb_test "python print (ilist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 1"
+    gdb_test "python print (repr (ilist\[0\]))" \
+	"$repr_pattern" "Check invisible bp obj exists 1"
     gdb_test "python print (ilist\[0\].location)" \
 	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 1"
     gdb_test "python print (ilist\[0\].visible)" \
@@ -243,8 +249,8 @@ proc_with_prefix test_bkpt_invisible { } {
 	"Set invisible breakpoint" 0
     gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
 	"Get Breakpoint List" 0
-    gdb_test "python print (ilist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 2"
+    gdb_test "python print (repr (ilist\[0\]))" \
+	"$repr_pattern" "Check invisible bp obj exists 2"
     gdb_test "python print (ilist\[0\].location)" \
 	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 2"
     gdb_test "python print (ilist\[0\].visible)" \
diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
index ad06b07c2c6..e0baed9b6d4 100644
--- a/gdb/testsuite/gdb.python/py-symbol.exp
+++ b/gdb/testsuite/gdb.python/py-symbol.exp
@@ -44,6 +44,7 @@ clean_restart ${binfile}
 # point where we don't have a current frame, and we don't want to
 # require one.
 gdb_py_test_silent_cmd "python main_func = gdb.lookup_global_symbol(\"main\")" "Lookup main" 1
+gdb_test "python print (repr (main_func))" "<gdb.Symbol print_name=.*>" "test main_func.__repr__"
 gdb_test "python print (main_func.is_function)" "True" "test main_func.is_function"
 gdb_test "python print (gdb.lookup_global_symbol(\"junk\"))" "None" "test lookup_global_symbol(\"junk\")"
 
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 594c9749d8e..95cdfa54a6e 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -393,3 +393,7 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
       test_type_equality
   }
 }
+
+# Test __repr__()
+gdb_test "python print (repr (gdb.lookup_type ('char')))" \
+      "<gdb.Type code=TYPE_CODE_INT name=char>" "test __repr__()"
-- 
2.37.3.windows.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Add __repr__() implementation to a few Python types
  2023-01-11  0:35 ` [PATCH] Add __repr__() implementation to a few Python types Matheus Branco Borella
@ 2023-01-18 17:05   ` Bruno Larsen
  2023-01-18 18:02   ` Andrew Burgess
  1 sibling, 0 replies; 15+ messages in thread
From: Bruno Larsen @ 2023-01-18 17:05 UTC (permalink / raw)
  To: Matheus Branco Borella, gdb-patches

On 11/01/2023 01:35, Matheus Branco Borella via Gdb-patches wrote:
> Only a few types in the Python API currently have __repr__() implementations.
> This patch adds a few more of them. specifically: it adds __repr__()
> implementations to gdb.Symbol, gdb.Architecture, gdb.Block, gdb.Breakpoint,
> and gdb.Type.
>
> This makes it easier to play around the GDB Python API in the Python interpreter
> session invoked with the 'pi' command in GDB, giving more easily accessible tipe
> information to users.
>
> An example of how this would look like:
> ```
> (gdb) pi
>>> gdb.lookup_type("char")
> <gdb.Type code=TYPE_CODE_INT name=char>
>>> gdb.lookup_global_symbol("main")
> <gdb.Symbol print_name=main>
> ```
>
> One thing to note about this patch is that it makes use of u8 string literals,
> so as to make sure we meet python's expectations of strings passed to it using
> PyUnicode_FromFormat being encoded in utf8. This should remove the chance of
> odd compilation environments spitting out strings Python would consider invalid
> for the function we're calling.

Thanks for working on this. I am not very familiar with the python side 
of GDB code, so I'll mostly comment on style. Nits inlined.

Also, while applying I get this:

Applying: Add __repr__() implementation to a few Python types
.git/rebase-apply/patch:267: trailing whitespace.

I have tested and see no regressions on my x86 machine.

Tested-By: Bruno Larsen <blarsen@redhat.com>

> ---
>   gdb/python/py-arch.c                       | 18 +++++++-
>   gdb/python/py-block.c                      | 30 ++++++++++++-
>   gdb/python/py-breakpoint.c                 | 49 +++++++++++++++++++++-
>   gdb/python/py-symbol.c                     | 16 ++++++-
>   gdb/python/py-type.c                       | 31 +++++++++++++-
>   gdb/testsuite/gdb.python/py-arch.exp       |  4 ++
>   gdb/testsuite/gdb.python/py-block.exp      |  4 +-
>   gdb/testsuite/gdb.python/py-breakpoint.exp | 26 +++++++-----
>   gdb/testsuite/gdb.python/py-symbol.exp     |  1 +
>   gdb/testsuite/gdb.python/py-type.exp       |  4 ++
>   10 files changed, 165 insertions(+), 18 deletions(-)
>
> diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
> index cf0978560f9..5384a0d0d0c 100644
> --- a/gdb/python/py-arch.c
> +++ b/gdb/python/py-arch.c
> @@ -319,6 +319,22 @@ archpy_integer_type (PyObject *self, PyObject *args, PyObject *kw)
>     return type_to_type_object (type);
>   }
>   
> +/* __repr__ implementation for gdb.Architecture.  */
> +
> +static PyObject *
> +archpy_repr (PyObject *self)
> +{
> +  const auto gdbarch = arch_object_to_gdbarch (self);
> +  if (gdbarch == nullptr)
> +    return PyUnicode_FromFormat
> +      ("<gdb.Architecture (invalid)>");
> +
> +  return PyUnicode_FromFormat
> +    ("<gdb.Architecture arch_name=%s printable_name=%s>",
> +     gdbarch_bfd_arch_info (gdbarch)->arch_name,
> +     gdbarch_bfd_arch_info (gdbarch)->printable_name);
> +}
> +
>   /* Implementation of gdb.architecture_names().  Return a list of all the
>      BFD architecture names that GDB understands.  */
>   
> @@ -391,7 +407,7 @@ PyTypeObject arch_object_type = {
>     0,                                  /* tp_getattr */
>     0,                                  /* tp_setattr */
>     0,                                  /* tp_compare */
> -  0,                                  /* tp_repr */
> +  archpy_repr,                        /* tp_repr */
>     0,                                  /* tp_as_number */
>     0,                                  /* tp_as_sequence */
>     0,                                  /* tp_as_mapping */
> diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
> index b9aea3aca69..1b8433d41e7 100644
> --- a/gdb/python/py-block.c
> +++ b/gdb/python/py-block.c
> @@ -23,6 +23,7 @@
>   #include "symtab.h"
>   #include "python-internal.h"
>   #include "objfiles.h"
> +#include <sstream>
>   
>   struct block_object {
>     PyObject_HEAD
> @@ -424,6 +425,33 @@ blpy_iter_is_valid (PyObject *self, PyObject *args)
>     Py_RETURN_TRUE;
>   }
>   
> +/* __repr__ implementation for gdb.Block.  */
> +
> +static PyObject *
> +blpy_repr (PyObject *self)
> +{
> +  const auto block = block_object_to_block (self);
> +  if (block == nullptr)
> +    return PyUnicode_FromFormat("<gdb.Block (invalid)>");
missing space before (
> +
> +  const auto name = block->function () ? block->function ()->print_name () : "<anonymous>";
> +
> +  block_iterator iter;
> +  block_iterator_first (block, &iter);
> +
> +  std::stringstream ss;
> +  const struct symbol *symbol;
> +  while ((symbol = block_iterator_next (&iter)) != nullptr)
> +  {
> +    ss << std::endl;
> +    ss << symbol->print_name () << ",";
> +  }
> +  if(!ss.str ().empty ())
> +    ss << std::endl;
> +
> +  return PyUnicode_FromFormat("<gdb.Block %s {%s}>", name, ss.str ().c_str ());
and here
> +}
> +
>   int
>   gdbpy_initialize_blocks (void)
>   {
> @@ -486,7 +514,7 @@ PyTypeObject block_object_type = {
>     0,				  /*tp_getattr*/
>     0,				  /*tp_setattr*/
>     0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  blpy_repr,                     /*tp_repr*/
>     0,				  /*tp_as_number*/
>     0,				  /*tp_as_sequence*/
>     &block_object_as_mapping,	  /*tp_as_mapping*/
> diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
> index de7b9f4266b..ce307f7be21 100644
> --- a/gdb/python/py-breakpoint.c
> +++ b/gdb/python/py-breakpoint.c
> @@ -33,6 +33,7 @@
>   #include "location.h"
>   #include "py-event.h"
>   #include "linespec.h"
> +#include <sstream>
>   
>   extern PyTypeObject breakpoint_location_object_type
>       CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("breakpoint_location_object");
> @@ -967,6 +968,23 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>     return 0;
>   }
>   
> +/* __repr__ implementation for gdb.Breakpoint.  */
> +
> +static PyObject *
> +bppy_repr(PyObject *self)
here too
> +{
> +  const auto bp = (struct gdbpy_breakpoint_object*) self;
> +  if (bp->bp == nullptr)
> +    return PyUnicode_FromFormat ("<gdb.Breakpoint (invalid)>");
> +
> +  return PyUnicode_FromFormat
> +    ("<gdb.Breakpoint number=%d thread=%d hits=%d enable_count=%d>",
> +     bp->bp->number,
> +     bp->bp->thread,
> +     bp->bp->hit_count,
> +     bp->bp->enable_count);
I'm not sure if this is a "me" thing, but I try to keep line count as 
low as possible, and only break when reaching the 72 character soft limit.
> +}
> +
>   /* Append to LIST the breakpoint Python object associated to B.
>   
>      Return true on success.  Return false on failure, with the Python error
> @@ -1389,7 +1407,7 @@ PyTypeObject breakpoint_object_type =
>     0,				  /*tp_getattr*/
>     0,				  /*tp_setattr*/
>     0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  bppy_repr,                     /*tp_repr*/
>     0,				  /*tp_as_number*/
>     0,				  /*tp_as_sequence*/
>     0,				  /*tp_as_mapping*/
> @@ -1604,6 +1622,33 @@ bplocpy_dealloc (PyObject *py_self)
>     Py_TYPE (py_self)->tp_free (py_self);
>   }
>   
> +/* __repr__ implementation for gdb.BreakpointLocation.  */
> +
> +static PyObject *
> +bplocpy_repr (PyObject *py_self)
> +{
> +  const auto self = (gdbpy_breakpoint_location_object *) py_self;
> +  if (self->owner == nullptr || self->owner->bp == nullptr || self->owner->bp != self->bp_loc->owner)
> +    return PyUnicode_FromFormat ("<gdb.BreakpointLocation (invalid)>");
> +
> +  const auto enabled = self->bp_loc->enabled ? "enabled" : "disabled";
> +
> +  std::stringstream ss;
> +  ss << std::endl << enabled << std::endl;
> +  ss << "requested_address=0x" << std::hex << self->bp_loc->requested_address << " ";
> +  ss << "address=0x" << self->bp_loc->address << " " << std::dec << std::endl;
> +  if (self->bp_loc->symtab != nullptr)
> +  {
> +    ss << self->bp_loc->symtab->filename << ":" << self->bp_loc->line_number << " " << std::endl;
> +  }
> +
> +  const auto fn_name = self->bp_loc->function_name.get ();
> +  if (fn_name != nullptr)
> +    ss << "in " << fn_name << " " << std::endl;
> +
> +  return PyUnicode_FromFormat ("<gdb.BreakpointLocation %s>", ss.str ().c_str ());
> +}
> +
>   /* Attribute get/set Python definitions. */
>   
>   static gdb_PyGetSetDef bp_location_object_getset[] = {
> @@ -1635,7 +1680,7 @@ PyTypeObject breakpoint_location_object_type =
>     0,					/*tp_getattr*/
>     0,					/*tp_setattr*/
>     0,					/*tp_compare*/
> -  0,					/*tp_repr*/
> +  bplocpy_repr,                        /*tp_repr*/
>     0,					/*tp_as_number*/
>     0,					/*tp_as_sequence*/
>     0,					/*tp_as_mapping*/
> diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> index 93c86964f3e..5a8149bbe66 100644
> --- a/gdb/python/py-symbol.c
> +++ b/gdb/python/py-symbol.c
> @@ -375,6 +375,20 @@ sympy_dealloc (PyObject *obj)
>     Py_TYPE (obj)->tp_free (obj);
>   }
>   
> +/* __repr__ implementation for gdb.Symbol.  */
> +
> +static PyObject *
> +sympy_repr (PyObject *self)
> +{
> +  const auto symbol = symbol_object_to_symbol (self);
> +  if (symbol == nullptr)
> +    return PyUnicode_FromFormat ("<gdb.Symbol (invalid)>");
> +
> +  return PyUnicode_FromFormat
> +    ("<gdb.Symbol print_name=%s>",
> +     symbol->print_name ());
> +}
> +
>   /* Implementation of
>      gdb.lookup_symbol (name [, block] [, domain]) -> (symbol, is_field_of_this)
>      A tuple with 2 elements is always returned.  The first is the symbol
> @@ -732,7 +746,7 @@ PyTypeObject symbol_object_type = {
>     0,				  /*tp_getattr*/
>     0,				  /*tp_setattr*/
>     0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  sympy_repr,                    /*tp_repr*/
>     0,				  /*tp_as_number*/
>     0,				  /*tp_as_sequence*/
>     0,				  /*tp_as_mapping*/
> diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
> index 928efacfe8a..abe127eca76 100644
> --- a/gdb/python/py-type.c
> +++ b/gdb/python/py-type.c
> @@ -442,6 +442,7 @@ typy_is_signed (PyObject *self, void *closure)
>       Py_RETURN_TRUE;
>   }
>   
> +
spurious white line here
>   /* Return the type, stripped of typedefs. */
>   static PyObject *
>   typy_strip_typedefs (PyObject *self, PyObject *args)
> @@ -1026,6 +1027,34 @@ typy_template_argument (PyObject *self, PyObject *args)
>     return value_to_value_object (val);
>   }
>   
> +/* __repr__ implementation for gdb.Type.  */
> +
> +static PyObject *
> +typy_repr (PyObject *self)
> +{
> +  const auto type = type_object_to_type (self);
> +  if (type == nullptr)
> +    return PyUnicode_FromFormat ("<gdb.Type (invalid)>");
> +
> +  const char *code = pyty_codes[type->code ()].name;
> +  string_file type_name;
> +  try
> +    {
> +      current_language->print_type (type, "",
> +				    &type_name, -1, 0,
> +				    &type_print_raw_options);
> +    }
> +  catch (const gdb_exception &except)
> +    {
> +      GDB_PY_HANDLE_EXCEPTION (except);
> +    }
> +  auto py_typename = PyUnicode_Decode
> +    (type_name.c_str (), type_name.size (),
> +		 host_charset (), NULL);
> +	
> +	return PyUnicode_FromFormat ("<gdb.Type code=%s name=%U>", code, py_typename);
> +}
> +
>   static PyObject *
>   typy_str (PyObject *self)
>   {
> @@ -1612,7 +1641,7 @@ PyTypeObject type_object_type =
>     0,				  /*tp_getattr*/
>     0,				  /*tp_setattr*/
>     0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  typy_repr,                     /*tp_repr*/
>     &type_object_as_number,	  /*tp_as_number*/
>     0,				  /*tp_as_sequence*/
>     &typy_mapping,		  /*tp_as_mapping*/
> diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
> index 1fbbc47c872..a60b4a25cbb 100644
> --- a/gdb/testsuite/gdb.python/py-arch.exp
> +++ b/gdb/testsuite/gdb.python/py-arch.exp
> @@ -29,6 +29,8 @@ if ![runto_main] {
>   # Test python/15461.  Invalid architectures should not trigger an
>   # internal GDB assert.
>   gdb_py_test_silent_cmd "python empty = gdb.Architecture()" "get empty arch" 0
> +gdb_test "python print(repr (empty))" "<gdb\\.Architecture \\(invalid\\)>" \
> +    "Test empty achitecture __repr__ does not trigger an assert"
>   gdb_test "python print(empty.name())" ".*Architecture is invalid.*" \
>       "Test empty architecture.name does not trigger an assert"
>   gdb_test "python print(empty.disassemble())" ".*Architecture is invalid.*" \
> @@ -46,6 +48,8 @@ gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
>   gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(gdb.Value(pc))" \
>     "disassemble no end no count" 0
>   
> +gdb_test "python print (repr (arch))" "<gdb.Architecture arch_name=.* printable_name=.*>" "test __repr__ for architecture"
> +
>   gdb_test "python print (len(insn_list1))" "1" "test number of instructions 1"
>   gdb_test "python print (len(insn_list2))" "1" "test number of instructions 2"
>   gdb_test "python print (len(insn_list3))" "1" "test number of instructions 3"
> diff --git a/gdb/testsuite/gdb.python/py-block.exp b/gdb/testsuite/gdb.python/py-block.exp
> index 0a88aec56a0..5e3d1c72d5e 100644
> --- a/gdb/testsuite/gdb.python/py-block.exp
> +++ b/gdb/testsuite/gdb.python/py-block.exp
> @@ -39,7 +39,7 @@ gdb_continue_to_breakpoint "Block break here."
>   gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
>   gdb_py_test_silent_cmd "python block = frame.block()" \
>       "Get block, initial innermost block" 0
> -gdb_test "python print (block)" "<gdb.Block object at $hex>" "check block not None"
> +gdb_test "python print (block)" "<gdb.Block .* \{.*\}>" "check block not None"
>   gdb_test "python print (block.function)" "None" "first anonymous block"
>   gdb_test "python print (block.start)" "${decimal}" "check start not None"
>   gdb_test "python print (block.end)" "${decimal}" "check end not None"
> @@ -73,7 +73,7 @@ gdb_test "python print (block.function)" "block_func" \
>   gdb_test "up" ".*"
>   gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
>   gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
> -gdb_test "python print (block)" "<gdb.Block object at $hex>" \
> +gdb_test "python print (repr (block))" "<gdb.Block .* \{.*\}>" \
>            "Check Frame 2's block not None"
>   gdb_test "python print (block.function)" "main" "main block"
>   
> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
> index e36e87dc291..0c904a12c90 100644
> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
> @@ -50,11 +50,14 @@ proc_with_prefix test_bkpt_basic { } {
>   	return 0
>       }
>   
> +    set num_exp "-?\[0-9\]+"
> +    set repr_pattern "<gdb.Breakpoint number=$num_exp thread=$num_exp hits=$num_exp enable_count=$num_exp>"
> +
>       # Now there should be one breakpoint: main.
>       gdb_py_test_silent_cmd "python blist = gdb.breakpoints()" \
>   	"Get Breakpoint List" 0
> -    gdb_test "python print (blist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check obj exists @main"
> +    gdb_test "python print (repr (blist\[0\]))" \
> +	"$repr_pattern" "Check obj exists @main"
>       gdb_test "python print (blist\[0\].location)" \
>   	"main." "Check breakpoint location @main"
>       gdb_test "python print (blist\[0\].pending)" "False" \
> @@ -71,12 +74,12 @@ proc_with_prefix test_bkpt_basic { } {
>   	"Get Breakpoint List" 0
>       gdb_test "python print (len(blist))" \
>   	"2" "Check for two breakpoints"
> -    gdb_test "python print (blist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check obj exists @main 2"
> +    gdb_test "python print (repr (blist\[0\]))" \
> +	"$repr_pattern" "Check obj exists @main 2"
>       gdb_test "python print (blist\[0\].location)" \
>   	"main." "Check breakpoint location @main 2"
> -    gdb_test "python print (blist\[1\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check obj exists @mult_line"
> +    gdb_test "python print (repr (blist\[1\]))" \
> +	"$repr_pattern" "Check obj exists @mult_line"
>   
>       gdb_test "python print (blist\[1\].location)" \
>   	"py-breakpoint\.c:${mult_line}*" \
> @@ -224,14 +227,17 @@ proc_with_prefix test_bkpt_invisible { } {
>   	return 0
>       }
>   
> +    set num_exp "-?\[0-9\]+"
> +    set repr_pattern "<gdb.Breakpoint number=$num_exp thread=$num_exp hits=$num_exp enable_count=$num_exp>"
> +
>       delete_breakpoints
>       set ibp_location [gdb_get_line_number "Break at multiply."]
>       gdb_py_test_silent_cmd  "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=False)" \
>   	"Set invisible breakpoint" 0
>       gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
>   	"Get Breakpoint List" 0
> -    gdb_test "python print (ilist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 1"
> +    gdb_test "python print (repr (ilist\[0\]))" \
> +	"$repr_pattern" "Check invisible bp obj exists 1"
>       gdb_test "python print (ilist\[0\].location)" \
>   	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 1"
>       gdb_test "python print (ilist\[0\].visible)" \
> @@ -243,8 +249,8 @@ proc_with_prefix test_bkpt_invisible { } {
>   	"Set invisible breakpoint" 0
>       gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
>   	"Get Breakpoint List" 0
> -    gdb_test "python print (ilist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 2"
> +    gdb_test "python print (repr (ilist\[0\]))" \
> +	"$repr_pattern" "Check invisible bp obj exists 2"
>       gdb_test "python print (ilist\[0\].location)" \
>   	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 2"
>       gdb_test "python print (ilist\[0\].visible)" \
> diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
> index ad06b07c2c6..e0baed9b6d4 100644
> --- a/gdb/testsuite/gdb.python/py-symbol.exp
> +++ b/gdb/testsuite/gdb.python/py-symbol.exp
> @@ -44,6 +44,7 @@ clean_restart ${binfile}
>   # point where we don't have a current frame, and we don't want to
>   # require one.
>   gdb_py_test_silent_cmd "python main_func = gdb.lookup_global_symbol(\"main\")" "Lookup main" 1
> +gdb_test "python print (repr (main_func))" "<gdb.Symbol print_name=.*>" "test main_func.__repr__"
>   gdb_test "python print (main_func.is_function)" "True" "test main_func.is_function"
>   gdb_test "python print (gdb.lookup_global_symbol(\"junk\"))" "None" "test lookup_global_symbol(\"junk\")"
>   
> diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
> index 594c9749d8e..95cdfa54a6e 100644
> --- a/gdb/testsuite/gdb.python/py-type.exp
> +++ b/gdb/testsuite/gdb.python/py-type.exp
> @@ -393,3 +393,7 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
>         test_type_equality
>     }
>   }
> +
> +# Test __repr__()
> +gdb_test "python print (repr (gdb.lookup_type ('char')))" \
> +      "<gdb.Type code=TYPE_CODE_INT name=char>" "test __repr__()"


-- 
Cheers,
Bruno


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Add __repr__() implementation to a few Python types
  2023-01-11  0:35 ` [PATCH] Add __repr__() implementation to a few Python types Matheus Branco Borella
  2023-01-18 17:05   ` Bruno Larsen
@ 2023-01-18 18:02   ` Andrew Burgess
  2023-01-19  8:23     ` Bruno Larsen
  2023-01-20  1:43     ` Matheus Branco Borella
  1 sibling, 2 replies; 15+ messages in thread
From: Andrew Burgess @ 2023-01-18 18:02 UTC (permalink / raw)
  To: Matheus Branco Borella via Gdb-patches, gdb-patches
  Cc: Matheus Branco Borella

Matheus Branco Borella via Gdb-patches <gdb-patches@sourceware.org>
writes:

> Only a few types in the Python API currently have __repr__() implementations.
> This patch adds a few more of them. specifically: it adds __repr__()
> implementations to gdb.Symbol, gdb.Architecture, gdb.Block, gdb.Breakpoint,
> and gdb.Type.
>
> This makes it easier to play around the GDB Python API in the Python interpreter
> session invoked with the 'pi' command in GDB, giving more easily accessible tipe
> information to users.
>
> An example of how this would look like:
> ```
> (gdb) pi
>>> gdb.lookup_type("char")
> <gdb.Type code=TYPE_CODE_INT name=char>
>>> gdb.lookup_global_symbol("main")
> <gdb.Symbol print_name=main>
> ```

Thanks for working on this, I think this will be really useful.

>
> One thing to note about this patch is that it makes use of u8 string literals,
> so as to make sure we meet python's expectations of strings passed to it using
> PyUnicode_FromFormat being encoded in utf8. This should remove the chance of
> odd compilation environments spitting out strings Python would consider invalid
> for the function we're calling.

Sorry for being a little slow.  What does this actually mean?  When you
say "makes use of u8 string literals" - does this mean you have string
literals in this patch containing non ASCII characters?

I've trying to understand why this is different to any other part of GDB
that prints stuff via Python.

> ---
>  gdb/python/py-arch.c                       | 18 +++++++-
>  gdb/python/py-block.c                      | 30 ++++++++++++-
>  gdb/python/py-breakpoint.c                 | 49 +++++++++++++++++++++-
>  gdb/python/py-symbol.c                     | 16 ++++++-
>  gdb/python/py-type.c                       | 31 +++++++++++++-
>  gdb/testsuite/gdb.python/py-arch.exp       |  4 ++
>  gdb/testsuite/gdb.python/py-block.exp      |  4 +-
>  gdb/testsuite/gdb.python/py-breakpoint.exp | 26 +++++++-----
>  gdb/testsuite/gdb.python/py-symbol.exp     |  1 +
>  gdb/testsuite/gdb.python/py-type.exp       |  4 ++
>  10 files changed, 165 insertions(+), 18 deletions(-)
>
> diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
> index cf0978560f9..5384a0d0d0c 100644
> --- a/gdb/python/py-arch.c
> +++ b/gdb/python/py-arch.c
> @@ -319,6 +319,22 @@ archpy_integer_type (PyObject *self, PyObject *args, PyObject *kw)
>    return type_to_type_object (type);
>  }
>  
> +/* __repr__ implementation for gdb.Architecture.  */
> +
> +static PyObject *
> +archpy_repr (PyObject *self)
> +{
> +  const auto gdbarch = arch_object_to_gdbarch (self);
> +  if (gdbarch == nullptr)
> +    return PyUnicode_FromFormat
> +      ("<gdb.Architecture (invalid)>");
> +
> +  return PyUnicode_FromFormat
> +    ("<gdb.Architecture arch_name=%s printable_name=%s>",
> +     gdbarch_bfd_arch_info (gdbarch)->arch_name,
> +     gdbarch_bfd_arch_info (gdbarch)->printable_name);
> +}
> +
>  /* Implementation of gdb.architecture_names().  Return a list of all the
>     BFD architecture names that GDB understands.  */
>  
> @@ -391,7 +407,7 @@ PyTypeObject arch_object_type = {
>    0,                                  /* tp_getattr */
>    0,                                  /* tp_setattr */
>    0,                                  /* tp_compare */
> -  0,                                  /* tp_repr */
> +  archpy_repr,                        /* tp_repr */
>    0,                                  /* tp_as_number */
>    0,                                  /* tp_as_sequence */
>    0,                                  /* tp_as_mapping */
> diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
> index b9aea3aca69..1b8433d41e7 100644
> --- a/gdb/python/py-block.c
> +++ b/gdb/python/py-block.c
> @@ -23,6 +23,7 @@
>  #include "symtab.h"
>  #include "python-internal.h"
>  #include "objfiles.h"
> +#include <sstream>
>  
>  struct block_object {
>    PyObject_HEAD
> @@ -424,6 +425,33 @@ blpy_iter_is_valid (PyObject *self, PyObject *args)
>    Py_RETURN_TRUE;
>  }
>  
> +/* __repr__ implementation for gdb.Block.  */
> +
> +static PyObject *
> +blpy_repr (PyObject *self)
> +{
> +  const auto block = block_object_to_block (self);
> +  if (block == nullptr)
> +    return PyUnicode_FromFormat("<gdb.Block (invalid)>");

Missing space before '('.  I think Bruno pointed out more of these, so
I'll not both pointing out the others I see.

> +
> +  const auto name = block->function () ? block->function ()->print_name () : "<anonymous>";

This line is too long and needs to be wrapped to under 80 characters.

> +
> +  block_iterator iter;
> +  block_iterator_first (block, &iter);
> +
> +  std::stringstream ss;
> +  const struct symbol *symbol;
> +  while ((symbol = block_iterator_next (&iter)) != nullptr)
> +  {
> +    ss << std::endl;
> +    ss << symbol->print_name () << ",";
> +  }
> +  if(!ss.str ().empty ())
> +    ss << std::endl;

We don't make much use of std::stringstream in GDB, and unless there's a
compelling reason, then for consistency, I'd prefer to see this written
using std::string.  While playing with this I rewrote this as:

  std::string str;
  const struct symbol *symbol;
  while ((symbol = block_iterator_next (&iter)) != nullptr)
    str = (str + "\n") + symbol->print_name () + ",";
  if(!str.empty ())
    str += "\n";

  return PyUnicode_FromFormat("<gdb.Block %s {%s}>", name, str.c_str ());

which still seems to pass your tests.

> +
> +  return PyUnicode_FromFormat("<gdb.Block %s {%s}>", name, ss.str ().c_str ());
> +}
> +
>  int
>  gdbpy_initialize_blocks (void)
>  {
> @@ -486,7 +514,7 @@ PyTypeObject block_object_type = {
>    0,				  /*tp_getattr*/
>    0,				  /*tp_setattr*/
>    0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  blpy_repr,                     /*tp_repr*/
>    0,				  /*tp_as_number*/
>    0,				  /*tp_as_sequence*/
>    &block_object_as_mapping,	  /*tp_as_mapping*/
> diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
> index de7b9f4266b..ce307f7be21 100644
> --- a/gdb/python/py-breakpoint.c
> +++ b/gdb/python/py-breakpoint.c
> @@ -33,6 +33,7 @@
>  #include "location.h"
>  #include "py-event.h"
>  #include "linespec.h"
> +#include <sstream>
>  
>  extern PyTypeObject breakpoint_location_object_type
>      CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("breakpoint_location_object");
> @@ -967,6 +968,23 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>    return 0;
>  }
>  
> +/* __repr__ implementation for gdb.Breakpoint.  */
> +
> +static PyObject *
> +bppy_repr(PyObject *self)
> +{
> +  const auto bp = (struct gdbpy_breakpoint_object*) self;
> +  if (bp->bp == nullptr)
> +    return PyUnicode_FromFormat ("<gdb.Breakpoint (invalid)>");
> +
> +  return PyUnicode_FromFormat
> +    ("<gdb.Breakpoint number=%d thread=%d hits=%d enable_count=%d>",
> +     bp->bp->number,
> +     bp->bp->thread,
> +     bp->bp->hit_count,
> +     bp->bp->enable_count);

I think I'd rather see the fields that are included here added
dynamically based on their values.  For example you choose to include
'thread' but not 'task' which seems pretty arbitrary.

Of the fields given here 'number' and 'hits' seem like they should
always be included, but I'd then only include 'thread' if it's not -1,
and 'task' if it's not 0, similarly, the 'enabled_count' is probably
only worth including if it's greater than zero I guess.

This leaves the door open for adding more optional fields in the future.

> +}
> +
>  /* Append to LIST the breakpoint Python object associated to B.
>  
>     Return true on success.  Return false on failure, with the Python error
> @@ -1389,7 +1407,7 @@ PyTypeObject breakpoint_object_type =
>    0,				  /*tp_getattr*/
>    0,				  /*tp_setattr*/
>    0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  bppy_repr,                     /*tp_repr*/
>    0,				  /*tp_as_number*/
>    0,				  /*tp_as_sequence*/
>    0,				  /*tp_as_mapping*/
> @@ -1604,6 +1622,33 @@ bplocpy_dealloc (PyObject *py_self)
>    Py_TYPE (py_self)->tp_free (py_self);
>  }
>  
> +/* __repr__ implementation for gdb.BreakpointLocation.  */
> +
> +static PyObject *
> +bplocpy_repr (PyObject *py_self)
> +{
> +  const auto self = (gdbpy_breakpoint_location_object *) py_self;
> +  if (self->owner == nullptr || self->owner->bp == nullptr || self->owner->bp != self->bp_loc->owner)
> +    return PyUnicode_FromFormat ("<gdb.BreakpointLocation (invalid)>");

That `if` condition line is too long.

> +
> +  const auto enabled = self->bp_loc->enabled ? "enabled" : "disabled";
> +
> +  std::stringstream ss;
> +  ss << std::endl << enabled << std::endl;
> +  ss << "requested_address=0x" << std::hex << self->bp_loc->requested_address << " ";
> +  ss << "address=0x" << self->bp_loc->address << " " << std::dec << std::endl;
> +  if (self->bp_loc->symtab != nullptr)
> +  {
> +    ss << self->bp_loc->symtab->filename << ":" << self->bp_loc->line_number << " " << std::endl;
> +  }
> +
> +  const auto fn_name = self->bp_loc->function_name.get ();
> +  if (fn_name != nullptr)
> +    ss << "in " << fn_name << " " << std::endl;

I think this can all be rewritten using std::string instead of
std::stringstream, maybe using string_printf to do some of the
formatting.  IMHO this would be more consistent with the rest of GDB.

> +
> +  return PyUnicode_FromFormat ("<gdb.BreakpointLocation %s>", ss.str ().c_str ());
> +}
> +
>  /* Attribute get/set Python definitions. */
>  
>  static gdb_PyGetSetDef bp_location_object_getset[] = {
> @@ -1635,7 +1680,7 @@ PyTypeObject breakpoint_location_object_type =
>    0,					/*tp_getattr*/
>    0,					/*tp_setattr*/
>    0,					/*tp_compare*/
> -  0,					/*tp_repr*/
> +  bplocpy_repr,                        /*tp_repr*/
>    0,					/*tp_as_number*/
>    0,					/*tp_as_sequence*/
>    0,					/*tp_as_mapping*/
> diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> index 93c86964f3e..5a8149bbe66 100644
> --- a/gdb/python/py-symbol.c
> +++ b/gdb/python/py-symbol.c
> @@ -375,6 +375,20 @@ sympy_dealloc (PyObject *obj)
>    Py_TYPE (obj)->tp_free (obj);
>  }
>  
> +/* __repr__ implementation for gdb.Symbol.  */
> +
> +static PyObject *
> +sympy_repr (PyObject *self)
> +{
> +  const auto symbol = symbol_object_to_symbol (self);
> +  if (symbol == nullptr)
> +    return PyUnicode_FromFormat ("<gdb.Symbol (invalid)>");
> +
> +  return PyUnicode_FromFormat
> +    ("<gdb.Symbol print_name=%s>",
> +     symbol->print_name ());
> +}
> +
>  /* Implementation of
>     gdb.lookup_symbol (name [, block] [, domain]) -> (symbol, is_field_of_this)
>     A tuple with 2 elements is always returned.  The first is the symbol
> @@ -732,7 +746,7 @@ PyTypeObject symbol_object_type = {
>    0,				  /*tp_getattr*/
>    0,				  /*tp_setattr*/
>    0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  sympy_repr,                    /*tp_repr*/
>    0,				  /*tp_as_number*/
>    0,				  /*tp_as_sequence*/
>    0,				  /*tp_as_mapping*/
> diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
> index 928efacfe8a..abe127eca76 100644
> --- a/gdb/python/py-type.c
> +++ b/gdb/python/py-type.c
> @@ -442,6 +442,7 @@ typy_is_signed (PyObject *self, void *closure)
>      Py_RETURN_TRUE;
>  }
>  
> +

Random extra blank line here.

>  /* Return the type, stripped of typedefs. */
>  static PyObject *
>  typy_strip_typedefs (PyObject *self, PyObject *args)
> @@ -1026,6 +1027,34 @@ typy_template_argument (PyObject *self, PyObject *args)
>    return value_to_value_object (val);
>  }
>  
> +/* __repr__ implementation for gdb.Type.  */
> +
> +static PyObject *
> +typy_repr (PyObject *self)
> +{
> +  const auto type = type_object_to_type (self);
> +  if (type == nullptr)
> +    return PyUnicode_FromFormat ("<gdb.Type (invalid)>");
> +
> +  const char *code = pyty_codes[type->code ()].name;
> +  string_file type_name;
> +  try
> +    {
> +      current_language->print_type (type, "",
> +				    &type_name, -1, 0,
> +				    &type_print_raw_options);
> +    }
> +  catch (const gdb_exception &except)
> +    {
> +      GDB_PY_HANDLE_EXCEPTION (except);
> +    }
> +  auto py_typename = PyUnicode_Decode
> +    (type_name.c_str (), type_name.size (),
> +		 host_charset (), NULL);
> +	
> +	return PyUnicode_FromFormat ("<gdb.Type code=%s name=%U>", code, py_typename);

Here's your whitespace error.  The return line is over indented, and the
preceding line has trailing whitespace.

> +}
> +
>  static PyObject *
>  typy_str (PyObject *self)
>  {
> @@ -1612,7 +1641,7 @@ PyTypeObject type_object_type =
>    0,				  /*tp_getattr*/
>    0,				  /*tp_setattr*/
>    0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  typy_repr,                     /*tp_repr*/
>    &type_object_as_number,	  /*tp_as_number*/
>    0,				  /*tp_as_sequence*/
>    &typy_mapping,		  /*tp_as_mapping*/
> diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
> index 1fbbc47c872..a60b4a25cbb 100644
> --- a/gdb/testsuite/gdb.python/py-arch.exp
> +++ b/gdb/testsuite/gdb.python/py-arch.exp
> @@ -29,6 +29,8 @@ if ![runto_main] {
>  # Test python/15461.  Invalid architectures should not trigger an
>  # internal GDB assert.
>  gdb_py_test_silent_cmd "python empty = gdb.Architecture()" "get empty arch" 0
> +gdb_test "python print(repr (empty))" "<gdb\\.Architecture \\(invalid\\)>" \
> +    "Test empty achitecture __repr__ does not trigger an assert"

I guess I was surprised that so many of the new tests included an
explicit call to repr, given the premise of the change was that simply
'print(empty)' would now print something useful.

I guess maybe it doesn't hurt to _also_ include some explicit repr
calls, but I was expecting most tests to just be printing the object
directly.

Is there some reasoning here that I'm missing?

>  gdb_test "python print(empty.name())" ".*Architecture is invalid.*" \
>      "Test empty architecture.name does not trigger an assert"
>  gdb_test "python print(empty.disassemble())" ".*Architecture is invalid.*" \
> @@ -46,6 +48,8 @@ gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
>  gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(gdb.Value(pc))" \
>    "disassemble no end no count" 0
>  
> +gdb_test "python print (repr (arch))" "<gdb.Architecture arch_name=.* printable_name=.*>" "test __repr__ for architecture"

Over long line, please wrap a little.  There's other long lines in your
patch, I'll not point out each one.

> +
>  gdb_test "python print (len(insn_list1))" "1" "test number of instructions 1"
>  gdb_test "python print (len(insn_list2))" "1" "test number of instructions 2"
>  gdb_test "python print (len(insn_list3))" "1" "test number of instructions 3"
> diff --git a/gdb/testsuite/gdb.python/py-block.exp b/gdb/testsuite/gdb.python/py-block.exp
> index 0a88aec56a0..5e3d1c72d5e 100644
> --- a/gdb/testsuite/gdb.python/py-block.exp
> +++ b/gdb/testsuite/gdb.python/py-block.exp
> @@ -39,7 +39,7 @@ gdb_continue_to_breakpoint "Block break here."
>  gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
>  gdb_py_test_silent_cmd "python block = frame.block()" \
>      "Get block, initial innermost block" 0
> -gdb_test "python print (block)" "<gdb.Block object at $hex>" "check block not None"
> +gdb_test "python print (block)" "<gdb.Block .* \{.*\}>" "check block not None"
>  gdb_test "python print (block.function)" "None" "first anonymous block"
>  gdb_test "python print (block.start)" "${decimal}" "check start not None"
>  gdb_test "python print (block.end)" "${decimal}" "check end not None"
> @@ -73,7 +73,7 @@ gdb_test "python print (block.function)" "block_func" \
>  gdb_test "up" ".*"
>  gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
>  gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
> -gdb_test "python print (block)" "<gdb.Block object at $hex>" \
> +gdb_test "python print (repr (block))" "<gdb.Block .* \{.*\}>" \
>           "Check Frame 2's block not None"
>  gdb_test "python print (block.function)" "main" "main block"
>  
> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
> index e36e87dc291..0c904a12c90 100644
> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
> @@ -50,11 +50,14 @@ proc_with_prefix test_bkpt_basic { } {
>  	return 0
>      }
>  
> +    set num_exp "-?\[0-9\]+"
> +    set repr_pattern "<gdb.Breakpoint number=$num_exp thread=$num_exp hits=$num_exp enable_count=$num_exp>"

We already have $decimal.  I think if you change py-breakpoint.c as I
suggest then you should be able to make use of that and remove num_exp.
This applies below too.

Thanks,
Andrew

> +
>      # Now there should be one breakpoint: main.
>      gdb_py_test_silent_cmd "python blist = gdb.breakpoints()" \
>  	"Get Breakpoint List" 0
> -    gdb_test "python print (blist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check obj exists @main"
> +    gdb_test "python print (repr (blist\[0\]))" \
> +	"$repr_pattern" "Check obj exists @main"
>      gdb_test "python print (blist\[0\].location)" \
>  	"main." "Check breakpoint location @main"
>      gdb_test "python print (blist\[0\].pending)" "False" \
> @@ -71,12 +74,12 @@ proc_with_prefix test_bkpt_basic { } {
>  	"Get Breakpoint List" 0
>      gdb_test "python print (len(blist))" \
>  	"2" "Check for two breakpoints"
> -    gdb_test "python print (blist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check obj exists @main 2"
> +    gdb_test "python print (repr (blist\[0\]))" \
> +	"$repr_pattern" "Check obj exists @main 2"
>      gdb_test "python print (blist\[0\].location)" \
>  	"main." "Check breakpoint location @main 2"
> -    gdb_test "python print (blist\[1\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check obj exists @mult_line"
> +    gdb_test "python print (repr (blist\[1\]))" \
> +	"$repr_pattern" "Check obj exists @mult_line"
>  
>      gdb_test "python print (blist\[1\].location)" \
>  	"py-breakpoint\.c:${mult_line}*" \
> @@ -224,14 +227,17 @@ proc_with_prefix test_bkpt_invisible { } {
>  	return 0
>      }
>  
> +    set num_exp "-?\[0-9\]+"
> +    set repr_pattern "<gdb.Breakpoint number=$num_exp thread=$num_exp hits=$num_exp enable_count=$num_exp>"
> +
>      delete_breakpoints
>      set ibp_location [gdb_get_line_number "Break at multiply."]
>      gdb_py_test_silent_cmd  "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=False)" \
>  	"Set invisible breakpoint" 0
>      gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
>  	"Get Breakpoint List" 0
> -    gdb_test "python print (ilist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 1"
> +    gdb_test "python print (repr (ilist\[0\]))" \
> +	"$repr_pattern" "Check invisible bp obj exists 1"
>      gdb_test "python print (ilist\[0\].location)" \
>  	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 1"
>      gdb_test "python print (ilist\[0\].visible)" \
> @@ -243,8 +249,8 @@ proc_with_prefix test_bkpt_invisible { } {
>  	"Set invisible breakpoint" 0
>      gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
>  	"Get Breakpoint List" 0
> -    gdb_test "python print (ilist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 2"
> +    gdb_test "python print (repr (ilist\[0\]))" \
> +	"$repr_pattern" "Check invisible bp obj exists 2"
>      gdb_test "python print (ilist\[0\].location)" \
>  	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 2"
>      gdb_test "python print (ilist\[0\].visible)" \
> diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
> index ad06b07c2c6..e0baed9b6d4 100644
> --- a/gdb/testsuite/gdb.python/py-symbol.exp
> +++ b/gdb/testsuite/gdb.python/py-symbol.exp
> @@ -44,6 +44,7 @@ clean_restart ${binfile}
>  # point where we don't have a current frame, and we don't want to
>  # require one.
>  gdb_py_test_silent_cmd "python main_func = gdb.lookup_global_symbol(\"main\")" "Lookup main" 1
> +gdb_test "python print (repr (main_func))" "<gdb.Symbol print_name=.*>" "test main_func.__repr__"
>  gdb_test "python print (main_func.is_function)" "True" "test main_func.is_function"
>  gdb_test "python print (gdb.lookup_global_symbol(\"junk\"))" "None" "test lookup_global_symbol(\"junk\")"
>  
> diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
> index 594c9749d8e..95cdfa54a6e 100644
> --- a/gdb/testsuite/gdb.python/py-type.exp
> +++ b/gdb/testsuite/gdb.python/py-type.exp
> @@ -393,3 +393,7 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
>        test_type_equality
>    }
>  }
> +
> +# Test __repr__()
> +gdb_test "python print (repr (gdb.lookup_type ('char')))" \
> +      "<gdb.Type code=TYPE_CODE_INT name=char>" "test __repr__()"
> -- 
> 2.37.3.windows.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Add __repr__() implementation to a few Python types
  2023-01-18 18:02   ` Andrew Burgess
@ 2023-01-19  8:23     ` Bruno Larsen
  2023-01-20  1:43     ` Matheus Branco Borella
  1 sibling, 0 replies; 15+ messages in thread
From: Bruno Larsen @ 2023-01-19  8:23 UTC (permalink / raw)
  To: Andrew Burgess, Matheus Branco Borella via Gdb-patches
  Cc: Matheus Branco Borella

On 18/01/2023 19:02, Andrew Burgess via Gdb-patches wrote:
>> --- a/gdb/testsuite/gdb.python/py-arch.exp
>> +++ b/gdb/testsuite/gdb.python/py-arch.exp
>> @@ -29,6 +29,8 @@ if ![runto_main] {
>>   # Test python/15461.  Invalid architectures should not trigger an
>>   # internal GDB assert.
>>   gdb_py_test_silent_cmd "python empty = gdb.Architecture()" "get empty arch" 0
>> +gdb_test "python print(repr (empty))" "<gdb\\.Architecture \\(invalid\\)>" \
>> +    "Test empty achitecture __repr__ does not trigger an assert"
> I guess I was surprised that so many of the new tests included an
> explicit call to repr, given the premise of the change was that simply
> 'print(empty)' would now print something useful.
>
> I guess maybe it doesn't hurt to_also_  include some explicit repr
> calls, but I was expecting most tests to just be printing the object
> directly.
>
> Is there some reasoning here that I'm missing?
>
When you make a print(empty) call, python internally calls __str__ 
instead or __repr__. The latter is only called when the former is not 
available or when it is being printed by the interpreter. Since 
py-type.c already includes a __str__ implementation, the test would 
still pass if you just had print(empty) without Matheus's changes.

-- 
Cheers,
Bruno


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH] Add __repr__() implementation to a few Python types
  2023-01-18 18:02   ` Andrew Burgess
  2023-01-19  8:23     ` Bruno Larsen
@ 2023-01-20  1:43     ` Matheus Branco Borella
  2023-01-20 16:45       ` Andrew Burgess
  2023-01-24 14:45       ` Andrew Burgess
  1 sibling, 2 replies; 15+ messages in thread
From: Matheus Branco Borella @ 2023-01-20  1:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Matheus Branco Borella

Only a few types in the Python API currently have __repr__() implementations.
This patch adds a few more of them. specifically: it adds __repr__()
implementations to gdb.Symbol, gdb.Architecture, gdb.Block, gdb.Breakpoint,
and gdb.Type.

This makes it easier to play around the GDB Python API in the Python interpreter
session invoked with the 'pi' command in GDB, giving more easily accessible tipe
information to users.

An example of how this would look like:
```
(gdb) pi
>> gdb.lookup_type("char")
<gdb.Type code=TYPE_CODE_INT name=char>
>> gdb.lookup_global_symbol("main")
<gdb.Symbol print_name=main>
```

> Sorry for being a little slow.  What does this actually mean?  When you
> say "makes use of u8 string literals" - does this mean you have string
> literals in this patch containing non ASCII characters?
>
> I've trying to understand why this is different to any other part of GDB
> that prints stuff via Python.

I forgot to take that out of the commit message, my bad. Originally, I'd 
intended for the string literals in the patch that get handed to Python to be
all u8 literals so that I could guarantee it wouldn't break in an environment
that doesn't output regular string literals in an ASCII-compatible encoding,
as Python expects all strings handed to it to be encoded in UTF-8. But seeing
as all of the rest of the Python interface code uses regular string literals, 
I figured it wouldn't make much of difference having them in anyway.

> I guess I was surprised that so many of the new tests included an
> explicit call to repr, given the premise of the change was that simply
> 'print(empty)' would now print something useful.
>
> I guess maybe it doesn't hurt to _also_ include some explicit repr
> calls, but I was expecting most tests to just be printing the object
> directly.
As blarsen@ also pointed out, `print`-ing an object directly that has an 
implmentation of __str__() will print whatever its __str__() functions returns, 
regardless of whether it implements __repr__() or not, which is not what we want 
here. __repr__() is always preferred in the REPL, though, so it's understandable 
it might not be clear at first why I'm calling `repr()` explicitly.

> Over long line, please wrap a little.  There's other long lines in your
> patch, I'll not point out each one.

Should be all fixed now (hopefully I didn't miss any), with the exception of the
`repr_pattern` strings in `py-breakpoint.exp`, which I couldn't for the life of 
me get to match properly with the output were they not on a single line.

---
 gdb/python/py-arch.c                       | 18 +++++-
 gdb/python/py-block.c                      | 27 ++++++++-
 gdb/python/py-breakpoint.c                 | 68 +++++++++++++++++++++-
 gdb/python/py-symbol.c                     | 16 ++++-
 gdb/python/py-type.c                       | 30 +++++++++-
 gdb/testsuite/gdb.python/py-arch.exp       |  6 ++
 gdb/testsuite/gdb.python/py-block.exp      |  4 +-
 gdb/testsuite/gdb.python/py-breakpoint.exp | 24 ++++----
 gdb/testsuite/gdb.python/py-symbol.exp     |  2 +
 gdb/testsuite/gdb.python/py-type.exp       |  4 ++
 10 files changed, 181 insertions(+), 18 deletions(-)

diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index cf0978560f9..5384a0d0d0c 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -319,6 +319,22 @@ archpy_integer_type (PyObject *self, PyObject *args, PyObject *kw)
   return type_to_type_object (type);
 }
 
+/* __repr__ implementation for gdb.Architecture.  */
+
+static PyObject *
+archpy_repr (PyObject *self)
+{
+  const auto gdbarch = arch_object_to_gdbarch (self);
+  if (gdbarch == nullptr)
+    return PyUnicode_FromFormat
+      ("<gdb.Architecture (invalid)>");
+
+  return PyUnicode_FromFormat
+    ("<gdb.Architecture arch_name=%s printable_name=%s>",
+     gdbarch_bfd_arch_info (gdbarch)->arch_name,
+     gdbarch_bfd_arch_info (gdbarch)->printable_name);
+}
+
 /* Implementation of gdb.architecture_names().  Return a list of all the
    BFD architecture names that GDB understands.  */
 
@@ -391,7 +407,7 @@ PyTypeObject arch_object_type = {
   0,                                  /* tp_getattr */
   0,                                  /* tp_setattr */
   0,                                  /* tp_compare */
-  0,                                  /* tp_repr */
+  archpy_repr,                        /* tp_repr */
   0,                                  /* tp_as_number */
   0,                                  /* tp_as_sequence */
   0,                                  /* tp_as_mapping */
diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index b9aea3aca69..b4c55add765 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -424,6 +424,31 @@ blpy_iter_is_valid (PyObject *self, PyObject *args)
   Py_RETURN_TRUE;
 }
 
+/* __repr__ implementation for gdb.Block.  */
+
+static PyObject *
+blpy_repr (PyObject *self)
+{
+  const auto block = block_object_to_block (self);
+  if (block == nullptr)
+    return PyUnicode_FromFormat ("<gdb.Block (invalid)>");
+
+  const auto name = block->function () ?
+    block->function ()->print_name () : "<anonymous>";
+
+  block_iterator iter;
+  block_iterator_first (block, &iter);
+
+  std::string str;
+  const struct symbol *symbol;
+  while ((symbol = block_iterator_next (&iter)) != nullptr)
+    str = (str + "\n") + symbol->print_name () + ",";
+  if(!str.empty ())
+    str += "\n";
+
+  return PyUnicode_FromFormat ("<gdb.Block %s {%s}>", name, str.c_str ());
+}
+
 int
 gdbpy_initialize_blocks (void)
 {
@@ -486,7 +511,7 @@ PyTypeObject block_object_type = {
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  blpy_repr,                     /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   &block_object_as_mapping,	  /*tp_as_mapping*/
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index de7b9f4266b..d68a205330c 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -33,6 +33,7 @@
 #include "location.h"
 #include "py-event.h"
 #include "linespec.h"
+#include "gdbsupport/common-utils.h"
 
 extern PyTypeObject breakpoint_location_object_type
     CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("breakpoint_location_object");
@@ -967,6 +968,31 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
   return 0;
 }
 
+/* __repr__ implementation for gdb.Breakpoint.  */
+
+static PyObject *
+bppy_repr (PyObject *self)
+{
+  const auto bp = (struct gdbpy_breakpoint_object*) self;
+  if (bp->bp == nullptr)
+    return PyUnicode_FromFormat ("<gdb.Breakpoint (invalid)>");
+
+  std::string str = " ";
+  if (bp->bp->thread != -1)
+    str += string_printf ("thread=%d ", bp->bp->thread);
+  if (bp->bp->task > 0)
+    str += string_printf ("task=%d ", bp->bp->task);
+  if (bp->bp->enable_count > 0)
+    str += string_printf ("enable_count=%d ", bp->bp->enable_count);
+  str.pop_back ();
+
+  return PyUnicode_FromFormat
+    ("<gdb.Breakpoint number=%d hits=%d%s>",
+     bp->bp->number,
+     bp->bp->hit_count,
+     str.c_str ());
+}
+
 /* Append to LIST the breakpoint Python object associated to B.
 
    Return true on success.  Return false on failure, with the Python error
@@ -1389,7 +1415,7 @@ PyTypeObject breakpoint_object_type =
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  bppy_repr,                     /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   0,				  /*tp_as_mapping*/
@@ -1604,6 +1630,44 @@ bplocpy_dealloc (PyObject *py_self)
   Py_TYPE (py_self)->tp_free (py_self);
 }
 
+/* __repr__ implementation for gdb.BreakpointLocation.  */
+
+static PyObject *
+bplocpy_repr (PyObject *py_self)
+{
+  const auto self = (gdbpy_breakpoint_location_object *) py_self;
+  if (self->owner == nullptr || self->owner->bp == nullptr
+    || self->owner->bp != self->bp_loc->owner)
+    return PyUnicode_FromFormat ("<gdb.BreakpointLocation (invalid)>");
+
+  const auto enabled = self->bp_loc->enabled ? "enabled" : "disabled";
+
+  std::string str(enabled);
+
+  str += " requested_address=0x";
+  str += string_printf ("%lx", self->bp_loc->requested_address);
+
+  str += " address=0x";
+  str += string_printf ("%lx", self->bp_loc->address);
+
+  if (self->bp_loc->symtab != nullptr)
+  {
+    str += " source=";
+    str += self->bp_loc->symtab->filename;
+    str += ":";
+    str += string_printf ("%d", self->bp_loc->line_number);
+  }
+
+  const auto fn_name = self->bp_loc->function_name.get ();
+  if (fn_name != nullptr)
+  {
+    str += " in ";
+    str += fn_name;
+  }
+
+  return PyUnicode_FromFormat ("<gdb.BreakpointLocation %s>", str.c_str ());
+}
+
 /* Attribute get/set Python definitions. */
 
 static gdb_PyGetSetDef bp_location_object_getset[] = {
@@ -1635,7 +1699,7 @@ PyTypeObject breakpoint_location_object_type =
   0,					/*tp_getattr*/
   0,					/*tp_setattr*/
   0,					/*tp_compare*/
-  0,					/*tp_repr*/
+  bplocpy_repr,                        /*tp_repr*/
   0,					/*tp_as_number*/
   0,					/*tp_as_sequence*/
   0,					/*tp_as_mapping*/
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index 93c86964f3e..5a8149bbe66 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -375,6 +375,20 @@ sympy_dealloc (PyObject *obj)
   Py_TYPE (obj)->tp_free (obj);
 }
 
+/* __repr__ implementation for gdb.Symbol.  */
+
+static PyObject *
+sympy_repr (PyObject *self)
+{
+  const auto symbol = symbol_object_to_symbol (self);
+  if (symbol == nullptr)
+    return PyUnicode_FromFormat ("<gdb.Symbol (invalid)>");
+
+  return PyUnicode_FromFormat
+    ("<gdb.Symbol print_name=%s>",
+     symbol->print_name ());
+}
+
 /* Implementation of
    gdb.lookup_symbol (name [, block] [, domain]) -> (symbol, is_field_of_this)
    A tuple with 2 elements is always returned.  The first is the symbol
@@ -732,7 +746,7 @@ PyTypeObject symbol_object_type = {
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  sympy_repr,                    /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   0,				  /*tp_as_mapping*/
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 928efacfe8a..eb11ef029ca 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -1026,6 +1026,34 @@ typy_template_argument (PyObject *self, PyObject *args)
   return value_to_value_object (val);
 }
 
+/* __repr__ implementation for gdb.Type.  */
+
+static PyObject *
+typy_repr (PyObject *self)
+{
+  const auto type = type_object_to_type (self);
+  if (type == nullptr)
+    return PyUnicode_FromFormat ("<gdb.Type (invalid)>");
+
+  const char *code = pyty_codes[type->code ()].name;
+  string_file type_name;
+  try
+    {
+      current_language->print_type (type, "",
+				    &type_name, -1, 0,
+				    &type_print_raw_options);
+    }
+  catch (const gdb_exception &except)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+    }
+  auto py_typename = PyUnicode_Decode
+    (type_name.c_str (), type_name.size (),
+		 host_charset (), NULL);
+	
+  return PyUnicode_FromFormat ("<gdb.Type code=%s name=%U>", code, py_typename);
+}
+
 static PyObject *
 typy_str (PyObject *self)
 {
@@ -1612,7 +1640,7 @@ PyTypeObject type_object_type =
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  typy_repr,                     /*tp_repr*/
   &type_object_as_number,	  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   &typy_mapping,		  /*tp_as_mapping*/
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
index 1fbbc47c872..d436c957e25 100644
--- a/gdb/testsuite/gdb.python/py-arch.exp
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -29,6 +29,8 @@ if ![runto_main] {
 # Test python/15461.  Invalid architectures should not trigger an
 # internal GDB assert.
 gdb_py_test_silent_cmd "python empty = gdb.Architecture()" "get empty arch" 0
+gdb_test "python print(repr (empty))" "<gdb\\.Architecture \\(invalid\\)>" \
+    "Test empty achitecture __repr__ does not trigger an assert"
 gdb_test "python print(empty.name())" ".*Architecture is invalid.*" \
     "Test empty architecture.name does not trigger an assert"
 gdb_test "python print(empty.disassemble())" ".*Architecture is invalid.*" \
@@ -46,6 +48,10 @@ gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
 gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(gdb.Value(pc))" \
   "disassemble no end no count" 0
 
+gdb_test "python print (repr (arch))" \
+    "<gdb.Architecture arch_name=.* printable_name=.*>" \
+    "test __repr__ for architecture"
+
 gdb_test "python print (len(insn_list1))" "1" "test number of instructions 1"
 gdb_test "python print (len(insn_list2))" "1" "test number of instructions 2"
 gdb_test "python print (len(insn_list3))" "1" "test number of instructions 3"
diff --git a/gdb/testsuite/gdb.python/py-block.exp b/gdb/testsuite/gdb.python/py-block.exp
index 0a88aec56a0..5e3d1c72d5e 100644
--- a/gdb/testsuite/gdb.python/py-block.exp
+++ b/gdb/testsuite/gdb.python/py-block.exp
@@ -39,7 +39,7 @@ gdb_continue_to_breakpoint "Block break here."
 gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
 gdb_py_test_silent_cmd "python block = frame.block()" \
     "Get block, initial innermost block" 0
-gdb_test "python print (block)" "<gdb.Block object at $hex>" "check block not None"
+gdb_test "python print (block)" "<gdb.Block .* \{.*\}>" "check block not None"
 gdb_test "python print (block.function)" "None" "first anonymous block"
 gdb_test "python print (block.start)" "${decimal}" "check start not None"
 gdb_test "python print (block.end)" "${decimal}" "check end not None"
@@ -73,7 +73,7 @@ gdb_test "python print (block.function)" "block_func" \
 gdb_test "up" ".*"
 gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
 gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
-gdb_test "python print (block)" "<gdb.Block object at $hex>" \
+gdb_test "python print (repr (block))" "<gdb.Block .* \{.*\}>" \
          "Check Frame 2's block not None"
 gdb_test "python print (block.function)" "main" "main block"
 
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index e36e87dc291..4da46431a3a 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -50,11 +50,13 @@ proc_with_prefix test_bkpt_basic { } {
 	return 0
     }
 
+    set repr_pattern "<gdb.Breakpoint number=-?$decimal hits=-?$decimal\( thread=$decimal\)?\( task=$decimal\)?\( enable_count=$decimal\)?>"
+
     # Now there should be one breakpoint: main.
     gdb_py_test_silent_cmd "python blist = gdb.breakpoints()" \
 	"Get Breakpoint List" 0
-    gdb_test "python print (blist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check obj exists @main"
+    gdb_test "python print (repr (blist\[0\]))" \
+	"$repr_pattern" "Check obj exists @main"
     gdb_test "python print (blist\[0\].location)" \
 	"main." "Check breakpoint location @main"
     gdb_test "python print (blist\[0\].pending)" "False" \
@@ -71,12 +73,12 @@ proc_with_prefix test_bkpt_basic { } {
 	"Get Breakpoint List" 0
     gdb_test "python print (len(blist))" \
 	"2" "Check for two breakpoints"
-    gdb_test "python print (blist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check obj exists @main 2"
+    gdb_test "python print (repr (blist\[0\]))" \
+	"$repr_pattern" "Check obj exists @main 2"
     gdb_test "python print (blist\[0\].location)" \
 	"main." "Check breakpoint location @main 2"
-    gdb_test "python print (blist\[1\])" \
-	"<gdb.Breakpoint object at $hex>" "Check obj exists @mult_line"
+    gdb_test "python print (repr (blist\[1\]))" \
+	"$repr_pattern" "Check obj exists @mult_line"
 
     gdb_test "python print (blist\[1\].location)" \
 	"py-breakpoint\.c:${mult_line}*" \
@@ -224,14 +226,16 @@ proc_with_prefix test_bkpt_invisible { } {
 	return 0
     }
 
+    set repr_pattern "<gdb.Breakpoint number=-?$decimal hits=-?$decimal\( thread=$decimal\)?\( task=$decimal\)?\( enable_count=$decimal\)?>"
+
     delete_breakpoints
     set ibp_location [gdb_get_line_number "Break at multiply."]
     gdb_py_test_silent_cmd  "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=False)" \
 	"Set invisible breakpoint" 0
     gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
 	"Get Breakpoint List" 0
-    gdb_test "python print (ilist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 1"
+    gdb_test "python print (repr (ilist\[0\]))" \
+	"$repr_pattern" "Check invisible bp obj exists 1"
     gdb_test "python print (ilist\[0\].location)" \
 	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 1"
     gdb_test "python print (ilist\[0\].visible)" \
@@ -243,8 +247,8 @@ proc_with_prefix test_bkpt_invisible { } {
 	"Set invisible breakpoint" 0
     gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
 	"Get Breakpoint List" 0
-    gdb_test "python print (ilist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 2"
+    gdb_test "python print (repr (ilist\[0\]))" \
+	"$repr_pattern" "Check invisible bp obj exists 2"
     gdb_test "python print (ilist\[0\].location)" \
 	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 2"
     gdb_test "python print (ilist\[0\].visible)" \
diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
index ad06b07c2c6..979b7dfb8fb 100644
--- a/gdb/testsuite/gdb.python/py-symbol.exp
+++ b/gdb/testsuite/gdb.python/py-symbol.exp
@@ -44,6 +44,8 @@ clean_restart ${binfile}
 # point where we don't have a current frame, and we don't want to
 # require one.
 gdb_py_test_silent_cmd "python main_func = gdb.lookup_global_symbol(\"main\")" "Lookup main" 1
+gdb_test "python print (repr (main_func))" "<gdb.Symbol print_name=.*>" \
+    "test main_func.__repr__"
 gdb_test "python print (main_func.is_function)" "True" "test main_func.is_function"
 gdb_test "python print (gdb.lookup_global_symbol(\"junk\"))" "None" "test lookup_global_symbol(\"junk\")"
 
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 594c9749d8e..95cdfa54a6e 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -393,3 +393,7 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
       test_type_equality
   }
 }
+
+# Test __repr__()
+gdb_test "python print (repr (gdb.lookup_type ('char')))" \
+      "<gdb.Type code=TYPE_CODE_INT name=char>" "test __repr__()"
-- 
2.37.3.windows.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Add __repr__() implementation to a few Python types
  2023-01-20  1:43     ` Matheus Branco Borella
@ 2023-01-20 16:45       ` Andrew Burgess
  2023-01-24 14:45       ` Andrew Burgess
  1 sibling, 0 replies; 15+ messages in thread
From: Andrew Burgess @ 2023-01-20 16:45 UTC (permalink / raw)
  To: Matheus Branco Borella via Gdb-patches, gdb-patches
  Cc: Matheus Branco Borella

Matheus Branco Borella via Gdb-patches <gdb-patches@sourceware.org>
writes:

> Only a few types in the Python API currently have __repr__() implementations.
> This patch adds a few more of them. specifically: it adds __repr__()
> implementations to gdb.Symbol, gdb.Architecture, gdb.Block, gdb.Breakpoint,
> and gdb.Type.
>
> This makes it easier to play around the GDB Python API in the Python interpreter
> session invoked with the 'pi' command in GDB, giving more easily accessible tipe
> information to users.
>
> An example of how this would look like:
> ```
> (gdb) pi
>>> gdb.lookup_type("char")
> <gdb.Type code=TYPE_CODE_INT name=char>
>>> gdb.lookup_global_symbol("main")
> <gdb.Symbol print_name=main>
> ```
>
>> Sorry for being a little slow.  What does this actually mean?  When you
>> say "makes use of u8 string literals" - does this mean you have string
>> literals in this patch containing non ASCII characters?
>>
>> I've trying to understand why this is different to any other part of GDB
>> that prints stuff via Python.
>
> I forgot to take that out of the commit message, my bad. Originally, I'd 
> intended for the string literals in the patch that get handed to Python to be
> all u8 literals so that I could guarantee it wouldn't break in an environment
> that doesn't output regular string literals in an ASCII-compatible encoding,
> as Python expects all strings handed to it to be encoded in UTF-8. But seeing
> as all of the rest of the Python interface code uses regular string literals, 
> I figured it wouldn't make much of difference having them in anyway.

Thanks, that makes sense.

>
>> I guess I was surprised that so many of the new tests included an
>> explicit call to repr, given the premise of the change was that simply
>> 'print(empty)' would now print something useful.
>>
>> I guess maybe it doesn't hurt to _also_ include some explicit repr
>> calls, but I was expecting most tests to just be printing the object
>> directly.
> As blarsen@ also pointed out, `print`-ing an object directly that has an 
> implmentation of __str__() will print whatever its __str__() functions returns, 
> regardless of whether it implements __repr__() or not, which is not what we want 
> here. __repr__() is always preferred in the REPL, though, so it's understandable 
> it might not be clear at first why I'm calling `repr()` explicitly.

Again, thanks (and to Bruno too) for the explanation.  I understand
__str__ and __repr__ better now :)  I agree that what you're doing makes
sense now I understand it.

>
>> Over long line, please wrap a little.  There's other long lines in your
>> patch, I'll not point out each one.
>
> Should be all fixed now (hopefully I didn't miss any), with the exception of the
> `repr_pattern` strings in `py-breakpoint.exp`, which I couldn't for the life of 
> me get to match properly with the output were they not on a single line.
>
> ---
>  gdb/python/py-arch.c                       | 18 +++++-
>  gdb/python/py-block.c                      | 27 ++++++++-
>  gdb/python/py-breakpoint.c                 | 68 +++++++++++++++++++++-
>  gdb/python/py-symbol.c                     | 16 ++++-
>  gdb/python/py-type.c                       | 30 +++++++++-
>  gdb/testsuite/gdb.python/py-arch.exp       |  6 ++
>  gdb/testsuite/gdb.python/py-block.exp      |  4 +-
>  gdb/testsuite/gdb.python/py-breakpoint.exp | 24 ++++----
>  gdb/testsuite/gdb.python/py-symbol.exp     |  2 +
>  gdb/testsuite/gdb.python/py-type.exp       |  4 ++
>  10 files changed, 181 insertions(+), 18 deletions(-)
>
> diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
> index cf0978560f9..5384a0d0d0c 100644
> --- a/gdb/python/py-arch.c
> +++ b/gdb/python/py-arch.c
> @@ -319,6 +319,22 @@ archpy_integer_type (PyObject *self, PyObject *args, PyObject *kw)
>    return type_to_type_object (type);
>  }
>  
> +/* __repr__ implementation for gdb.Architecture.  */
> +
> +static PyObject *
> +archpy_repr (PyObject *self)
> +{
> +  const auto gdbarch = arch_object_to_gdbarch (self);
> +  if (gdbarch == nullptr)
> +    return PyUnicode_FromFormat
> +      ("<gdb.Architecture (invalid)>");
> +
> +  return PyUnicode_FromFormat
> +    ("<gdb.Architecture arch_name=%s printable_name=%s>",
> +     gdbarch_bfd_arch_info (gdbarch)->arch_name,
> +     gdbarch_bfd_arch_info (gdbarch)->printable_name);
> +}
> +
>  /* Implementation of gdb.architecture_names().  Return a list of all the
>     BFD architecture names that GDB understands.  */
>  
> @@ -391,7 +407,7 @@ PyTypeObject arch_object_type = {
>    0,                                  /* tp_getattr */
>    0,                                  /* tp_setattr */
>    0,                                  /* tp_compare */
> -  0,                                  /* tp_repr */
> +  archpy_repr,                        /* tp_repr */
>    0,                                  /* tp_as_number */
>    0,                                  /* tp_as_sequence */
>    0,                                  /* tp_as_mapping */
> diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
> index b9aea3aca69..b4c55add765 100644
> --- a/gdb/python/py-block.c
> +++ b/gdb/python/py-block.c
> @@ -424,6 +424,31 @@ blpy_iter_is_valid (PyObject *self, PyObject *args)
>    Py_RETURN_TRUE;
>  }
>  
> +/* __repr__ implementation for gdb.Block.  */
> +
> +static PyObject *
> +blpy_repr (PyObject *self)
> +{
> +  const auto block = block_object_to_block (self);
> +  if (block == nullptr)
> +    return PyUnicode_FromFormat ("<gdb.Block (invalid)>");
> +
> +  const auto name = block->function () ?
> +    block->function ()->print_name () : "<anonymous>";
> +
> +  block_iterator iter;
> +  block_iterator_first (block, &iter);
> +
> +  std::string str;
> +  const struct symbol *symbol;
> +  while ((symbol = block_iterator_next (&iter)) != nullptr)

You should be using ALL_BLOCK_SYMBOLS here rather than calling the
block_iterator_* functions directly.  As it's currently written this
code will crash on a block with no symbols.

I've included a fix-up patch at the end of this email that both fixes
this issue, and extended the existing test to check this case.  Please
feel free to merge this with your work.

> +    str = (str + "\n") + symbol->print_name () + ",";

I don't object to including all the symbol names as you've done.  But
did you consider that some blocks could have a _lot_ of symbols?

As an alternative did you consider just counting the symbols, and
including the count?

This isn't a requirement, if you feel the symbol list is going to be
more useful then I don't feel strongly enough, but I just wanted to
ask.

> +  if(!str.empty ())
> +    str += "\n";
> +
> +  return PyUnicode_FromFormat ("<gdb.Block %s {%s}>", name, str.c_str ());
> +}
> +
>  int
>  gdbpy_initialize_blocks (void)
>  {
> @@ -486,7 +511,7 @@ PyTypeObject block_object_type = {
>    0,				  /*tp_getattr*/
>    0,				  /*tp_setattr*/
>    0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  blpy_repr,                     /*tp_repr*/
>    0,				  /*tp_as_number*/
>    0,				  /*tp_as_sequence*/
>    &block_object_as_mapping,	  /*tp_as_mapping*/
> diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
> index de7b9f4266b..d68a205330c 100644
> --- a/gdb/python/py-breakpoint.c
> +++ b/gdb/python/py-breakpoint.c
> @@ -33,6 +33,7 @@
>  #include "location.h"
>  #include "py-event.h"
>  #include "linespec.h"
> +#include "gdbsupport/common-utils.h"
>  
>  extern PyTypeObject breakpoint_location_object_type
>      CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("breakpoint_location_object");
> @@ -967,6 +968,31 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>    return 0;
>  }
>  
> +/* __repr__ implementation for gdb.Breakpoint.  */
> +
> +static PyObject *
> +bppy_repr (PyObject *self)
> +{
> +  const auto bp = (struct gdbpy_breakpoint_object*) self;
> +  if (bp->bp == nullptr)
> +    return PyUnicode_FromFormat ("<gdb.Breakpoint (invalid)>");
> +
> +  std::string str = " ";
> +  if (bp->bp->thread != -1)
> +    str += string_printf ("thread=%d ", bp->bp->thread);
> +  if (bp->bp->task > 0)
> +    str += string_printf ("task=%d ", bp->bp->task);
> +  if (bp->bp->enable_count > 0)
> +    str += string_printf ("enable_count=%d ", bp->bp->enable_count);
> +  str.pop_back ();
> +
> +  return PyUnicode_FromFormat
> +    ("<gdb.Breakpoint number=%d hits=%d%s>",
> +     bp->bp->number,
> +     bp->bp->hit_count,
> +     str.c_str ());
> +}
> +
>  /* Append to LIST the breakpoint Python object associated to B.
>  
>     Return true on success.  Return false on failure, with the Python error
> @@ -1389,7 +1415,7 @@ PyTypeObject breakpoint_object_type =
>    0,				  /*tp_getattr*/
>    0,				  /*tp_setattr*/
>    0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  bppy_repr,                     /*tp_repr*/
>    0,				  /*tp_as_number*/
>    0,				  /*tp_as_sequence*/
>    0,				  /*tp_as_mapping*/
> @@ -1604,6 +1630,44 @@ bplocpy_dealloc (PyObject *py_self)
>    Py_TYPE (py_self)->tp_free (py_self);
>  }
>  
> +/* __repr__ implementation for gdb.BreakpointLocation.  */
> +
> +static PyObject *
> +bplocpy_repr (PyObject *py_self)
> +{
> +  const auto self = (gdbpy_breakpoint_location_object *) py_self;
> +  if (self->owner == nullptr || self->owner->bp == nullptr
> +    || self->owner->bp != self->bp_loc->owner)
> +    return PyUnicode_FromFormat ("<gdb.BreakpointLocation (invalid)>");
> +
> +  const auto enabled = self->bp_loc->enabled ? "enabled" : "disabled";
> +
> +  std::string str(enabled);
> +
> +  str += " requested_address=0x";
> +  str += string_printf ("%lx", self->bp_loc->requested_address);
> +
> +  str += " address=0x";
> +  str += string_printf ("%lx", self->bp_loc->address);

I'd suggest placing 'address' first, and only include
'requested_address' if it's different to 'address'.  In almost all cases
these two fields will be the same, so this would cut down on some
clutter.

Additionally, you should use core_addr_to_string_nz to format the
CORE_ADDR, like this:

  str += " address=";
  str += string_printf ("%s", core_addr_to_string_nz (self->bp_loc->address));


> +
> +  if (self->bp_loc->symtab != nullptr)
> +  {
> +    str += " source=";
> +    str += self->bp_loc->symtab->filename;
> +    str += ":";
> +    str += string_printf ("%d", self->bp_loc->line_number);
> +  }

The GNU style is to indent the opening and closing curly braces by an
additional 2 spaces, and then everything within the block gets 2 extra
spaces beyond that.

> +
> +  const auto fn_name = self->bp_loc->function_name.get ();
> +  if (fn_name != nullptr)
> +  {
> +    str += " in ";
> +    str += fn_name;
> +  }

Indentation again here.

> +
> +  return PyUnicode_FromFormat ("<gdb.BreakpointLocation %s>", str.c_str ());
> +}
> +
>  /* Attribute get/set Python definitions. */
>  
>  static gdb_PyGetSetDef bp_location_object_getset[] = {
> @@ -1635,7 +1699,7 @@ PyTypeObject breakpoint_location_object_type =
>    0,					/*tp_getattr*/
>    0,					/*tp_setattr*/
>    0,					/*tp_compare*/
> -  0,					/*tp_repr*/
> +  bplocpy_repr,                        /*tp_repr*/
>    0,					/*tp_as_number*/
>    0,					/*tp_as_sequence*/
>    0,					/*tp_as_mapping*/
> diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> index 93c86964f3e..5a8149bbe66 100644
> --- a/gdb/python/py-symbol.c
> +++ b/gdb/python/py-symbol.c
> @@ -375,6 +375,20 @@ sympy_dealloc (PyObject *obj)
>    Py_TYPE (obj)->tp_free (obj);
>  }
>  
> +/* __repr__ implementation for gdb.Symbol.  */
> +
> +static PyObject *
> +sympy_repr (PyObject *self)
> +{
> +  const auto symbol = symbol_object_to_symbol (self);
> +  if (symbol == nullptr)
> +    return PyUnicode_FromFormat ("<gdb.Symbol (invalid)>");
> +
> +  return PyUnicode_FromFormat
> +    ("<gdb.Symbol print_name=%s>",
> +     symbol->print_name ());
> +}
> +
>  /* Implementation of
>     gdb.lookup_symbol (name [, block] [, domain]) -> (symbol, is_field_of_this)
>     A tuple with 2 elements is always returned.  The first is the symbol
> @@ -732,7 +746,7 @@ PyTypeObject symbol_object_type = {
>    0,				  /*tp_getattr*/
>    0,				  /*tp_setattr*/
>    0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  sympy_repr,                    /*tp_repr*/
>    0,				  /*tp_as_number*/
>    0,				  /*tp_as_sequence*/
>    0,				  /*tp_as_mapping*/
> diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
> index 928efacfe8a..eb11ef029ca 100644
> --- a/gdb/python/py-type.c
> +++ b/gdb/python/py-type.c
> @@ -1026,6 +1026,34 @@ typy_template_argument (PyObject *self, PyObject *args)
>    return value_to_value_object (val);
>  }
>  
> +/* __repr__ implementation for gdb.Type.  */
> +
> +static PyObject *
> +typy_repr (PyObject *self)
> +{
> +  const auto type = type_object_to_type (self);
> +  if (type == nullptr)
> +    return PyUnicode_FromFormat ("<gdb.Type (invalid)>");
> +
> +  const char *code = pyty_codes[type->code ()].name;
> +  string_file type_name;
> +  try
> +    {
> +      current_language->print_type (type, "",
> +				    &type_name, -1, 0,
> +				    &type_print_raw_options);
> +    }
> +  catch (const gdb_exception &except)
> +    {
> +      GDB_PY_HANDLE_EXCEPTION (except);
> +    }
> +  auto py_typename = PyUnicode_Decode
> +    (type_name.c_str (), type_name.size (),
> +		 host_charset (), NULL);

I think there's enough space to start the arguments on the
PyUnicode_Decode line (i.e. no line break before '(' char).  The
'host_charset ()' would still be the start of a second line, and should
align under 'type_name':

  auto py_typename = PyUnicode_Decode (type_name.c_str (), type_name.size (),
				       host_charset (), NULL);

> +	

You still have some trailing whitespace on this ^^^ line.

> +  return PyUnicode_FromFormat ("<gdb.Type code=%s name=%U>", code, py_typename);
> +}
> +
>  static PyObject *
>  typy_str (PyObject *self)
>  {
> @@ -1612,7 +1640,7 @@ PyTypeObject type_object_type =
>    0,				  /*tp_getattr*/
>    0,				  /*tp_setattr*/
>    0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  typy_repr,                     /*tp_repr*/
>    &type_object_as_number,	  /*tp_as_number*/
>    0,				  /*tp_as_sequence*/
>    &typy_mapping,		  /*tp_as_mapping*/
> diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
> index 1fbbc47c872..d436c957e25 100644
> --- a/gdb/testsuite/gdb.python/py-arch.exp
> +++ b/gdb/testsuite/gdb.python/py-arch.exp
> @@ -29,6 +29,8 @@ if ![runto_main] {
>  # Test python/15461.  Invalid architectures should not trigger an
>  # internal GDB assert.
>  gdb_py_test_silent_cmd "python empty = gdb.Architecture()" "get empty arch" 0
> +gdb_test "python print(repr (empty))" "<gdb\\.Architecture \\(invalid\\)>" \
> +    "Test empty achitecture __repr__ does not trigger an assert"
>  gdb_test "python print(empty.name())" ".*Architecture is invalid.*" \
>      "Test empty architecture.name does not trigger an assert"
>  gdb_test "python print(empty.disassemble())" ".*Architecture is invalid.*" \
> @@ -46,6 +48,10 @@ gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
>  gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(gdb.Value(pc))" \
>    "disassemble no end no count" 0
>  
> +gdb_test "python print (repr (arch))" \
> +    "<gdb.Architecture arch_name=.* printable_name=.*>" \
> +    "test __repr__ for architecture"
> +
>  gdb_test "python print (len(insn_list1))" "1" "test number of instructions 1"
>  gdb_test "python print (len(insn_list2))" "1" "test number of instructions 2"
>  gdb_test "python print (len(insn_list3))" "1" "test number of instructions 3"
> diff --git a/gdb/testsuite/gdb.python/py-block.exp b/gdb/testsuite/gdb.python/py-block.exp
> index 0a88aec56a0..5e3d1c72d5e 100644
> --- a/gdb/testsuite/gdb.python/py-block.exp
> +++ b/gdb/testsuite/gdb.python/py-block.exp
> @@ -39,7 +39,7 @@ gdb_continue_to_breakpoint "Block break here."
>  gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
>  gdb_py_test_silent_cmd "python block = frame.block()" \
>      "Get block, initial innermost block" 0
> -gdb_test "python print (block)" "<gdb.Block object at $hex>" "check block not None"
> +gdb_test "python print (block)" "<gdb.Block .* \{.*\}>" "check block not None"

I think I'd be tempted to at least replace the first '.*' with the
function name we expect to see.  Might as well make the test as accurate
as possible.

>  gdb_test "python print (block.function)" "None" "first anonymous block"
>  gdb_test "python print (block.start)" "${decimal}" "check start not None"
>  gdb_test "python print (block.end)" "${decimal}" "check end not None"
> @@ -73,7 +73,7 @@ gdb_test "python print (block.function)" "block_func" \
>  gdb_test "up" ".*"
>  gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
>  gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
> -gdb_test "python print (block)" "<gdb.Block object at $hex>" \
> +gdb_test "python print (repr (block))" "<gdb.Block .* \{.*\}>" \

Same again where with the function name.

>           "Check Frame 2's block not None"
>  gdb_test "python print (block.function)" "main" "main block"
>  
> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
> index e36e87dc291..4da46431a3a 100644
> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
> @@ -50,11 +50,13 @@ proc_with_prefix test_bkpt_basic { } {
>  	return 0
>      }
>  
> +    set repr_pattern "<gdb.Breakpoint number=-?$decimal hits=-?$decimal\( thread=$decimal\)?\( task=$decimal\)?\( enable_count=$decimal\)?>"
> +
>      # Now there should be one breakpoint: main.
>      gdb_py_test_silent_cmd "python blist = gdb.breakpoints()" \
>  	"Get Breakpoint List" 0
> -    gdb_test "python print (blist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check obj exists @main"
> +    gdb_test "python print (repr (blist\[0\]))" \
> +	"$repr_pattern" "Check obj exists @main"
>      gdb_test "python print (blist\[0\].location)" \
>  	"main." "Check breakpoint location @main"
>      gdb_test "python print (blist\[0\].pending)" "False" \
> @@ -71,12 +73,12 @@ proc_with_prefix test_bkpt_basic { } {
>  	"Get Breakpoint List" 0
>      gdb_test "python print (len(blist))" \
>  	"2" "Check for two breakpoints"
> -    gdb_test "python print (blist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check obj exists @main 2"
> +    gdb_test "python print (repr (blist\[0\]))" \
> +	"$repr_pattern" "Check obj exists @main 2"
>      gdb_test "python print (blist\[0\].location)" \
>  	"main." "Check breakpoint location @main 2"
> -    gdb_test "python print (blist\[1\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check obj exists @mult_line"
> +    gdb_test "python print (repr (blist\[1\]))" \
> +	"$repr_pattern" "Check obj exists @mult_line"
>  
>      gdb_test "python print (blist\[1\].location)" \
>  	"py-breakpoint\.c:${mult_line}*" \
> @@ -224,14 +226,16 @@ proc_with_prefix test_bkpt_invisible { } {
>  	return 0
>      }
>  
> +    set repr_pattern "<gdb.Breakpoint number=-?$decimal hits=-?$decimal\( thread=$decimal\)?\( task=$decimal\)?\( enable_count=$decimal\)?>"
> +
>      delete_breakpoints
>      set ibp_location [gdb_get_line_number "Break at multiply."]
>      gdb_py_test_silent_cmd  "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=False)" \
>  	"Set invisible breakpoint" 0
>      gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
>  	"Get Breakpoint List" 0
> -    gdb_test "python print (ilist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 1"
> +    gdb_test "python print (repr (ilist\[0\]))" \
> +	"$repr_pattern" "Check invisible bp obj exists 1"
>      gdb_test "python print (ilist\[0\].location)" \
>  	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 1"
>      gdb_test "python print (ilist\[0\].visible)" \
> @@ -243,8 +247,8 @@ proc_with_prefix test_bkpt_invisible { } {
>  	"Set invisible breakpoint" 0
>      gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
>  	"Get Breakpoint List" 0
> -    gdb_test "python print (ilist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 2"
> +    gdb_test "python print (repr (ilist\[0\]))" \
> +	"$repr_pattern" "Check invisible bp obj exists 2"
>      gdb_test "python print (ilist\[0\].location)" \
>  	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 2"
>      gdb_test "python print (ilist\[0\].visible)" \
> diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
> index ad06b07c2c6..979b7dfb8fb 100644
> --- a/gdb/testsuite/gdb.python/py-symbol.exp
> +++ b/gdb/testsuite/gdb.python/py-symbol.exp
> @@ -44,6 +44,8 @@ clean_restart ${binfile}
>  # point where we don't have a current frame, and we don't want to
>  # require one.
>  gdb_py_test_silent_cmd "python main_func = gdb.lookup_global_symbol(\"main\")" "Lookup main" 1
> +gdb_test "python print (repr (main_func))" "<gdb.Symbol print_name=.*>" \

I think replace '.*' with 'main' here.

> +    "test main_func.__repr__"
>  gdb_test "python print (main_func.is_function)" "True" "test main_func.is_function"
>  gdb_test "python print (gdb.lookup_global_symbol(\"junk\"))" "None" "test lookup_global_symbol(\"junk\")"
>  
> diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
> index 594c9749d8e..95cdfa54a6e 100644
> --- a/gdb/testsuite/gdb.python/py-type.exp
> +++ b/gdb/testsuite/gdb.python/py-type.exp
> @@ -393,3 +393,7 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
>        test_type_equality
>    }
>  }
> +
> +# Test __repr__()

Missing '.' at the end of the comment.

> +gdb_test "python print (repr (gdb.lookup_type ('char')))" \
> +      "<gdb.Type code=TYPE_CODE_INT name=char>" "test __repr__()"

Thanks
Andrew

---

diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index a9d45b75dc9..c642be4208d 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -437,11 +437,10 @@ blpy_repr (PyObject *self)
     block->function ()->print_name () : "<anonymous>";
 
   block_iterator iter;
-  block_iterator_first (block, &iter);
-
-  std::string str;
   const struct symbol *symbol;
-  while ((symbol = block_iterator_next (&iter)) != nullptr)
+  std::string str;
+
+  ALL_BLOCK_SYMBOLS (block, iter, symbol)
     str = (str + "\n") + symbol->print_name () + ",";
   if(!str.empty ())
     str += "\n";
diff --git a/gdb/testsuite/gdb.python/py-block.c b/gdb/testsuite/gdb.python/py-block.c
index a0c6e165605..4c503827347 100644
--- a/gdb/testsuite/gdb.python/py-block.c
+++ b/gdb/testsuite/gdb.python/py-block.c
@@ -30,9 +30,14 @@ int block_func (void)
   }
 }
 
+int
+no_locals_func (void)
+{
+  return block_func ();
+}
 
 int main (int argc, char *argv[])
 {
-  block_func ();
+  no_locals_func ();
   return 0; /* Break at end. */
 }
diff --git a/gdb/testsuite/gdb.python/py-block.exp b/gdb/testsuite/gdb.python/py-block.exp
index 5cca798eeb3..3ac8c97dc69 100644
--- a/gdb/testsuite/gdb.python/py-block.exp
+++ b/gdb/testsuite/gdb.python/py-block.exp
@@ -68,15 +68,22 @@ gdb_test_no_output "python block = block.superblock" "get superblock 2"
 gdb_test "python print (block.function)" "block_func" \
          "Print superblock 2 function"
 
+gdb_test "up" ".*" "up to no_locals_func"
+gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
+gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
+gdb_test "python print (repr (block))" "<gdb.Block no_locals_func \{\}>" \
+    "Check block in empty_frame"
+gdb_test "python print (block.function)" "no_locals_func" \
+    "no_locals_func block"
+
 # Switch frames, then test for main block.
-gdb_test "up" ".*"
+gdb_test "up" ".*" "up to main"
 gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
 gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
 gdb_test "python print (repr (block))" "<gdb.Block .* \{.*\}>" \
          "Check Frame 2's block not None"
 gdb_test "python print (block.function)" "main" "main block"
 
-
 # Test Block is_valid.  This must always be the last test in this
 # testcase as it unloads the object file.
 delete_breakpoints


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Add __repr__() implementation to a few Python types
  2023-01-20  1:43     ` Matheus Branco Borella
  2023-01-20 16:45       ` Andrew Burgess
@ 2023-01-24 14:45       ` Andrew Burgess
  2023-05-18  3:33         ` Matheus Branco Borella
  1 sibling, 1 reply; 15+ messages in thread
From: Andrew Burgess @ 2023-01-24 14:45 UTC (permalink / raw)
  To: Matheus Branco Borella via Gdb-patches, gdb-patches
  Cc: Matheus Branco Borella

Matheus Branco Borella via Gdb-patches <gdb-patches@sourceware.org>
writes:

> Only a few types in the Python API currently have __repr__() implementations.
> This patch adds a few more of them. specifically: it adds __repr__()
> implementations to gdb.Symbol, gdb.Architecture, gdb.Block, gdb.Breakpoint,
> and gdb.Type.
>
> This makes it easier to play around the GDB Python API in the Python interpreter
> session invoked with the 'pi' command in GDB, giving more easily accessible tipe
> information to users.
>
> An example of how this would look like:
> ```
> (gdb) pi
>>> gdb.lookup_type("char")
> <gdb.Type code=TYPE_CODE_INT name=char>
>>> gdb.lookup_global_symbol("main")
> <gdb.Symbol print_name=main>
> ```
>
>> Sorry for being a little slow.  What does this actually mean?  When you
>> say "makes use of u8 string literals" - does this mean you have string
>> literals in this patch containing non ASCII characters?
>>
>> I've trying to understand why this is different to any other part of GDB
>> that prints stuff via Python.
>
> I forgot to take that out of the commit message, my bad. Originally, I'd 
> intended for the string literals in the patch that get handed to Python to be
> all u8 literals so that I could guarantee it wouldn't break in an environment
> that doesn't output regular string literals in an ASCII-compatible encoding,
> as Python expects all strings handed to it to be encoded in UTF-8. But seeing
> as all of the rest of the Python interface code uses regular string literals, 
> I figured it wouldn't make much of difference having them in anyway.
>
>> I guess I was surprised that so many of the new tests included an
>> explicit call to repr, given the premise of the change was that simply
>> 'print(empty)' would now print something useful.
>>
>> I guess maybe it doesn't hurt to _also_ include some explicit repr
>> calls, but I was expecting most tests to just be printing the object
>> directly.
> As blarsen@ also pointed out, `print`-ing an object directly that has an 
> implmentation of __str__() will print whatever its __str__() functions returns, 
> regardless of whether it implements __repr__() or not, which is not what we want 
> here. __repr__() is always preferred in the REPL, though, so it's understandable 
> it might not be clear at first why I'm calling `repr()` explicitly.
>
>> Over long line, please wrap a little.  There's other long lines in your
>> patch, I'll not point out each one.
>
> Should be all fixed now (hopefully I didn't miss any), with the exception of the
> `repr_pattern` strings in `py-breakpoint.exp`, which I couldn't for the life of 
> me get to match properly with the output were they not on a single line.
>
> ---
>  gdb/python/py-arch.c                       | 18 +++++-
>  gdb/python/py-block.c                      | 27 ++++++++-
>  gdb/python/py-breakpoint.c                 | 68 +++++++++++++++++++++-
>  gdb/python/py-symbol.c                     | 16 ++++-
>  gdb/python/py-type.c                       | 30 +++++++++-
>  gdb/testsuite/gdb.python/py-arch.exp       |  6 ++
>  gdb/testsuite/gdb.python/py-block.exp      |  4 +-
>  gdb/testsuite/gdb.python/py-breakpoint.exp | 24 ++++----
>  gdb/testsuite/gdb.python/py-symbol.exp     |  2 +
>  gdb/testsuite/gdb.python/py-type.exp       |  4 ++
>  10 files changed, 181 insertions(+), 18 deletions(-)
>
> diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
> index cf0978560f9..5384a0d0d0c 100644
> --- a/gdb/python/py-arch.c
> +++ b/gdb/python/py-arch.c
> @@ -319,6 +319,22 @@ archpy_integer_type (PyObject *self, PyObject *args, PyObject *kw)
>    return type_to_type_object (type);
>  }
>  
> +/* __repr__ implementation for gdb.Architecture.  */
> +
> +static PyObject *
> +archpy_repr (PyObject *self)
> +{
> +  const auto gdbarch = arch_object_to_gdbarch (self);
> +  if (gdbarch == nullptr)
> +    return PyUnicode_FromFormat
> +      ("<gdb.Architecture (invalid)>");

Additionally, I think that instead of hard-coding gdb.Architecture, we
should do:

    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);

The benefit being that if a user sub-classes gdb.Architecture, and
doesn't override the __repr__ method, then the name printed will be the
name of the sub-class, rather than the base-class.

This obviously applies throughout this patch.

Thanks,
Andrew

> +
> +  return PyUnicode_FromFormat
> +    ("<gdb.Architecture arch_name=%s printable_name=%s>",
> +     gdbarch_bfd_arch_info (gdbarch)->arch_name,
> +     gdbarch_bfd_arch_info (gdbarch)->printable_name);
> +}
> +
>  /* Implementation of gdb.architecture_names().  Return a list of all the
>     BFD architecture names that GDB understands.  */
>  
> @@ -391,7 +407,7 @@ PyTypeObject arch_object_type = {
>    0,                                  /* tp_getattr */
>    0,                                  /* tp_setattr */
>    0,                                  /* tp_compare */
> -  0,                                  /* tp_repr */
> +  archpy_repr,                        /* tp_repr */
>    0,                                  /* tp_as_number */
>    0,                                  /* tp_as_sequence */
>    0,                                  /* tp_as_mapping */
> diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
> index b9aea3aca69..b4c55add765 100644
> --- a/gdb/python/py-block.c
> +++ b/gdb/python/py-block.c
> @@ -424,6 +424,31 @@ blpy_iter_is_valid (PyObject *self, PyObject *args)
>    Py_RETURN_TRUE;
>  }
>  
> +/* __repr__ implementation for gdb.Block.  */
> +
> +static PyObject *
> +blpy_repr (PyObject *self)
> +{
> +  const auto block = block_object_to_block (self);
> +  if (block == nullptr)
> +    return PyUnicode_FromFormat ("<gdb.Block (invalid)>");
> +
> +  const auto name = block->function () ?
> +    block->function ()->print_name () : "<anonymous>";
> +
> +  block_iterator iter;
> +  block_iterator_first (block, &iter);
> +
> +  std::string str;
> +  const struct symbol *symbol;
> +  while ((symbol = block_iterator_next (&iter)) != nullptr)
> +    str = (str + "\n") + symbol->print_name () + ",";
> +  if(!str.empty ())
> +    str += "\n";
> +
> +  return PyUnicode_FromFormat ("<gdb.Block %s {%s}>", name, str.c_str ());
> +}
> +
>  int
>  gdbpy_initialize_blocks (void)
>  {
> @@ -486,7 +511,7 @@ PyTypeObject block_object_type = {
>    0,				  /*tp_getattr*/
>    0,				  /*tp_setattr*/
>    0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  blpy_repr,                     /*tp_repr*/
>    0,				  /*tp_as_number*/
>    0,				  /*tp_as_sequence*/
>    &block_object_as_mapping,	  /*tp_as_mapping*/
> diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
> index de7b9f4266b..d68a205330c 100644
> --- a/gdb/python/py-breakpoint.c
> +++ b/gdb/python/py-breakpoint.c
> @@ -33,6 +33,7 @@
>  #include "location.h"
>  #include "py-event.h"
>  #include "linespec.h"
> +#include "gdbsupport/common-utils.h"
>  
>  extern PyTypeObject breakpoint_location_object_type
>      CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("breakpoint_location_object");
> @@ -967,6 +968,31 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>    return 0;
>  }
>  
> +/* __repr__ implementation for gdb.Breakpoint.  */
> +
> +static PyObject *
> +bppy_repr (PyObject *self)
> +{
> +  const auto bp = (struct gdbpy_breakpoint_object*) self;
> +  if (bp->bp == nullptr)
> +    return PyUnicode_FromFormat ("<gdb.Breakpoint (invalid)>");
> +
> +  std::string str = " ";
> +  if (bp->bp->thread != -1)
> +    str += string_printf ("thread=%d ", bp->bp->thread);
> +  if (bp->bp->task > 0)
> +    str += string_printf ("task=%d ", bp->bp->task);
> +  if (bp->bp->enable_count > 0)
> +    str += string_printf ("enable_count=%d ", bp->bp->enable_count);
> +  str.pop_back ();
> +
> +  return PyUnicode_FromFormat
> +    ("<gdb.Breakpoint number=%d hits=%d%s>",
> +     bp->bp->number,
> +     bp->bp->hit_count,
> +     str.c_str ());
> +}
> +
>  /* Append to LIST the breakpoint Python object associated to B.
>  
>     Return true on success.  Return false on failure, with the Python error
> @@ -1389,7 +1415,7 @@ PyTypeObject breakpoint_object_type =
>    0,				  /*tp_getattr*/
>    0,				  /*tp_setattr*/
>    0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  bppy_repr,                     /*tp_repr*/
>    0,				  /*tp_as_number*/
>    0,				  /*tp_as_sequence*/
>    0,				  /*tp_as_mapping*/
> @@ -1604,6 +1630,44 @@ bplocpy_dealloc (PyObject *py_self)
>    Py_TYPE (py_self)->tp_free (py_self);
>  }
>  
> +/* __repr__ implementation for gdb.BreakpointLocation.  */
> +
> +static PyObject *
> +bplocpy_repr (PyObject *py_self)
> +{
> +  const auto self = (gdbpy_breakpoint_location_object *) py_self;
> +  if (self->owner == nullptr || self->owner->bp == nullptr
> +    || self->owner->bp != self->bp_loc->owner)
> +    return PyUnicode_FromFormat ("<gdb.BreakpointLocation (invalid)>");
> +
> +  const auto enabled = self->bp_loc->enabled ? "enabled" : "disabled";
> +
> +  std::string str(enabled);
> +
> +  str += " requested_address=0x";
> +  str += string_printf ("%lx", self->bp_loc->requested_address);
> +
> +  str += " address=0x";
> +  str += string_printf ("%lx", self->bp_loc->address);
> +
> +  if (self->bp_loc->symtab != nullptr)
> +  {
> +    str += " source=";
> +    str += self->bp_loc->symtab->filename;
> +    str += ":";
> +    str += string_printf ("%d", self->bp_loc->line_number);
> +  }
> +
> +  const auto fn_name = self->bp_loc->function_name.get ();
> +  if (fn_name != nullptr)
> +  {
> +    str += " in ";
> +    str += fn_name;
> +  }
> +
> +  return PyUnicode_FromFormat ("<gdb.BreakpointLocation %s>", str.c_str ());
> +}
> +
>  /* Attribute get/set Python definitions. */
>  
>  static gdb_PyGetSetDef bp_location_object_getset[] = {
> @@ -1635,7 +1699,7 @@ PyTypeObject breakpoint_location_object_type =
>    0,					/*tp_getattr*/
>    0,					/*tp_setattr*/
>    0,					/*tp_compare*/
> -  0,					/*tp_repr*/
> +  bplocpy_repr,                        /*tp_repr*/
>    0,					/*tp_as_number*/
>    0,					/*tp_as_sequence*/
>    0,					/*tp_as_mapping*/
> diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> index 93c86964f3e..5a8149bbe66 100644
> --- a/gdb/python/py-symbol.c
> +++ b/gdb/python/py-symbol.c
> @@ -375,6 +375,20 @@ sympy_dealloc (PyObject *obj)
>    Py_TYPE (obj)->tp_free (obj);
>  }
>  
> +/* __repr__ implementation for gdb.Symbol.  */
> +
> +static PyObject *
> +sympy_repr (PyObject *self)
> +{
> +  const auto symbol = symbol_object_to_symbol (self);
> +  if (symbol == nullptr)
> +    return PyUnicode_FromFormat ("<gdb.Symbol (invalid)>");
> +
> +  return PyUnicode_FromFormat
> +    ("<gdb.Symbol print_name=%s>",
> +     symbol->print_name ());
> +}
> +
>  /* Implementation of
>     gdb.lookup_symbol (name [, block] [, domain]) -> (symbol, is_field_of_this)
>     A tuple with 2 elements is always returned.  The first is the symbol
> @@ -732,7 +746,7 @@ PyTypeObject symbol_object_type = {
>    0,				  /*tp_getattr*/
>    0,				  /*tp_setattr*/
>    0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  sympy_repr,                    /*tp_repr*/
>    0,				  /*tp_as_number*/
>    0,				  /*tp_as_sequence*/
>    0,				  /*tp_as_mapping*/
> diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
> index 928efacfe8a..eb11ef029ca 100644
> --- a/gdb/python/py-type.c
> +++ b/gdb/python/py-type.c
> @@ -1026,6 +1026,34 @@ typy_template_argument (PyObject *self, PyObject *args)
>    return value_to_value_object (val);
>  }
>  
> +/* __repr__ implementation for gdb.Type.  */
> +
> +static PyObject *
> +typy_repr (PyObject *self)
> +{
> +  const auto type = type_object_to_type (self);
> +  if (type == nullptr)
> +    return PyUnicode_FromFormat ("<gdb.Type (invalid)>");
> +
> +  const char *code = pyty_codes[type->code ()].name;
> +  string_file type_name;
> +  try
> +    {
> +      current_language->print_type (type, "",
> +				    &type_name, -1, 0,
> +				    &type_print_raw_options);
> +    }
> +  catch (const gdb_exception &except)
> +    {
> +      GDB_PY_HANDLE_EXCEPTION (except);
> +    }
> +  auto py_typename = PyUnicode_Decode
> +    (type_name.c_str (), type_name.size (),
> +		 host_charset (), NULL);
> +	
> +  return PyUnicode_FromFormat ("<gdb.Type code=%s name=%U>", code, py_typename);
> +}
> +
>  static PyObject *
>  typy_str (PyObject *self)
>  {
> @@ -1612,7 +1640,7 @@ PyTypeObject type_object_type =
>    0,				  /*tp_getattr*/
>    0,				  /*tp_setattr*/
>    0,				  /*tp_compare*/
> -  0,				  /*tp_repr*/
> +  typy_repr,                     /*tp_repr*/
>    &type_object_as_number,	  /*tp_as_number*/
>    0,				  /*tp_as_sequence*/
>    &typy_mapping,		  /*tp_as_mapping*/
> diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
> index 1fbbc47c872..d436c957e25 100644
> --- a/gdb/testsuite/gdb.python/py-arch.exp
> +++ b/gdb/testsuite/gdb.python/py-arch.exp
> @@ -29,6 +29,8 @@ if ![runto_main] {
>  # Test python/15461.  Invalid architectures should not trigger an
>  # internal GDB assert.
>  gdb_py_test_silent_cmd "python empty = gdb.Architecture()" "get empty arch" 0
> +gdb_test "python print(repr (empty))" "<gdb\\.Architecture \\(invalid\\)>" \
> +    "Test empty achitecture __repr__ does not trigger an assert"
>  gdb_test "python print(empty.name())" ".*Architecture is invalid.*" \
>      "Test empty architecture.name does not trigger an assert"
>  gdb_test "python print(empty.disassemble())" ".*Architecture is invalid.*" \
> @@ -46,6 +48,10 @@ gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
>  gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(gdb.Value(pc))" \
>    "disassemble no end no count" 0
>  
> +gdb_test "python print (repr (arch))" \
> +    "<gdb.Architecture arch_name=.* printable_name=.*>" \
> +    "test __repr__ for architecture"
> +
>  gdb_test "python print (len(insn_list1))" "1" "test number of instructions 1"
>  gdb_test "python print (len(insn_list2))" "1" "test number of instructions 2"
>  gdb_test "python print (len(insn_list3))" "1" "test number of instructions 3"
> diff --git a/gdb/testsuite/gdb.python/py-block.exp b/gdb/testsuite/gdb.python/py-block.exp
> index 0a88aec56a0..5e3d1c72d5e 100644
> --- a/gdb/testsuite/gdb.python/py-block.exp
> +++ b/gdb/testsuite/gdb.python/py-block.exp
> @@ -39,7 +39,7 @@ gdb_continue_to_breakpoint "Block break here."
>  gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
>  gdb_py_test_silent_cmd "python block = frame.block()" \
>      "Get block, initial innermost block" 0
> -gdb_test "python print (block)" "<gdb.Block object at $hex>" "check block not None"
> +gdb_test "python print (block)" "<gdb.Block .* \{.*\}>" "check block not None"
>  gdb_test "python print (block.function)" "None" "first anonymous block"
>  gdb_test "python print (block.start)" "${decimal}" "check start not None"
>  gdb_test "python print (block.end)" "${decimal}" "check end not None"
> @@ -73,7 +73,7 @@ gdb_test "python print (block.function)" "block_func" \
>  gdb_test "up" ".*"
>  gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
>  gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
> -gdb_test "python print (block)" "<gdb.Block object at $hex>" \
> +gdb_test "python print (repr (block))" "<gdb.Block .* \{.*\}>" \
>           "Check Frame 2's block not None"
>  gdb_test "python print (block.function)" "main" "main block"
>  
> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
> index e36e87dc291..4da46431a3a 100644
> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
> @@ -50,11 +50,13 @@ proc_with_prefix test_bkpt_basic { } {
>  	return 0
>      }
>  
> +    set repr_pattern "<gdb.Breakpoint number=-?$decimal hits=-?$decimal\( thread=$decimal\)?\( task=$decimal\)?\( enable_count=$decimal\)?>"
> +
>      # Now there should be one breakpoint: main.
>      gdb_py_test_silent_cmd "python blist = gdb.breakpoints()" \
>  	"Get Breakpoint List" 0
> -    gdb_test "python print (blist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check obj exists @main"
> +    gdb_test "python print (repr (blist\[0\]))" \
> +	"$repr_pattern" "Check obj exists @main"
>      gdb_test "python print (blist\[0\].location)" \
>  	"main." "Check breakpoint location @main"
>      gdb_test "python print (blist\[0\].pending)" "False" \
> @@ -71,12 +73,12 @@ proc_with_prefix test_bkpt_basic { } {
>  	"Get Breakpoint List" 0
>      gdb_test "python print (len(blist))" \
>  	"2" "Check for two breakpoints"
> -    gdb_test "python print (blist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check obj exists @main 2"
> +    gdb_test "python print (repr (blist\[0\]))" \
> +	"$repr_pattern" "Check obj exists @main 2"
>      gdb_test "python print (blist\[0\].location)" \
>  	"main." "Check breakpoint location @main 2"
> -    gdb_test "python print (blist\[1\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check obj exists @mult_line"
> +    gdb_test "python print (repr (blist\[1\]))" \
> +	"$repr_pattern" "Check obj exists @mult_line"
>  
>      gdb_test "python print (blist\[1\].location)" \
>  	"py-breakpoint\.c:${mult_line}*" \
> @@ -224,14 +226,16 @@ proc_with_prefix test_bkpt_invisible { } {
>  	return 0
>      }
>  
> +    set repr_pattern "<gdb.Breakpoint number=-?$decimal hits=-?$decimal\( thread=$decimal\)?\( task=$decimal\)?\( enable_count=$decimal\)?>"
> +
>      delete_breakpoints
>      set ibp_location [gdb_get_line_number "Break at multiply."]
>      gdb_py_test_silent_cmd  "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=False)" \
>  	"Set invisible breakpoint" 0
>      gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
>  	"Get Breakpoint List" 0
> -    gdb_test "python print (ilist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 1"
> +    gdb_test "python print (repr (ilist\[0\]))" \
> +	"$repr_pattern" "Check invisible bp obj exists 1"
>      gdb_test "python print (ilist\[0\].location)" \
>  	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 1"
>      gdb_test "python print (ilist\[0\].visible)" \
> @@ -243,8 +247,8 @@ proc_with_prefix test_bkpt_invisible { } {
>  	"Set invisible breakpoint" 0
>      gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
>  	"Get Breakpoint List" 0
> -    gdb_test "python print (ilist\[0\])" \
> -	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 2"
> +    gdb_test "python print (repr (ilist\[0\]))" \
> +	"$repr_pattern" "Check invisible bp obj exists 2"
>      gdb_test "python print (ilist\[0\].location)" \
>  	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 2"
>      gdb_test "python print (ilist\[0\].visible)" \
> diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
> index ad06b07c2c6..979b7dfb8fb 100644
> --- a/gdb/testsuite/gdb.python/py-symbol.exp
> +++ b/gdb/testsuite/gdb.python/py-symbol.exp
> @@ -44,6 +44,8 @@ clean_restart ${binfile}
>  # point where we don't have a current frame, and we don't want to
>  # require one.
>  gdb_py_test_silent_cmd "python main_func = gdb.lookup_global_symbol(\"main\")" "Lookup main" 1
> +gdb_test "python print (repr (main_func))" "<gdb.Symbol print_name=.*>" \
> +    "test main_func.__repr__"
>  gdb_test "python print (main_func.is_function)" "True" "test main_func.is_function"
>  gdb_test "python print (gdb.lookup_global_symbol(\"junk\"))" "None" "test lookup_global_symbol(\"junk\")"
>  
> diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
> index 594c9749d8e..95cdfa54a6e 100644
> --- a/gdb/testsuite/gdb.python/py-type.exp
> +++ b/gdb/testsuite/gdb.python/py-type.exp
> @@ -393,3 +393,7 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
>        test_type_equality
>    }
>  }
> +
> +# Test __repr__()
> +gdb_test "python print (repr (gdb.lookup_type ('char')))" \
> +      "<gdb.Type code=TYPE_CODE_INT name=char>" "test __repr__()"
> -- 
> 2.37.3.windows.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH] Add __repr__() implementation to a few Python types
  2023-01-24 14:45       ` Andrew Burgess
@ 2023-05-18  3:33         ` Matheus Branco Borella
  2023-05-19 21:27           ` [PATCHv3 0/2] " Andrew Burgess
  0 siblings, 1 reply; 15+ messages in thread
From: Matheus Branco Borella @ 2023-05-18  3:33 UTC (permalink / raw)
  To: gdb-patches

From: Matheus Branco Borella via Gdb-patches <gdb-patches@sourceware.org>

Only a few types in the Python API currently have __repr__() implementations.
This patch adds a few more of them. specifically: it adds __repr__()
implementations to gdb.Symbol, gdb.Architecture, gdb.Block, gdb.Breakpoint,
and gdb.Type.

This makes it easier to play around the GDB Python API in the Python interpreter
session invoked with the 'pi' command in GDB, giving more easily accessible tipe
information to users.

An example of how this would look like:
```
(gdb) pi
>> gdb.lookup_type("char")
<gdb.Type code=TYPE_CODE_INT name=char>
>> gdb.lookup_global_symbol("main")
<gdb.Symbol print_name=main>
```

Okay, this should have all of the changes suggested in the replies. Sorry it
took me this long to get back to this. Life happened. I also made it so that
no more than five symbols get written out by `gdb.Block`, showing how many were
elided, if any. This should be a better compromise between expressiveness and
clutter than the first implementation. This patch makes use of `type_allocator`
for type initialization and of `block_iterator_range` for block symbol
iteration.
---
 gdb/python/py-arch.c                       | 17 ++++-
 gdb/python/py-block.c                      | 47 +++++++++++++-
 gdb/python/py-breakpoint.c                 | 75 +++++++++++++++++++++-
 gdb/python/py-symbol.c                     | 17 ++++-
 gdb/python/py-type.c                       | 32 ++++++++-
 gdb/testsuite/gdb.python/py-arch.exp       |  6 ++
 gdb/testsuite/gdb.python/py-block.c        | 28 ++++++++
 gdb/testsuite/gdb.python/py-block.exp      | 37 ++++++++++-
 gdb/testsuite/gdb.python/py-breakpoint.exp | 24 ++++---
 gdb/testsuite/gdb.python/py-symbol.exp     |  2 +
 gdb/testsuite/gdb.python/py-type.exp       |  4 ++
 11 files changed, 270 insertions(+), 19 deletions(-)

diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index 4d133d1fe1..44df6db545 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -319,6 +319,21 @@ archpy_integer_type (PyObject *self, PyObject *args, PyObject *kw)
   return type_to_type_object (type);
 }
 
+/* __repr__ implementation for gdb.Architecture.  */
+
+static PyObject *
+archpy_repr (PyObject *self)
+{
+  const auto gdbarch = arch_object_to_gdbarch (self);
+  if (gdbarch == nullptr)
+    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+
+  return PyUnicode_FromFormat ("<%s arch_name=%s printable_name=%s>",
+                               Py_TYPE (self)->tp_name,
+                               gdbarch_bfd_arch_info (gdbarch)->arch_name,
+                               gdbarch_bfd_arch_info (gdbarch)->printable_name);
+}
+
 /* Implementation of gdb.architecture_names().  Return a list of all the
    BFD architecture names that GDB understands.  */
 
@@ -395,7 +410,7 @@ PyTypeObject arch_object_type = {
   0,                                  /* tp_getattr */
   0,                                  /* tp_setattr */
   0,                                  /* tp_compare */
-  0,                                  /* tp_repr */
+  archpy_repr,                        /* tp_repr */
   0,                                  /* tp_as_number */
   0,                                  /* tp_as_sequence */
   0,                                  /* tp_as_mapping */
diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index 09fa74d862..6bca88d2fb 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -418,6 +418,51 @@ blpy_iter_is_valid (PyObject *self, PyObject *args)
   Py_RETURN_TRUE;
 }
 
+/* __repr__ implementation for gdb.Block.  */
+
+static PyObject *
+blpy_repr (PyObject *self)
+{
+  const auto block = block_object_to_block (self);
+  if (block == nullptr)
+    return PyUnicode_FromFormat ("<%s (invalid)>", 
+                                 Py_TYPE (self)->tp_name);
+
+  const auto name = block->function () ?
+    block->function ()->print_name () : "<anonymous>";
+
+  std::string str;
+  unsigned int written_symbols = 0;
+  const int len = mdict_size (block->multidict ());
+  for (struct symbol *symbol : block_iterator_range (block))
+    {
+      if(++written_symbols >= 6)
+        {
+          const int remaining = len - 5;
+          if (remaining == 1)
+            {
+              str = (str + "\n    ") + 
+                string_printf("... (%d more symbol)", len - 5);
+            }
+          else
+            {
+              str = (str + "\n    ") + 
+                string_printf("... (%d more symbols)", len - 5);
+            }
+
+          break;
+        }
+      str = (str + "\n    ") + symbol->print_name ();
+      if (written_symbols < len)
+        str = str + ",";
+    }
+  if(!str.empty ())
+    str += "\n";
+
+  return PyUnicode_FromFormat ("<%s %s {%s}>", Py_TYPE (self)->tp_name, 
+                               name, str.c_str ());
+}
+
 static int CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION
 gdbpy_initialize_blocks (void)
 {
@@ -482,7 +527,7 @@ PyTypeObject block_object_type = {
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  blpy_repr,                      /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   &block_object_as_mapping,	  /*tp_as_mapping*/
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index becb04c91c..1dd746ba94 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -33,6 +33,7 @@
 #include "location.h"
 #include "py-event.h"
 #include "linespec.h"
+#include "gdbsupport/common-utils.h"
 
 extern PyTypeObject breakpoint_location_object_type
     CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("breakpoint_location_object");
@@ -981,6 +982,32 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
   return 0;
 }
 
+/* __repr__ implementation for gdb.Breakpoint.  */
+
+static PyObject *
+bppy_repr (PyObject *self)
+{
+  const auto bp = (struct gdbpy_breakpoint_object*) self;
+  if (bp->bp == nullptr)
+    return PyUnicode_FromFormat ("<%s (invalid)>", 
+                                 Py_TYPE (self)->tp_name);
+
+  std::string str = " ";
+  if (bp->bp->thread != -1)
+    str += string_printf ("thread=%d ", bp->bp->thread);
+  if (bp->bp->task > 0)
+    str += string_printf ("task=%d ", bp->bp->task);
+  if (bp->bp->enable_count > 0)
+    str += string_printf ("enable_count=%d ", 
+                          bp->bp->enable_count);
+  str.pop_back ();
+
+  return PyUnicode_FromFormat ("<%s number=%d hits=%d%s>",
+                               Py_TYPE (self)->tp_name,
+                               bp->bp->number, bp->bp->hit_count,
+                               str.c_str ());
+}
+
 /* Append to LIST the breakpoint Python object associated to B.
 
    Return true on success.  Return false on failure, with the Python error
@@ -1406,7 +1433,7 @@ PyTypeObject breakpoint_object_type =
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  bppy_repr,                     /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   0,				  /*tp_as_mapping*/
@@ -1624,6 +1651,50 @@ bplocpy_dealloc (PyObject *py_self)
   Py_TYPE (py_self)->tp_free (py_self);
 }
 
+/* __repr__ implementation for gdb.BreakpointLocation.  */
+
+static PyObject *
+bplocpy_repr (PyObject *py_self)
+{
+  const auto self = (gdbpy_breakpoint_location_object *) py_self;
+  if (self->owner == nullptr || self->owner->bp == nullptr
+    || self->owner->bp != self->bp_loc->owner)
+    return PyUnicode_FromFormat ("<%s (invalid)>", 
+                                 Py_TYPE (self)->tp_name);
+
+  const auto enabled = self->bp_loc->enabled ? "enabled" : "disabled";
+
+  std::string str(enabled);
+
+  str += " address=0x";
+  str += string_printf 
+    ("%s", core_addr_to_string_nz (self->bp_loc->address));
+
+  if (self->bp_loc->requested_address != self->bp_loc->address)
+    {
+      str += " requested_address=0x";
+      str += string_printf 
+        ("%s", core_addr_to_string_nz (self->bp_loc->requested_address));
+    }
+  if (self->bp_loc->symtab != nullptr)
+    {
+      str += " source=";
+      str += self->bp_loc->symtab->filename;
+      str += ":";
+      str += string_printf ("%d", self->bp_loc->line_number);
+    }
+
+  const auto fn_name = self->bp_loc->function_name.get ();
+  if (fn_name != nullptr)
+    {
+      str += " in ";
+      str += fn_name;
+    }
+
+  return PyUnicode_FromFormat ("<%s %s>", Py_TYPE (self)->tp_name, 
+                               str.c_str ());
+}
+
 /* Attribute get/set Python definitions. */
 
 static gdb_PyGetSetDef bp_location_object_getset[] = {
@@ -1655,7 +1726,7 @@ PyTypeObject breakpoint_location_object_type =
   0,					/*tp_getattr*/
   0,					/*tp_setattr*/
   0,					/*tp_compare*/
-  0,					/*tp_repr*/
+  bplocpy_repr,                        /*tp_repr*/
   0,					/*tp_as_number*/
   0,					/*tp_as_sequence*/
   0,					/*tp_as_mapping*/
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index ff3d18504e..6cf9825d7f 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -378,6 +378,21 @@ sympy_dealloc (PyObject *obj)
   Py_TYPE (obj)->tp_free (obj);
 }
 
+/* __repr__ implementation for gdb.Symbol.  */
+
+static PyObject *
+sympy_repr (PyObject *self)
+{
+  const auto symbol = symbol_object_to_symbol (self);
+  if (symbol == nullptr)
+    return PyUnicode_FromFormat ("<%s (invalid)>", 
+                                 Py_TYPE (self)->tp_name);
+
+  return PyUnicode_FromFormat ("<%s print_name=%s>",
+                               Py_TYPE (self)->tp_name,
+                               symbol->print_name ());
+}
+
 /* Implementation of
    gdb.lookup_symbol (name [, block] [, domain]) -> (symbol, is_field_of_this)
    A tuple with 2 elements is always returned.  The first is the symbol
@@ -741,7 +756,7 @@ PyTypeObject symbol_object_type = {
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  sympy_repr,                    /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   0,				  /*tp_as_mapping*/
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index b9fa741177..283d595546 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -1028,6 +1028,36 @@ typy_template_argument (PyObject *self, PyObject *args)
   return result;
 }
 
+/* __repr__ implementation for gdb.Type.  */
+
+static PyObject *
+typy_repr (PyObject *self)
+{
+  const auto type = type_object_to_type (self);
+  if (type == nullptr)
+    return PyUnicode_FromFormat ("<%s (invalid)>", 
+                                 Py_TYPE (self)->tp_name);
+
+  const char *code = pyty_codes[type->code ()].name;
+  string_file type_name;
+  try
+    {
+      current_language->print_type (type, "", &type_name, -1, 0, 
+                                    &type_print_raw_options);
+    }
+  catch (const gdb_exception &except)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+    }
+  auto py_typename = PyUnicode_Decode (type_name.c_str (), 
+                                       type_name.size (),
+                                       host_charset (), NULL);
+
+  return PyUnicode_FromFormat ("<%s code=%s name=%U>", 
+                               Py_TYPE (self)->tp_name, code, 
+                               py_typename);
+}
+
 static PyObject *
 typy_str (PyObject *self)
 {
@@ -1617,7 +1647,7 @@ PyTypeObject type_object_type =
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  typy_repr,                     /*tp_repr*/
   &type_object_as_number,	  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   &typy_mapping,		  /*tp_as_mapping*/
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
index 4f4b4aa766..597943ff68 100644
--- a/gdb/testsuite/gdb.python/py-arch.exp
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -27,6 +27,8 @@ if ![runto_main] {
 # Test python/15461.  Invalid architectures should not trigger an
 # internal GDB assert.
 gdb_py_test_silent_cmd "python empty = gdb.Architecture()" "get empty arch" 0
+gdb_test "python print(repr (empty))" "<gdb\\.Architecture \\(invalid\\)>" \
+    "Test empty achitecture __repr__ does not trigger an assert"
 gdb_test "python print(empty.name())" ".*Architecture is invalid.*" \
     "Test empty architecture.name does not trigger an assert"
 gdb_test "python print(empty.disassemble())" ".*Architecture is invalid.*" \
@@ -44,6 +46,10 @@ gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
 gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(gdb.Value(pc))" \
   "disassemble no end no count" 0
 
+gdb_test "python print (repr (arch))" \
+    "<gdb.Architecture arch_name=.* printable_name=.*>" \
+    "test __repr__ for architecture"
+
 gdb_test "python print (len(insn_list1))" "1" "test number of instructions 1"
 gdb_test "python print (len(insn_list2))" "1" "test number of instructions 2"
 gdb_test "python print (len(insn_list3))" "1" "test number of instructions 3"
diff --git a/gdb/testsuite/gdb.python/py-block.c b/gdb/testsuite/gdb.python/py-block.c
index a0c6e16560..7e26f2d1fe 100644
--- a/gdb/testsuite/gdb.python/py-block.c
+++ b/gdb/testsuite/gdb.python/py-block.c
@@ -30,9 +30,37 @@ int block_func (void)
   }
 }
 
+int no_locals_func (void)
+{
+  return block_func ();
+}
+
+int few_locals_func (void)
+{
+  int i = 0;
+  int j = 0;
+  int k = 0;
+  int x = 0;
+  int y = 0; 
+  return block_func ();
+}
+
+int many_locals_func (void)
+{
+  int i = 0;
+  int j = 0;
+  int k = 0;
+  int x = 0;
+  int y = 0; 
+  int z = 0; 
+  return block_func ();
+}
 
 int main (int argc, char *argv[])
 {
   block_func ();
+  no_locals_func ();
+  few_locals_func ();
+  many_locals_func ();
   return 0; /* Break at end. */
 }
diff --git a/gdb/testsuite/gdb.python/py-block.exp b/gdb/testsuite/gdb.python/py-block.exp
index 3bdf97294a..dc2f2f53af 100644
--- a/gdb/testsuite/gdb.python/py-block.exp
+++ b/gdb/testsuite/gdb.python/py-block.exp
@@ -38,7 +38,7 @@ gdb_continue_to_breakpoint "Block break here."
 gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
 gdb_py_test_silent_cmd "python block = frame.block()" \
     "Get block, initial innermost block" 0
-gdb_test "python print (block)" "<gdb.Block object at $hex>" "check block not None"
+gdb_test "python print (block)" "<gdb.Block <anonymous> \{.*\}>" "check block not None"
 gdb_test "python print (block.function)" "None" "first anonymous block"
 gdb_test "python print (block.start)" "${decimal}" "check start not None"
 gdb_test "python print (block.end)" "${decimal}" "check end not None"
@@ -68,15 +68,46 @@ gdb_test_no_output "python block = block.superblock" "get superblock 2"
 gdb_test "python print (block.function)" "block_func" \
          "Print superblock 2 function"
 
+# Switch frames, then test block for no_locals_func.
+gdb_test "continue" ".*" "continue to no_locals_func breakpoint"
+gdb_test "up" ".*" "up to no_locals_func"
+gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
+gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
+gdb_test "python print (repr (block))" "<gdb.Block no_locals_func \{\}>" \
+    "Check block in no_locals_func"
+gdb_test "python print (block.function)" "no_locals_func" \
+    "no_locals_func block"
+
+# Switch frames, then test block for few_locals_func.
+gdb_test "continue" ".*" "continue to few_locals_func breakpoint"
+gdb_test "up" ".*" "up to few_locals_func"
+gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
+gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
+gdb_test "python print (repr (block))" \
+    "<gdb.Block few_locals_func \{.*i,.*j,.*k,.*x,.*y.*\}>" \
+    "Check block in few_locals_func"
+gdb_test "python print (block.function)" "few_locals_func" \
+    "few_locals_func block"
+
+# Switch frames, then test block for many_locals_func.
+gdb_test "continue" ".*" "continue to many_locals_func breakpoint"
+gdb_test "up" ".*" "up to many_locals_func"
+gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
+gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
+gdb_test "python print (repr (block))" \
+    "<gdb.Block many_locals_func \{.*i,.*j,.*k,.*x,.*y,.*\.\.\. \\(1 more symbol\\).*\}>" \
+    "Check block in many_locals_func"
+gdb_test "python print (block.function)" "many_locals_func" \
+    "many_locals_func block"
+
 # Switch frames, then test for main block.
 gdb_test "up" ".*"
 gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
 gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
-gdb_test "python print (block)" "<gdb.Block object at $hex>" \
+gdb_test "python print (repr (block))" "<gdb.Block main \{.*\}>" \
          "Check Frame 2's block not None"
 gdb_test "python print (block.function)" "main" "main block"
 
-
 # Test Block is_valid.  This must always be the last test in this
 # testcase as it unloads the object file.
 delete_breakpoints
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index 76094c95d1..77e44f6e3c 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -51,11 +51,13 @@ proc_with_prefix test_bkpt_basic { } {
 	return 0
     }
 
+    set repr_pattern "<gdb.Breakpoint number=-?$decimal hits=-?$decimal\( thread=$decimal\)?\( task=$decimal\)?\( enable_count=$decimal\)?>"
+
     # Now there should be one breakpoint: main.
     gdb_py_test_silent_cmd "python blist = gdb.breakpoints()" \
 	"Get Breakpoint List" 0
-    gdb_test "python print (blist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check obj exists @main"
+    gdb_test "python print (repr (blist\[0\]))" \
+	"$repr_pattern" "Check obj exists @main"
     gdb_test "python print (blist\[0\].location)" \
 	"main" "Check breakpoint location @main"
     gdb_test "python print (blist\[0\].pending)" "False" \
@@ -72,12 +74,12 @@ proc_with_prefix test_bkpt_basic { } {
 	"Get Breakpoint List" 0
     gdb_test "python print (len(blist))" \
 	"2" "Check for two breakpoints"
-    gdb_test "python print (blist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check obj exists @main 2"
+    gdb_test "python print (repr (blist\[0\]))" \
+	"$repr_pattern" "Check obj exists @main 2"
     gdb_test "python print (blist\[0\].location)" \
 	"main" "Check breakpoint location @main 2"
-    gdb_test "python print (blist\[1\])" \
-	"<gdb.Breakpoint object at $hex>" "Check obj exists @mult_line"
+    gdb_test "python print (repr (blist\[1\]))" \
+	"$repr_pattern" "Check obj exists @mult_line"
 
     gdb_test "python print (blist\[1\].location)" \
 	"py-breakpoint\.c:${mult_line}*" \
@@ -225,14 +227,16 @@ proc_with_prefix test_bkpt_invisible { } {
 	return 0
     }
 
+    set repr_pattern "<gdb.Breakpoint number=-?$decimal hits=-?$decimal\( thread=$decimal\)?\( task=$decimal\)?\( enable_count=$decimal\)?>"
+
     delete_breakpoints
     set ibp_location [gdb_get_line_number "Break at multiply."]
     gdb_py_test_silent_cmd  "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=False)" \
 	"Set invisible breakpoint" 0
     gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
 	"Get Breakpoint List" 0
-    gdb_test "python print (ilist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 1"
+    gdb_test "python print (repr (ilist\[0\]))" \
+	"$repr_pattern" "Check invisible bp obj exists 1"
     gdb_test "python print (ilist\[0\].location)" \
 	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 1"
     gdb_test "python print (ilist\[0\].visible)" \
@@ -244,8 +248,8 @@ proc_with_prefix test_bkpt_invisible { } {
 	"Set invisible breakpoint" 0
     gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
 	"Get Breakpoint List" 0
-    gdb_test "python print (ilist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 2"
+    gdb_test "python print (repr (ilist\[0\]))" \
+	"$repr_pattern" "Check invisible bp obj exists 2"
     gdb_test "python print (ilist\[0\].location)" \
 	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 2"
     gdb_test "python print (ilist\[0\].visible)" \
diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
index 9ec2f44e9c..9bd5a35ed1 100644
--- a/gdb/testsuite/gdb.python/py-symbol.exp
+++ b/gdb/testsuite/gdb.python/py-symbol.exp
@@ -43,6 +43,8 @@ clean_restart ${binfile}
 # point where we don't have a current frame, and we don't want to
 # require one.
 gdb_py_test_silent_cmd "python main_func = gdb.lookup_global_symbol(\"main\")" "Lookup main" 1
+gdb_test "python print (repr (main_func))" "<gdb.Symbol print_name=main>" \
+    "test main_func.__repr__"
 gdb_test "python print (main_func.is_function)" "True" "test main_func.is_function"
 gdb_test "python print (gdb.lookup_global_symbol(\"junk\"))" "None" "test lookup_global_symbol(\"junk\")"
 
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index c245d41a1a..918216ddd6 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -388,3 +388,7 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
       test_type_equality
   }
 }
+
+# Test __repr__().
+gdb_test "python print (repr (gdb.lookup_type ('char')))" \
+      "<gdb.Type code=TYPE_CODE_INT name=char>" "test __repr__()"
-- 
2.40.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv3 0/2] Add __repr__() implementation to a few Python types
  2023-05-18  3:33         ` Matheus Branco Borella
@ 2023-05-19 21:27           ` Andrew Burgess
  2023-05-19 21:27             ` [PATCHv3 1/2] gdb: have mdict_size always return a symbol count Andrew Burgess
                               ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Andrew Burgess @ 2023-05-19 21:27 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Matheus,

Thanks for moving this forward.

I took a look through, and I think this looks great.  I noticed a few
whitespace issues with the patch, so I fixed all of them.  But during
testing I ended up making a few more changes, so I figured I'd just
post what I ended up with.

The biggest change you'll see is a whole extra patch.  It turns out
that the way you were using mdict_size wasn't correct -- mdict_size
doesn't always return the number of symbols!  As a result the symbol
list for block_func (in the py-block.exp test) would end up printed
without comma's between the symbols.  Anyway, the new first patch
fixes this.

While we're in py-block.c, I changed the output format.  Rather than
displaying the symbols one per line, they are now listed all on one
line.  The multi-line format looks great if all you are displaying is
the one object's repr, but if the repr is printed as part of a larger
string then I think the multi-line layout doesn't look as good.  Now
we're only printing a few symbols I figure we can afford to go with a
one line layout.

On the testing side I've tightened the patterns in py-block.exp, and
extended the test to check the ouptut in block_func -- this exposes
the mdict_size issue.

I've added some tests for gdb.BreakpointLocation, which were missing.

And I've made the patterns in py-breakpoint.exp more precise, and
added some additional tests to catch more cases.

Would be great to hear your thoughts on the updates,

Thanks,
Andrew

---

Andrew Burgess (1):
  gdb: have mdict_size always return a symbol count

Matheus Branco Borella via Gdb-patches (1):
  gdb: add __repr__() implementation to a few Python types

 gdb/dictionary.c                             | 13 +++-
 gdb/dictionary.h                             |  3 +-
 gdb/python/py-arch.c                         | 17 ++++-
 gdb/python/py-block.c                        | 37 ++++++++++-
 gdb/python/py-breakpoint.c                   | 67 ++++++++++++++++++-
 gdb/python/py-symbol.c                       | 15 ++++-
 gdb/python/py-type.c                         | 30 ++++++++-
 gdb/symmisc.c                                |  2 +-
 gdb/testsuite/gdb.python/py-arch.exp         |  6 ++
 gdb/testsuite/gdb.python/py-block.c          | 31 +++++++++
 gdb/testsuite/gdb.python/py-block.exp        | 38 ++++++++++-
 gdb/testsuite/gdb.python/py-bp-locations.exp | 32 +++++++++
 gdb/testsuite/gdb.python/py-breakpoint.exp   | 69 +++++++++++++++++---
 gdb/testsuite/gdb.python/py-symbol.exp       |  2 +
 gdb/testsuite/gdb.python/py-type.exp         |  4 ++
 15 files changed, 343 insertions(+), 23 deletions(-)


base-commit: e84060b489746d031ed1ec9e7b6b39fdf4b6cfe3
-- 
2.25.4


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv3 1/2] gdb: have mdict_size always return a symbol count
  2023-05-19 21:27           ` [PATCHv3 0/2] " Andrew Burgess
@ 2023-05-19 21:27             ` Andrew Burgess
  2023-05-19 21:27             ` [PATCHv3 2/2] gdb: add __repr__() implementation to a few Python types Andrew Burgess
  2023-06-07 17:05             ` [PATCHv3 0/2] Add " Matheus Branco Borella
  2 siblings, 0 replies; 15+ messages in thread
From: Andrew Burgess @ 2023-05-19 21:27 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

In the next commit we would like to have mdict_size return the number
of symbols in the dictionary, currently mdict_size is just a
heuristic, sometimes it returns the number of symbols, and sometimes
the number of buckets in a hashing dictionary (see size_hashed in
dictionary.c).

Currently this vague notion of size is good enough, the only place
mdict_size is used is in a maintenance command in order to print a
message containing the size of the dictionary ... so we don't really
care that the value isn't correct.

However, in the next commit we do want the size returned to be the
number of symbols in the dictionary, so this commit makes mdict_size
return the symbol count in all cases.

The new use is still not on a hot path -- it's going to be a Python
__repr__ method, so all I do in this commit is have size_hashed walk
the dictionary and count the entries, obviously this could be slow if
we have a large number of symbols, but for now I'm not worrying about
that case.  We could always store the symbol count if we wanted, but
that would increase the size of every dictionary for a use case that
isn't going to be hit that often.

I've updated the text in 'maint print symbols' so that we don't talk
about the size being 'syms/buckets', but just 'symbols' now.
---
 gdb/dictionary.c | 13 ++++++++++++-
 gdb/dictionary.h |  3 +--
 gdb/symmisc.c    |  2 +-
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/gdb/dictionary.c b/gdb/dictionary.c
index 4f8df240a3e..f3dfe092830 100644
--- a/gdb/dictionary.c
+++ b/gdb/dictionary.c
@@ -650,7 +650,18 @@ insert_symbol_hashed (struct dictionary *dict,
 static int
 size_hashed (const struct dictionary *dict)
 {
-  return DICT_HASHED_NBUCKETS (dict);
+  int nbuckets = DICT_HASHED_NBUCKETS (dict);
+  int total = 0;
+
+  for (int i = 0; i < nbuckets; ++i)
+    {
+      for (struct symbol *sym = DICT_HASHED_BUCKET (dict, i);
+	   sym != nullptr;
+	   sym = sym->hash_next)
+	total++;
+    }
+
+  return total;
 }
 
 /* Functions only for DICT_HASHED_EXPANDABLE.  */
diff --git a/gdb/dictionary.h b/gdb/dictionary.h
index d982396cb31..44bd075fcf2 100644
--- a/gdb/dictionary.h
+++ b/gdb/dictionary.h
@@ -159,8 +159,7 @@ extern struct symbol *
 extern struct symbol *mdict_iter_match_next (const lookup_name_info &name,
 					     struct mdict_iterator *miterator);
 
-/* Return some notion of the size of the multidictionary: the number of
-   symbols if we have that, the number of hash buckets otherwise.  */
+/* Return the number of symbols in multidictionary MDICT.  */
 
 extern int mdict_size (const struct multidictionary *mdict);
 
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index ff7f31f885f..e414dea7bf6 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -286,7 +286,7 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile)
 	  /* drow/2002-07-10: We could save the total symbols count
 	     even if we're using a hashtable, but nothing else but this message
 	     wants it.  */
-	  gdb_printf (outfile, ", %d syms/buckets in ",
+	  gdb_printf (outfile, ", %d symbols in ",
 		      mdict_size (b->multidict ()));
 	  gdb_puts (paddress (gdbarch, b->start ()), outfile);
 	  gdb_printf (outfile, "..");
-- 
2.25.4


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv3 2/2] gdb: add __repr__() implementation to a few Python types
  2023-05-19 21:27           ` [PATCHv3 0/2] " Andrew Burgess
  2023-05-19 21:27             ` [PATCHv3 1/2] gdb: have mdict_size always return a symbol count Andrew Burgess
@ 2023-05-19 21:27             ` Andrew Burgess
  2023-06-07 17:05             ` [PATCHv3 0/2] Add " Matheus Branco Borella
  2 siblings, 0 replies; 15+ messages in thread
From: Andrew Burgess @ 2023-05-19 21:27 UTC (permalink / raw)
  To: gdb-patches

From: Matheus Branco Borella via Gdb-patches <gdb-patches@sourceware.org>

Only a few types in the Python API currently have __repr__()
implementations.  This patch adds a few more of them. specifically: it
adds __repr__() implementations to gdb.Symbol, gdb.Architecture,
gdb.Block, gdb.Breakpoint, gdb.BreakpointLocation, and gdb.Type.

This makes it easier to play around the GDB Python API in the Python
interpreter session invoked with the 'pi' command in GDB, giving more
easily accessible tipe information to users.

An example of how this would look like:

  (gdb) pi
  >> gdb.lookup_type("char")
  <gdb.Type code=TYPE_CODE_INT name=char>
  >> gdb.lookup_global_symbol("main")
  <gdb.Symbol print_name=main>

The gdb.Block.__repr__() method shows the first 5 symbols from the
block, and then a message to show how many more were elided (if any).
---
 gdb/python/py-arch.c                         | 17 ++++-
 gdb/python/py-block.c                        | 37 ++++++++++-
 gdb/python/py-breakpoint.c                   | 67 ++++++++++++++++++-
 gdb/python/py-symbol.c                       | 15 ++++-
 gdb/python/py-type.c                         | 30 ++++++++-
 gdb/testsuite/gdb.python/py-arch.exp         |  6 ++
 gdb/testsuite/gdb.python/py-block.c          | 31 +++++++++
 gdb/testsuite/gdb.python/py-block.exp        | 38 ++++++++++-
 gdb/testsuite/gdb.python/py-bp-locations.exp | 32 +++++++++
 gdb/testsuite/gdb.python/py-breakpoint.exp   | 69 +++++++++++++++++---
 gdb/testsuite/gdb.python/py-symbol.exp       |  2 +
 gdb/testsuite/gdb.python/py-type.exp         |  4 ++
 12 files changed, 329 insertions(+), 19 deletions(-)

diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index 4d133d1fe14..ac519331f18 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -319,6 +319,21 @@ archpy_integer_type (PyObject *self, PyObject *args, PyObject *kw)
   return type_to_type_object (type);
 }
 
+/* __repr__ implementation for gdb.Architecture.  */
+
+static PyObject *
+archpy_repr (PyObject *self)
+{
+  const auto gdbarch = arch_object_to_gdbarch (self);
+  if (gdbarch == nullptr)
+    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+
+  auto arch_info = gdbarch_bfd_arch_info (gdbarch);
+  return PyUnicode_FromFormat ("<%s arch_name=%s printable_name=%s>",
+			       Py_TYPE (self)->tp_name, arch_info->arch_name,
+			       arch_info->printable_name);
+}
+
 /* Implementation of gdb.architecture_names().  Return a list of all the
    BFD architecture names that GDB understands.  */
 
@@ -395,7 +410,7 @@ PyTypeObject arch_object_type = {
   0,                                  /* tp_getattr */
   0,                                  /* tp_setattr */
   0,                                  /* tp_compare */
-  0,                                  /* tp_repr */
+  archpy_repr,                        /* tp_repr */
   0,                                  /* tp_as_number */
   0,                                  /* tp_as_sequence */
   0,                                  /* tp_as_mapping */
diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index 09fa74d862c..dd6d6d278a0 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -418,6 +418,41 @@ blpy_iter_is_valid (PyObject *self, PyObject *args)
   Py_RETURN_TRUE;
 }
 
+/* __repr__ implementation for gdb.Block.  */
+
+static PyObject *
+blpy_repr (PyObject *self)
+{
+  const auto block = block_object_to_block (self);
+  if (block == nullptr)
+    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+
+  const auto name = block->function () ?
+    block->function ()->print_name () : "<anonymous>";
+
+  std::string str;
+  unsigned int written_symbols = 0;
+  const int len = mdict_size (block->multidict ());
+  static constexpr int SYMBOLS_TO_SHOW = 5;
+  for (struct symbol *symbol : block_iterator_range (block))
+    {
+      if (written_symbols == SYMBOLS_TO_SHOW)
+	{
+	  const int remaining = len - SYMBOLS_TO_SHOW;
+	  if (remaining == 1)
+	    str += string_printf ("... (%d more symbol)", remaining);
+	  else
+	    str += string_printf ("... (%d more symbols)", remaining);
+	  break;
+	}
+      str += symbol->print_name ();
+      if (++written_symbols < len)
+	str += ", ";
+    }
+  return PyUnicode_FromFormat ("<%s %s {%s}>", Py_TYPE (self)->tp_name,
+			       name, str.c_str ());
+}
+
 static int CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION
 gdbpy_initialize_blocks (void)
 {
@@ -482,7 +517,7 @@ PyTypeObject block_object_type = {
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  blpy_repr,                      /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   &block_object_as_mapping,	  /*tp_as_mapping*/
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index becb04c91c1..caf58e4b101 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -33,6 +33,7 @@
 #include "location.h"
 #include "py-event.h"
 #include "linespec.h"
+#include "gdbsupport/common-utils.h"
 
 extern PyTypeObject breakpoint_location_object_type
     CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("breakpoint_location_object");
@@ -981,6 +982,31 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
   return 0;
 }
 
+/* __repr__ implementation for gdb.Breakpoint.  */
+
+static PyObject *
+bppy_repr (PyObject *self)
+{
+  const auto bp = (struct gdbpy_breakpoint_object*) self;
+  if (bp->bp == nullptr)
+    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+
+  std::string str = " ";
+  if (bp->bp->thread != -1)
+    str += string_printf ("thread=%d ", bp->bp->thread);
+  if (bp->bp->task > 0)
+    str += string_printf ("task=%d ", bp->bp->task);
+  if (bp->bp->enable_count > 0)
+    str += string_printf ("enable_count=%d ", bp->bp->enable_count);
+  str.pop_back ();
+
+  return PyUnicode_FromFormat ("<%s%s number=%d hits=%d%s>",
+			       Py_TYPE (self)->tp_name,
+			       (bp->bp->enable_state == bp_enabled
+				? "" : " disabled"), bp->bp->number,
+			       bp->bp->hit_count, str.c_str ());
+}
+
 /* Append to LIST the breakpoint Python object associated to B.
 
    Return true on success.  Return false on failure, with the Python error
@@ -1406,7 +1432,7 @@ PyTypeObject breakpoint_object_type =
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  bppy_repr,                     /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   0,				  /*tp_as_mapping*/
@@ -1624,6 +1650,43 @@ bplocpy_dealloc (PyObject *py_self)
   Py_TYPE (py_self)->tp_free (py_self);
 }
 
+/* __repr__ implementation for gdb.BreakpointLocation.  */
+
+static PyObject *
+bplocpy_repr (PyObject *py_self)
+{
+  const auto self = (gdbpy_breakpoint_location_object *) py_self;
+  if (self->owner == nullptr || self->owner->bp == nullptr
+    || self->owner->bp != self->bp_loc->owner)
+    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+
+  const auto enabled = self->bp_loc->enabled ? "enabled" : "disabled";
+
+  std::string str (enabled);
+
+  str += string_printf (" address=%s",
+			paddress (self->bp_loc->owner->gdbarch,
+				  self->bp_loc->address));
+
+  if (self->bp_loc->requested_address != self->bp_loc->address)
+    str += string_printf (" requested_address=%s",
+			  paddress (self->bp_loc->owner->gdbarch,
+				    self->bp_loc->requested_address));
+  if (self->bp_loc->symtab != nullptr)
+    str += string_printf (" source=%s:%d", self->bp_loc->symtab->filename,
+			  self->bp_loc->line_number);
+
+  const auto fn_name = self->bp_loc->function_name.get ();
+  if (fn_name != nullptr)
+    {
+      str += " in ";
+      str += fn_name;
+    }
+
+  return PyUnicode_FromFormat ("<%s %s>", Py_TYPE (self)->tp_name,
+			       str.c_str ());
+}
+
 /* Attribute get/set Python definitions. */
 
 static gdb_PyGetSetDef bp_location_object_getset[] = {
@@ -1655,7 +1718,7 @@ PyTypeObject breakpoint_location_object_type =
   0,					/*tp_getattr*/
   0,					/*tp_setattr*/
   0,					/*tp_compare*/
-  0,					/*tp_repr*/
+  bplocpy_repr,                        /*tp_repr*/
   0,					/*tp_as_number*/
   0,					/*tp_as_sequence*/
   0,					/*tp_as_mapping*/
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index ff3d18504e7..ee863aa4df4 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -378,6 +378,19 @@ sympy_dealloc (PyObject *obj)
   Py_TYPE (obj)->tp_free (obj);
 }
 
+/* __repr__ implementation for gdb.Symbol.  */
+
+static PyObject *
+sympy_repr (PyObject *self)
+{
+  const auto symbol = symbol_object_to_symbol (self);
+  if (symbol == nullptr)
+    return PyUnicode_FromFormat ("<%s (invalid)>", Py_TYPE (self)->tp_name);
+
+  return PyUnicode_FromFormat ("<%s print_name=%s>", Py_TYPE (self)->tp_name,
+			       symbol->print_name ());
+}
+
 /* Implementation of
    gdb.lookup_symbol (name [, block] [, domain]) -> (symbol, is_field_of_this)
    A tuple with 2 elements is always returned.  The first is the symbol
@@ -741,7 +754,7 @@ PyTypeObject symbol_object_type = {
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  sympy_repr,                    /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   0,				  /*tp_as_mapping*/
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index b9fa741177f..b4d1e230b3b 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -1028,6 +1028,34 @@ typy_template_argument (PyObject *self, PyObject *args)
   return result;
 }
 
+/* __repr__ implementation for gdb.Type.  */
+
+static PyObject *
+typy_repr (PyObject *self)
+{
+  const auto type = type_object_to_type (self);
+  if (type == nullptr)
+    return PyUnicode_FromFormat ("<%s (invalid)>",
+				 Py_TYPE (self)->tp_name);
+
+  const char *code = pyty_codes[type->code ()].name;
+  string_file type_name;
+  try
+    {
+      current_language->print_type (type, "", &type_name, -1, 0,
+				    &type_print_raw_options);
+    }
+  catch (const gdb_exception &except)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+    }
+  auto py_typename = PyUnicode_Decode (type_name.c_str (), type_name.size (),
+				       host_charset (), NULL);
+
+  return PyUnicode_FromFormat ("<%s code=%s name=%U>", Py_TYPE (self)->tp_name,
+			       code, py_typename);
+}
+
 static PyObject *
 typy_str (PyObject *self)
 {
@@ -1617,7 +1645,7 @@ PyTypeObject type_object_type =
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  typy_repr,                     /*tp_repr*/
   &type_object_as_number,	  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   &typy_mapping,		  /*tp_as_mapping*/
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
index 4f4b4aa766f..597943ff682 100644
--- a/gdb/testsuite/gdb.python/py-arch.exp
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -27,6 +27,8 @@ if ![runto_main] {
 # Test python/15461.  Invalid architectures should not trigger an
 # internal GDB assert.
 gdb_py_test_silent_cmd "python empty = gdb.Architecture()" "get empty arch" 0
+gdb_test "python print(repr (empty))" "<gdb\\.Architecture \\(invalid\\)>" \
+    "Test empty achitecture __repr__ does not trigger an assert"
 gdb_test "python print(empty.name())" ".*Architecture is invalid.*" \
     "Test empty architecture.name does not trigger an assert"
 gdb_test "python print(empty.disassemble())" ".*Architecture is invalid.*" \
@@ -44,6 +46,10 @@ gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
 gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(gdb.Value(pc))" \
   "disassemble no end no count" 0
 
+gdb_test "python print (repr (arch))" \
+    "<gdb.Architecture arch_name=.* printable_name=.*>" \
+    "test __repr__ for architecture"
+
 gdb_test "python print (len(insn_list1))" "1" "test number of instructions 1"
 gdb_test "python print (len(insn_list2))" "1" "test number of instructions 2"
 gdb_test "python print (len(insn_list3))" "1" "test number of instructions 3"
diff --git a/gdb/testsuite/gdb.python/py-block.c b/gdb/testsuite/gdb.python/py-block.c
index a0c6e165605..dd2e195af4a 100644
--- a/gdb/testsuite/gdb.python/py-block.c
+++ b/gdb/testsuite/gdb.python/py-block.c
@@ -30,9 +30,40 @@ int block_func (void)
   }
 }
 
+/* A function with no locals.  Used for testing gdb.Block.__repr__().  */
+int no_locals_func (void)
+{
+  return block_func ();
+}
+
+/* A function with 5 locals.  Used for testing gdb.Block.__repr__().  */
+int few_locals_func (void)
+{
+  int i = 0;
+  int j = 0;
+  int k = 0;
+  int x = 0;
+  int y = 0;
+  return block_func ();
+}
+
+/* A function with 6 locals.  Used for testing gdb.Block.__repr__().  */
+int many_locals_func (void)
+{
+  int i = 0;
+  int j = 0;
+  int k = 0;
+  int x = 0;
+  int y = 0;
+  int z = 0;
+  return block_func ();
+}
 
 int main (int argc, char *argv[])
 {
   block_func ();
+  no_locals_func ();
+  few_locals_func ();
+  many_locals_func ();
   return 0; /* Break at end. */
 }
diff --git a/gdb/testsuite/gdb.python/py-block.exp b/gdb/testsuite/gdb.python/py-block.exp
index 3bdf97294ae..37e3105b4e3 100644
--- a/gdb/testsuite/gdb.python/py-block.exp
+++ b/gdb/testsuite/gdb.python/py-block.exp
@@ -38,7 +38,8 @@ gdb_continue_to_breakpoint "Block break here."
 gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame" 0
 gdb_py_test_silent_cmd "python block = frame.block()" \
     "Get block, initial innermost block" 0
-gdb_test "python print (block)" "<gdb.Block object at $hex>" "check block not None"
+gdb_test "python print (block)" "<gdb.Block <anonymous> \{i, f, b\}>" \
+    "check block not None"
 gdb_test "python print (block.function)" "None" "first anonymous block"
 gdb_test "python print (block.start)" "${decimal}" "check start not None"
 gdb_test "python print (block.end)" "${decimal}" "check end not None"
@@ -68,15 +69,46 @@ gdb_test_no_output "python block = block.superblock" "get superblock 2"
 gdb_test "python print (block.function)" "block_func" \
          "Print superblock 2 function"
 
+# Switch frames, then test block for no_locals_func.
+gdb_test "continue" ".*" "continue to no_locals_func breakpoint"
+gdb_test "up" ".*" "up to no_locals_func"
+gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
+gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
+gdb_test "python print (repr (block))" "<gdb.Block no_locals_func \{\}>" \
+    "Check block in no_locals_func"
+gdb_test "python print (block.function)" "no_locals_func" \
+    "no_locals_func block"
+
+# Switch frames, then test block for few_locals_func.
+gdb_test "continue" ".*" "continue to few_locals_func breakpoint"
+gdb_test "up" ".*" "up to few_locals_func"
+gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
+gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
+gdb_test "python print (repr (block))" \
+    "<gdb.Block few_locals_func \{i, j, k, x, y\}>" \
+    "Check block in few_locals_func"
+gdb_test "python print (block.function)" "few_locals_func" \
+    "few_locals_func block"
+
+# Switch frames, then test block for many_locals_func.
+gdb_test "continue" ".*" "continue to many_locals_func breakpoint"
+gdb_test "up" ".*" "up to many_locals_func"
+gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
+gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
+gdb_test "python print (repr (block))" \
+    "<gdb.Block many_locals_func \{i, j, k, x, y, \\.\\.\\. \\(1 more symbol\\)\}>" \
+    "Check block in many_locals_func"
+gdb_test "python print (block.function)" "many_locals_func" \
+    "many_locals_func block"
+
 # Switch frames, then test for main block.
 gdb_test "up" ".*"
 gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "Get Frame 2" 0
 gdb_py_test_silent_cmd "python block = frame.block()" "Get Frame 2's block" 0
-gdb_test "python print (block)" "<gdb.Block object at $hex>" \
+gdb_test "python print (repr (block))" "<gdb.Block main \{.*\}>" \
          "Check Frame 2's block not None"
 gdb_test "python print (block.function)" "main" "main block"
 
-
 # Test Block is_valid.  This must always be the last test in this
 # testcase as it unloads the object file.
 delete_breakpoints
diff --git a/gdb/testsuite/gdb.python/py-bp-locations.exp b/gdb/testsuite/gdb.python/py-bp-locations.exp
index f8649f6c105..b3a8c83bc0a 100644
--- a/gdb/testsuite/gdb.python/py-bp-locations.exp
+++ b/gdb/testsuite/gdb.python/py-bp-locations.exp
@@ -31,6 +31,30 @@ if ![runto_main] {
     return -1
 }
 
+# Build a regexp string that represents the __repr__ of a
+# gdb.BreakpointLocation object.  Accepts arguments -enabled, -address,
+# -source, -line, and -func.
+proc build_bpl_regexp { args } {
+    parse_args [list {enabled True} [list address "$::hex"] {source ".*"} \
+		    [list line "$::decimal"] {func ""}]
+
+    set pattern "<gdb.BreakpointLocation"
+
+    if {$enabled} {
+	set pattern "$pattern enabled"
+    } else {
+	set pattern "$pattern disabled"
+    }
+
+    set pattern "$pattern address=${address}(?: requested_address=$::hex)?"
+    set pattern "$pattern source=${source}:${line}"
+    if {$func ne ""} {
+	set pattern "$pattern in ${func}"
+    }
+    set pattern "$pattern>"
+    return $pattern
+}
+
 # Set breakpoint with 2 locations.
 gdb_breakpoint "add"
 
@@ -42,9 +66,17 @@ gdb_test "python print(gdb.breakpoints()\[1\].locations\[0\].source)" \
 	 ".*('.*py-bp-locations.c', $expected_line_a).*"
 gdb_test "python print(gdb.breakpoints()\[1\].locations\[1\].source)" \
 	 ".*('.*py-bp-locations.c', $expected_line_b).*"
+gdb_test "python print(gdb.breakpoints()\[1\].locations\[1\])" \
+    [build_bpl_regexp -enabled True -source ".*py-bp-locations.c" \
+	 -line "$expected_line_b" -func ".*"] \
+    "check repr of enabled breakpoint location"
 
 # Disable first location and make sure we don't hit it.
 gdb_test "python gdb.breakpoints()\[1\].locations\[0\].enabled = False" ""
+gdb_test "python print(gdb.breakpoints()\[1\].locations\[0\])" \
+    [build_bpl_regexp -enabled False -source ".*py-bp-locations.c" \
+	 -line "$expected_line_a" -func ".*"] \
+    "check repr of disabled breakpoint location"
 gdb_continue_to_breakpoint "" ".*25.*"
 
 if ![runto_main] {
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index 76094c95d10..df17d646b28 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -38,6 +38,36 @@ if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${options}]
 
 set past_throw_catch_line [gdb_get_line_number "Past throw-catch."]
 
+# Build a regexp string that can match against the repr of a gdb.Breakpoint
+# object.  Accepts arguments -enabled, -number, -hits, -thread, -task, and
+# -enable_count.  The -enabled argument is a boolean, while all of the others
+# take a regexp string.
+proc build_bp_repr { args } {
+    parse_args [list {enabled True} [list number "-?$::decimal"] \
+		    [list hits $::decimal] {thread ""} {task ""} \
+		    {enable_count ""}]
+
+    set pattern "<gdb\\.Breakpoint"
+
+    if {!$enabled} {
+	set pattern "$pattern disabled"
+    }
+
+    set pattern "$pattern number=$number hits=$hits"
+
+    if {$thread ne ""} {
+	set pattern "$pattern thread=$thread"
+    }
+    if {$task ne ""} {
+	set pattern "$pattern task=$task"
+    }
+    if {$enable_count ne ""} {
+	set pattern "$pattern enable_count=$enable_count"
+    }
+    set pattern "${pattern}>"
+    return $pattern
+}
+
 proc_with_prefix test_bkpt_basic { } {
     global srcfile testfile hex decimal
 
@@ -54,8 +84,8 @@ proc_with_prefix test_bkpt_basic { } {
     # Now there should be one breakpoint: main.
     gdb_py_test_silent_cmd "python blist = gdb.breakpoints()" \
 	"Get Breakpoint List" 0
-    gdb_test "python print (blist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check obj exists @main"
+    gdb_test "python print (repr (blist\[0\]))" \
+	[build_bp_repr -number 1 -hits 1] "Check obj exists @main"
     gdb_test "python print (blist\[0\].location)" \
 	"main" "Check breakpoint location @main"
     gdb_test "python print (blist\[0\].pending)" "False" \
@@ -72,12 +102,12 @@ proc_with_prefix test_bkpt_basic { } {
 	"Get Breakpoint List" 0
     gdb_test "python print (len(blist))" \
 	"2" "Check for two breakpoints"
-    gdb_test "python print (blist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check obj exists @main 2"
+    gdb_test "python print (repr (blist\[0\]))" \
+	[build_bp_repr -number 1 -hits 1] "Check obj exists @main 2"
     gdb_test "python print (blist\[0\].location)" \
 	"main" "Check breakpoint location @main 2"
-    gdb_test "python print (blist\[1\])" \
-	"<gdb.Breakpoint object at $hex>" "Check obj exists @mult_line"
+    gdb_test "python print (repr (blist\[1\]))" \
+	[build_bp_repr -number 2 -hits 1] "Check obj exists @mult_line"
 
     gdb_test "python print (blist\[1\].location)" \
 	"py-breakpoint\.c:${mult_line}*" \
@@ -102,6 +132,9 @@ proc_with_prefix test_bkpt_basic { } {
 	"True" "Check breakpoint enabled."
     gdb_py_test_silent_cmd  "python blist\[1\].enabled = False" \
 	"Set breakpoint disabled." 0
+    gdb_test "python print (repr (blist\[1\]))" \
+	[build_bp_repr -enabled False -number 2 -hits 6] \
+	"Check repr for a disabled breakpoint"
     gdb_continue_to_breakpoint "Break at add 2" ".*Break at add.*"
     gdb_py_test_silent_cmd  "python blist\[1\].enabled = True" \
 	"Set breakpoint enabled." 0
@@ -113,6 +146,13 @@ proc_with_prefix test_bkpt_basic { } {
 	"Get Breakpoint List" 0
     gdb_test "python print (blist\[1\].thread)" \
 	"None" "Check breakpoint thread"
+    gdb_py_test_silent_cmd "python blist\[1\].thread = 1" \
+	"set breakpoint thread" 0
+    gdb_test "python print (repr (blist\[1\]))" \
+	[build_bp_repr -number 2 -hits 7 -thread 1] \
+	"Check repr for a thread breakpoint"
+    gdb_py_test_silent_cmd "python blist\[1\].thread = None" \
+	"clear breakpoint thread" 0
     gdb_test "python print (blist\[1\].type == gdb.BP_BREAKPOINT)" \
 	"True" "Check breakpoint type"
     gdb_test "python print (blist\[0\].number)" \
@@ -231,8 +271,8 @@ proc_with_prefix test_bkpt_invisible { } {
 	"Set invisible breakpoint" 0
     gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
 	"Get Breakpoint List" 0
-    gdb_test "python print (ilist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 1"
+    gdb_test "python print (repr (ilist\[0\]))" \
+	[build_bp_repr -number 2 -hits 0] "Check invisible bp obj exists 1"
     gdb_test "python print (ilist\[0\].location)" \
 	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 1"
     gdb_test "python print (ilist\[0\].visible)" \
@@ -244,8 +284,9 @@ proc_with_prefix test_bkpt_invisible { } {
 	"Set invisible breakpoint" 0
     gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" \
 	"Get Breakpoint List" 0
-    gdb_test "python print (ilist\[0\])" \
-	"<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists 2"
+    gdb_test "python print (repr (ilist\[0\]))" \
+	[build_bp_repr -number "-$decimal" -hits 0] \
+	"Check invisible bp obj exists 2"
     gdb_test "python print (ilist\[0\].location)" \
 	"py-breakpoint\.c:$ibp_location*" "Check breakpoint location 2"
     gdb_test "python print (ilist\[0\].visible)" \
@@ -835,6 +876,14 @@ proc_with_prefix test_bkpt_auto_disable { } {
     set mult_line [gdb_get_line_number "Break at multiply."]
     gdb_breakpoint ${mult_line}
     gdb_test_no_output "enable count 1 2" "one shot enable"
+
+    # Find the Python gdb.Breakpoint object for breakpoint #2.
+    gdb_py_test_silent_cmd \
+	"python bp = \[b for b in gdb.breakpoints() if b.number == 2\]\[0\]" \
+	"Get breakpoint number 2" 0
+    gdb_test "python print (repr (bp))" \
+	[build_bp_repr -number 2 -hits 0 -enable_count 1]
+
     # Python 2 doesn't support print in lambda function, so use a named
     # function instead.
     gdb_test_multiline "Define print_bp_enabled" \
diff --git a/gdb/testsuite/gdb.python/py-symbol.exp b/gdb/testsuite/gdb.python/py-symbol.exp
index 9ec2f44e9c0..9bd5a35ed1c 100644
--- a/gdb/testsuite/gdb.python/py-symbol.exp
+++ b/gdb/testsuite/gdb.python/py-symbol.exp
@@ -43,6 +43,8 @@ clean_restart ${binfile}
 # point where we don't have a current frame, and we don't want to
 # require one.
 gdb_py_test_silent_cmd "python main_func = gdb.lookup_global_symbol(\"main\")" "Lookup main" 1
+gdb_test "python print (repr (main_func))" "<gdb.Symbol print_name=main>" \
+    "test main_func.__repr__"
 gdb_test "python print (main_func.is_function)" "True" "test main_func.is_function"
 gdb_test "python print (gdb.lookup_global_symbol(\"junk\"))" "None" "test lookup_global_symbol(\"junk\")"
 
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index c245d41a1ac..918216ddd69 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -388,3 +388,7 @@ if { [build_inferior "${binfile}-cxx" "c++"] == 0 } {
       test_type_equality
   }
 }
+
+# Test __repr__().
+gdb_test "python print (repr (gdb.lookup_type ('char')))" \
+      "<gdb.Type code=TYPE_CODE_INT name=char>" "test __repr__()"
-- 
2.25.4


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv3 0/2] Add __repr__() implementation to a few Python types
  2023-05-19 21:27           ` [PATCHv3 0/2] " Andrew Burgess
  2023-05-19 21:27             ` [PATCHv3 1/2] gdb: have mdict_size always return a symbol count Andrew Burgess
  2023-05-19 21:27             ` [PATCHv3 2/2] gdb: add __repr__() implementation to a few Python types Andrew Burgess
@ 2023-06-07 17:05             ` Matheus Branco Borella
  2023-06-08 18:46               ` Andrew Burgess
  2023-06-09 12:33               ` Andrew Burgess
  2 siblings, 2 replies; 15+ messages in thread
From: Matheus Branco Borella @ 2023-06-07 17:05 UTC (permalink / raw)
  To: aburgess; +Cc: dark.ryu.550, gdb-patches

All of the changes look good in my oppinion. I'm still getting used to the hang
of things, so I really appreciate the effort you put into making my patch more 
presentable! So I think it should be good to go?




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv3 0/2] Add __repr__() implementation to a few Python types
  2023-06-07 17:05             ` [PATCHv3 0/2] Add " Matheus Branco Borella
@ 2023-06-08 18:46               ` Andrew Burgess
  2023-06-09 12:33               ` Andrew Burgess
  1 sibling, 0 replies; 15+ messages in thread
From: Andrew Burgess @ 2023-06-08 18:46 UTC (permalink / raw)
  To: Matheus Branco Borella via Gdb-patches, aburgess
  Cc: dark.ryu.550, gdb-patches

Matheus Branco Borella via Gdb-patches <gdb-patches@sourceware.org>
writes:

> All of the changes look good in my oppinion. I'm still getting used to the hang
> of things, so I really appreciate the effort you put into making my patch more 
> presentable! So I think it should be good to go?

I'll update, retest, and push these patches tomorrow.

Thanks,
Andrew


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv3 0/2] Add __repr__() implementation to a few Python types
  2023-06-07 17:05             ` [PATCHv3 0/2] Add " Matheus Branco Borella
  2023-06-08 18:46               ` Andrew Burgess
@ 2023-06-09 12:33               ` Andrew Burgess
  2023-07-04 11:09                 ` Andrew Burgess
  1 sibling, 1 reply; 15+ messages in thread
From: Andrew Burgess @ 2023-06-09 12:33 UTC (permalink / raw)
  To: Matheus Branco Borella via Gdb-patches, aburgess
  Cc: dark.ryu.550, gdb-patches

Matheus Branco Borella via Gdb-patches <gdb-patches@sourceware.org>
writes:

> All of the changes look good in my oppinion. I'm still getting used to the hang
> of things, so I really appreciate the effort you put into making my patch more 
> presentable! So I think it should be good to go?

Do you have a copyright assignment in place?  If not then, given the
size of the change, I think you will need one before this could be
merged.

For details see:

  https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assignment

I think you'd need to complete this form:

  https://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future

and send it to assign@gnu.org, then you'd be sent the actual assignment
document to complete and send back.

Thanks,
Andrew


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv3 0/2] Add __repr__() implementation to a few Python types
  2023-06-09 12:33               ` Andrew Burgess
@ 2023-07-04 11:09                 ` Andrew Burgess
  0 siblings, 0 replies; 15+ messages in thread
From: Andrew Burgess @ 2023-07-04 11:09 UTC (permalink / raw)
  To: Matheus Branco Borella via Gdb-patches, aburgess
  Cc: dark.ryu.550, gdb-patches

Andrew Burgess <aburgess@redhat.com> writes:

> Matheus Branco Borella via Gdb-patches <gdb-patches@sourceware.org>
> writes:
>
>> All of the changes look good in my oppinion. I'm still getting used to the hang
>> of things, so I really appreciate the effort you put into making my patch more 
>> presentable! So I think it should be good to go?
>
> Do you have a copyright assignment in place?  If not then, given the
> size of the change, I think you will need one before this could be
> merged.

Matheus,

Thanks for getting the copyright assignment sorted.

I've now gone ahead and merged both patches in this series.

Thanks,
Andrew


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-07-04 11:09 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <3Cc9d2dc49-45c7-d6b9-c567-4ec78dd870a0>
2023-01-11  0:35 ` [PATCH] Add __repr__() implementation to a few Python types Matheus Branco Borella
2023-01-18 17:05   ` Bruno Larsen
2023-01-18 18:02   ` Andrew Burgess
2023-01-19  8:23     ` Bruno Larsen
2023-01-20  1:43     ` Matheus Branco Borella
2023-01-20 16:45       ` Andrew Burgess
2023-01-24 14:45       ` Andrew Burgess
2023-05-18  3:33         ` Matheus Branco Borella
2023-05-19 21:27           ` [PATCHv3 0/2] " Andrew Burgess
2023-05-19 21:27             ` [PATCHv3 1/2] gdb: have mdict_size always return a symbol count Andrew Burgess
2023-05-19 21:27             ` [PATCHv3 2/2] gdb: add __repr__() implementation to a few Python types Andrew Burgess
2023-06-07 17:05             ` [PATCHv3 0/2] Add " Matheus Branco Borella
2023-06-08 18:46               ` Andrew Burgess
2023-06-09 12:33               ` Andrew Burgess
2023-07-04 11:09                 ` Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).