[PR,python/17364] Cleanup registration of __gdb_builtin_type_bound128 pretty-printer

Message ID yjt21tqn7jvk.fsf@ruffy.mtv.corp.google.com
State New, archived
Headers

Commit Message

Doug Evans Oct. 4, 2014, 6:46 p.m. UTC
  Hi.

The registration of this pretty-printer is all wrong.
This patch creates a new global "builtin" collection of pretty-printers,
adds the bound pretty-printer to it, and provides an API call
to let one register more builtin pretty-printers.

I'd really like to get this into 7.8.1,
I don't want the way it is currently registered to get built on.

One thing that still bothers me is that the bound128 type is
x86-specific and yet the printer is arch-independent.
That should require changing the name __gdb_builtin_type_bound128
in i386-tdep.c to include x86 in the name (or some such),
but maybe that's too big a change for now.
But as gdb becomes more multi-arch, attention to collisions and confusion
in global namespaces (e.g., the space of builtin pretty-printers)
is going to be more important.
[One way to solve this would be to record such printers with
the arch, but that feels like overkill.]

2014-10-03  Doug Evans  <xdje42@gmail.com>

	PR python/17364
	* python/lib/gdb/__init__.py (packages): Add "printer".
	* python/lib/gdb/command/bound_registers.py: Moved to ...
	* python/lib/gdb/printer/bound_registers.py: ... here.
	Add printer to global set of builtin printers.  Rename printer from
	"bound" to "bound128".
	* python/lib/gdb/printing.py (_builtin_pretty_printers): New global,
	registered as global "builtin" printer.
	(add_builtin_pretty_printer): New function.
	* data-directory/Makefile.in (PYTHON_FILE_LIST): Update, and add
	gdb/printer/__init__.py.
  

Comments

Phil Muldoon Oct. 7, 2014, 12:17 p.m. UTC | #1
On 04/10/14 19:46, Doug Evans wrote:
> Hi.
>
> The registration of this pretty-printer is all wrong.
> This patch creates a new global "builtin" collection of pretty-printers,
> adds the bound pretty-printer to it, and provides an API call
> to let one register more builtin pretty-printers.
>
> I'd really like to get this into 7.8.1,
> I don't want the way it is currently registered to get built on.
>
> One thing that still bothers me is that the bound128 type is
> x86-specific and yet the printer is arch-independent.
> That should require changing the name __gdb_builtin_type_bound128
> in i386-tdep.c to include x86 in the name (or some such),
> but maybe that's too big a change for now.
> But as gdb becomes more multi-arch, attention to collisions and confusion
> in global namespaces (e.g., the space of builtin pretty-printers)
> is going to be more important.
> [One way to solve this would be to record such printers with
> the arch, but that feels like overkill.]
>
> 2014-10-03  Doug Evans  <xdje42@gmail.com>
>
>     PR python/17364
>     * python/lib/gdb/__init__.py (packages): Add "printer".
>     * python/lib/gdb/command/bound_registers.py: Moved to ...
>     * python/lib/gdb/printer/bound_registers.py: ... here.
>     Add printer to global set of builtin printers.  Rename printer from
>     "bound" to "bound128".
>     * python/lib/gdb/printing.py (_builtin_pretty_printers): New global,
>     registered as global "builtin" printer.
>     (add_builtin_pretty_printer): New function.
>     * data-directory/Makefile.in (PYTHON_FILE_LIST): Update, and add
>     gdb/printer/__init__.py.

Doug,

Looks fine to me.  I kind of like the idea of a more specific
"printer" based package for commands or functions that only
specifically relate to printers.

Cheers

Phil
  

Patch

diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index 1e8cd4b..00c70bb 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -64,7 +64,6 @@  PYTHON_FILE_LIST = \
 	gdb/printing.py \
 	gdb/prompt.py \
 	gdb/xmethod.py \
-	gdb/command/bound_registers.py \
 	gdb/command/__init__.py \
 	gdb/command/xmethods.py \
 	gdb/command/frame_filters.py \
@@ -74,7 +73,9 @@  PYTHON_FILE_LIST = \
 	gdb/command/explore.py \
 	gdb/function/__init__.py \
 	gdb/function/caller_is.py \
-	gdb/function/strfns.py
+	gdb/function/strfns.py \
+	gdb/printer/__init__.py \
+	gdb/printer/bound_registers.py
 
 @HAVE_PYTHON_TRUE@PYTHON_FILES = $(PYTHON_FILE_LIST)
 @HAVE_PYTHON_FALSE@PYTHON_FILES =
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 557e168..8c6eee2 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -81,7 +81,8 @@  PYTHONDIR = os.path.dirname(os.path.dirname(__file__))
 
 packages = [
     'function',
-    'command'
+    'command',
+    'printer'
 ]
 
 # pkgutil.iter_modules is not available prior to Python 2.6.  Instead,
diff --git a/gdb/python/lib/gdb/command/bound_registers.py b/gdb/python/lib/gdb/command/bound_registers.py
deleted file mode 100644
index 24d4c45..0000000
--- a/gdb/python/lib/gdb/command/bound_registers.py
+++ /dev/null
@@ -1,45 +0,0 @@ 
-# Pretty-printer utilities.
-# Copyright (C) 2013-2014 Free Software Foundation, Inc.
-
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 3 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-import gdb.printing
-
-class BoundPrinter:
-    """Adds size field to a _rawbound128 type."""
-
-    def __init__ (self, val):
-        self.val = val
-
-    def to_string (self):
-        upper = self.val["ubound"]
-        lower = self.val["lbound"]
-        size  = (long) ((upper) - (lower))
-        if size > -1:
-            size = size + 1
-        result = '{lbound = %s, ubound = %s} : size %s' % (lower, upper, size)
-        return result
-
-# There are two pattern matching used: first one is related to a library
-# second is related to the type. Since we are displaying a register all
-# libraries are accepted. Type to be processed is the same present
-# in the xml file.
-
-def build_pretty_printer ():
-    pp = gdb.printing.RegexpCollectionPrettyPrinter (".*")
-    pp.add_printer ('bound', '^__gdb_builtin_type_bound128', BoundPrinter)
-    return pp
-
-gdb.printing.register_pretty_printer (gdb.current_objfile (),
-                                      build_pretty_printer ())
diff --git a/gdb/python/lib/gdb/printer/__init__.py b/gdb/python/lib/gdb/printer/__init__.py
new file mode 100644
index 0000000..ca768c89
--- /dev/null
+++ b/gdb/python/lib/gdb/printer/__init__.py
@@ -0,0 +1,16 @@ 
+# Copyright (C) 2010-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
diff --git a/gdb/python/lib/gdb/printer/bound_registers.py b/gdb/python/lib/gdb/printer/bound_registers.py
new file mode 100644
index 0000000..e585dcb
--- /dev/null
+++ b/gdb/python/lib/gdb/printer/bound_registers.py
@@ -0,0 +1,36 @@ 
+# Pretty-printer utilities.
+# Copyright (C) 2013-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import gdb.printing
+
+class Bound128Printer:
+    """Adds size field to a _rawbound128 type."""
+
+    def __init__ (self, val):
+        self.val = val
+
+    def to_string (self):
+        upper = self.val["ubound"]
+        lower = self.val["lbound"]
+        size  = (long) ((upper) - (lower))
+        if size > -1:
+            size = size + 1
+        result = '{lbound = %s, ubound = %s} : size %s' % (lower, upper, size)
+        return result
+
+gdb.printing.add_builtin_pretty_printer ('bound128',
+                                         '^__gdb_builtin_type_bound128',
+                                         Bound128Printer)
diff --git a/gdb/python/lib/gdb/printing.py b/gdb/python/lib/gdb/printing.py
index 2940b93..ff5250a 100644
--- a/gdb/python/lib/gdb/printing.py
+++ b/gdb/python/lib/gdb/printing.py
@@ -263,3 +263,17 @@  class FlagEnumerationPrinter(PrettyPrinter):
             return _EnumInstance(self.enumerators, val)
         else:
             return None
+
+
+# Builtin pretty-printers.
+# The set is defined as empty, and files in printing/*.py add their printers
+# to this with add_builtin_pretty_printer.
+
+_builtin_pretty_printers = RegexpCollectionPrettyPrinter("builtin")
+
+register_pretty_printer(None, _builtin_pretty_printers)
+
+# Add a builtin pretty-printer.
+
+def add_builtin_pretty_printer(name, regexp, printer):
+    _builtin_pretty_printers.add_printer(name, regexp, printer)