public inbox for archer@sourceware.org
 help / color / mirror / Atom feed
* [python] [rfc] Patch for pretty-printers registration API change
@ 2009-02-17 14:03 Phil Muldoon
  2009-02-17 18:52 ` Tom Tromey
  0 siblings, 1 reply; 8+ messages in thread
From: Phil Muldoon @ 2009-02-17 14:03 UTC (permalink / raw)
  To: Project Archer

[-- Attachment #1: Type: text/plain, Size: 5580 bytes --]

One of this things we have been working on with Archer Python branch is 
a different method for registering Python based pretty-printers. This 
change will break the API for existing pretty-printers.

Previously Python based pretty-printers were registered via a dictionary 
by type.  In the global pretty-printer dictionary, like this:

gdb.pretty_printers['^std::basic_string<char,.*>$'] = lambda val: 
StdStringPrinter(None, val)

And also similarly for any pretty-printers registered with local object 
files. Now the method of addition is something like:

gdb.pretty_printers.add (some_lookup_function);

For reasons that have been discussed on the Archer mailing-list,  
registering a pretty-printer against a type was not what we wanted. So 
we decided instead to use a Python list that contains a sequence of 
functions that will be executed until data is returned. As each of these 
functions is iterated over,  they will be a passed a gdb.Value.  Each 
look-up function then uses its own arbitrary and anonymous logic to 
determine whether it can pretty print this value. If it can, the 
function should return an instantiation of the pretty-printer. If it 
cannot, it should return the Python type "None".  The return of "None" 
is the  cue that GDB will use to move onto the next pretty printer. 
There is no pre-defined order of execution

In writing this patch, I converted the libstdcxx printers to use this 
new method. I also converted the gdb.python/python-prettyprint.py 
test-suite file. These will probably serve as better examples than 
trying to further narrate the API change here.  They are included in the 
patch . This patch does not increase (or decrease) the regressions as 
compared to the archer-tromey-python branch. Some unordered* printers in 
the libstdcxx library do not work any more. After discussing it on irc 
we think this is because this patch exposed some paths where exceptions 
were not being handled correctly, and the errors are in the printers 
themselves. As they do not affect this patch directly, we'll fix these 
in parallel.

This patch was cut from:  archer-pmuldoon-pretty-printers-lookup 
branch.  When this email was being written, it should apply cleanly to 
the archer-tromey-python branch.

I have not added the ChangeLog to the patch, but included it here. I 
believe the policy of the archer-tromey-python branch has moved to 
ChangeLogs as part of the commit message.

Please do let me know  your comments and thoughts on this change. I look 
forward to hearing them.

Regards

Phil

Documentation ChangeLog.

2009-02-17  Phil Muldoon  <pmuldoon@redhat.com>

    * gdb.texinfo (Types From Inferior): Add strip_typedef entry.
    (Pretty Printing): Update explanation for printer selection,
    update example, and add a note on lookup function.

Testsuite ChangeLog

2009-02-17  Phil Muldoon  <pmuldoon@redhat.com>

    * gdb.python/python-prettyprint.py: Move printer registration to
    register_pretty_printers. Register lookup_function with
    gdb.pretty_printers list.
    (lookup_function): New    Function.
    (register_pretty_printers): New Function.


GDB ChangeLog
2009-02-17  Phil Muldoon  <pmuldoon@redhat.com>

    * varobj.c (struct varobj): Delete constructor member.
    (instantiate_pretty_printer): Do not instantiate from stored
    varobj constructor, but from constructor passed as an argument.
    (install_new_value): Check varobj->pretty_printer existence for
    changeable state.
    (install_new_value): Do not lazily instantiate pretty-printer.
    (install_visualizer): Remove constructor -> instantiate
    pretty-printer logic.
    (install_default_visualizer): Check value over type. Add None
    check for pretty printer. Pass instantiation of pretty-printer to
    install-visualizer.
    (varobj_set_visualizer): Add None check. Remove constructor logic and
    only deal with instantiations.
    (new_variable): Remove constructor member initialization.
    (free_variable): Remove constructor member python decref.
    (c_value_of_variable): Check pretty-printer over constructor.
    * python/python.c (search_pp_list): Rename from
    search_pp_dictionary. Rewrite to iterate a list.
    (find_pretty_printer): Take a value instead of a type, and
    similarly pass a value to search_pp_list. Convert all dictionary
    checks to list checks. Do not extract type information from value.
    (print_children): Add an  iterator error check, and print
    out exception if one occurs.
    (apply_val_pretty_printer): Adapt to find_pretty_printer list
    searching behaviour, and account for None and NULL returns.
    (gdbpy_get_varobj_pretty_printer): Receive a value, and similarly
    pass a value to find_pretty_printer.
    (gdbpy_default_visualizer): Do not instantiate constructor. Return
    a pretty printer from find_pretty_printer based on value.
    (_initialize_python): Convert gdb.pretty_printer to a list.
    * python/python-type.c (typy_strip_typedefs): New function
    Declare in type_object_methods[].
    * python/python-objfile.c (objfpy_new): Create an new list instead
    of a dictionary.
    (objfile_to_objfile_object): Likewise.
    (objfpy_set_printers): Check for a list, instead of a dictionary.
    * python/python-internal.h: Change gdbpy_get_varobj_pretty_printer
    declaration to take a value.
    * python/lib/gdb/libstdcxx/v6/printers.py
    (Tr1HashtableIterator.__init__): Use _M_element_count.
    (lookup_function): New function.
    (build_libstdcxx_dictionary): Register with compiled reg-ex as a
    key. Make dictionary private.


[-- Attachment #2: pretty_printer_reg.patch --]
[-- Type: text/x-patch, Size: 33246 bytes --]

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 4408635..2481ad8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18531,6 +18531,11 @@ target's @code{char} type will be an 8-bit byte.  However, on some
 unusual platforms, this type may have a different size.
 @end defmethod
 
+@defmethod Type strip_typedefs
+Return a new @code{gdb.Type} that represents the real type,
+after removing all layers of typedefs.
+@end defmethod
+
 @defmethod Type tag
 Return the tag name for this type.  The tag name is the name after
 @code{struct}, @code{union}, or @code{enum} in C; not all languages
@@ -18791,30 +18796,25 @@ convertible to @code{gdb.Value}, an exception is raised.
 
 @subsubsection Selecting Pretty-Printers
 
-The Python dictionary @code{gdb.pretty_printers} maps regular
-expressions (strings) onto constructors.  Each @code{gdb.Objfile} also
-contains a @code{pretty_printers} attribute.  A constructor, in this
-context, is a function which takes a single @code{gdb.Value} argument
-and returns a a pretty-printer conforming to the interface definition
-above.
-
-When printing a value, @value{GDBN} first computes the value's
-canonical type by following typedefs, following a reference type to
-its referenced type, and removing qualifiers, such as @code{const} or
-@code{volatile}.
+The Python list @code{gdb.pretty_printers} contains an array of
+functions that have been registered via addition as a
+pretty-printer. Each function will be called with a @code{gdb.Value}
+to be pretty-printed.  Each @code{gdb.Objfile} also contains a
+@code{pretty_printers} attribute. In both contexts, these functions
+takes a single @code{gdb.Value} argument and returns the output of the
+pretty-printer conforming to the interface definition above, or if the
+pretty-printer cannot print the value, returns the Python value: None.
 
-Then, @value{GDBN} tests each regular expression against this type name.
 @value{GDBN} first checks the @code{pretty_printers} attribute of each
-@code{gdb.Objfile}; after these have been exhausted it tries the
-global @code{gdb.pretty_printers}.
-
-If a regular expression matches, then the corresponding pretty-printer
-is invoked with a @code{gdb.Value} representing the value to be
-printed.
+@code{gdb.Objfile} and iteratively calls each function in the list for
+that @code{gdb.Objfile} until it receives pretty-printed output.
+After these @code{gdb.Objfile} have been exhausted, it tries the global
+@code{gdb.pretty-printers} list, again calling each function until
+output is generated.
 
 The order in which the objfiles are searched is not specified.
-Similarly, the order in which the regular expressions in a given
-dictionary are tried is not specified.
+Similarly, the order in which the functions are called in each list
+is not specified.
 
 Here is an example showing how a @code{std::string} printer might be
 written:
@@ -18823,13 +18823,33 @@ written:
 class StdStringPrinter:
     "Print a std::string"
 
-    def __init__(self, val):
+    def __init__ (self, val):
         self.val = val
 
-    def to_string(self):
+    def to_string (self):
         return self.val['_M_dataplus']['_M_p']
 @end smallexample
 
+How printers are selected is defined by the user. @value{GDBN}
+provides the value, and from this a decision returned whether the
+printer can print the value. For the example above, here is an example
+look-up function that determines whether to print the value, based on
+the type. If it is a type the printer is interested in, return an
+instance of the printer. If not, return: None.
+
+@smallexample
+def str_lookup_function (val):
+
+    lookup_tag = val.type ().tag ()
+    regex = re.compile ("^std::basic_string<char,.*>$")
+    if (lookup_tag  == None):
+        return None
+    if regex.match (lookup_tag):
+        return StdStringPrinter (val)
+    
+    return None
+@end smallexample
+
 We recommend that you put your core pretty-printers into a versioned
 python package, and then restrict your auto-loaded code to idempotent
 behavior -- for example, just @code{import}s of your printer modules,
@@ -18842,7 +18862,7 @@ For example, in addition to the above, this code might appear in
 
 @smallexample
 def register_printers (objfile):
-    objfile.pretty_printers['^std::basic_string<char.*>$'] = StdStringPrinter
+    objfile.pretty_printers.add (str_lookup_function)
 @end smallexample
 
 And then the corresponding contents of the auto-load file would be:
diff --git a/gdb/python/lib/gdb/libstdcxx/v6/printers.py b/gdb/python/lib/gdb/libstdcxx/v6/printers.py
index c9c8543..10cf234 100644
--- a/gdb/python/lib/gdb/libstdcxx/v6/printers.py
+++ b/gdb/python/lib/gdb/libstdcxx/v6/printers.py
@@ -1,6 +1,6 @@
 # Pretty-printers for libstc++.
 
-# Copyright (C) 2008 Free Software Foundation, Inc.
+# Copyright (C) 2008, 2009 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
@@ -17,6 +17,7 @@
 
 import gdb
 import itertools
+import re
 
 class StdPointerPrinter:
     "Print a smart pointer of some kind"
@@ -463,7 +464,7 @@ class StdStringPrinter:
 class Tr1HashtableIterator:
     def __init__ (self, hash):
         self.count = 0
-        self.n_buckets = hash['_M_bucket_count']
+        self.n_buckets = hash['_M_element_count']
         if self.n_buckets == 0:
             self.node = False
         else:
@@ -546,56 +547,90 @@ class Tr1UnorderedMapPrinter:
     def display_hint (self):
         return 'map'
 
-def register_libstdcxx_printers(obj):
+def register_libstdcxx_printers (obj):
     "Register libstdc++ pretty-printers with objfile Obj."
 
     if obj == None:
         obj = gdb
 
+    obj.pretty_printers.append(lookup_function)
+
+def lookup_function (val):
+    "Look-up and return a pretty-printer that can print val."
+
+    # Get the type.
+    type = val.type ();
+
+    # If it points to a reference, get the reference
+    if type.code () == gdb.TYPE_CODE_REF:
+        type = type.target ()
+
+    # Get the unqualified type, stripped of type defs
+    type = type.unqualified ().strip_typedefs ()
+
+    # Get the type name    
+    typename = type.tag ()
+    if (typename == None):
+        return None
+
+    # Iterate over local dictionary of types to determine
+    # if a printer is registered for that type. Return an
+    # instantiation of the printer if found.
+    for function in pretty_printers_dict:
+        if function.search (typename):
+            return pretty_printers_dict[function] (val)
+        
+    # Cannot find a pretty printer. Return None.
+    return None
+
+def build_libstdcxx_dictionary ():
     # libstdc++ objects requiring pretty-printing.
     # In order from:
     # http://gcc.gnu.org/onlinedocs/libstdc++/latest-doxygen/a01847.html
-    obj.pretty_printers['^std::basic_string<char,.*>$'] = lambda val: StdStringPrinter(None, val)
-    obj.pretty_printers['^std::basic_string<wchar_t,.*>$'] = lambda val: StdStringPrinter(target_wide_charset, val)
-    obj.pretty_printers['^std::basic_string<char16_t,.*>$'] = lambda val: StdStringPrinter('UTF-16', val)
-    obj.pretty_printers['^std::basic_string<char32_t,.*>$'] = lambda val: StdStringPrinter('UTF-32', val)
-    obj.pretty_printers['^std::bitset<.*>$'] = StdBitsetPrinter
-    obj.pretty_printers['^std::deque<.*>$'] = StdDequePrinter
-    obj.pretty_printers['^std::list<.*>$'] = StdListPrinter
-    obj.pretty_printers['^std::map<.*>$'] = lambda val: StdMapPrinter("std::map", val)
-    obj.pretty_printers['^std::multimap<.*>$'] = lambda val: StdMapPrinter("std::multimap", val)
-    obj.pretty_printers['^std::multiset<.*>$'] = lambda val: StdSetPrinter("std::multiset", val)
-    obj.pretty_printers['^std::priority_queue<.*>$'] = lambda val: StdStackOrQueuePrinter("std::priority_queue", val)
-    obj.pretty_printers['^std::queue<.*>$'] = lambda val: StdStackOrQueuePrinter("std::queue", val)
-    obj.pretty_printers['^std::set<.*>$'] = lambda val: StdSetPrinter("std::set", val)
-    obj.pretty_printers['^std::stack<.*>$'] = lambda val: StdStackOrQueuePrinter("std::stack", val)
-    obj.pretty_printers['^std::vector<.*>$'] = StdVectorPrinter
+    pretty_printers_dict[re.compile('^std::basic_string<char,.*>$')] = lambda val: StdStringPrinter(None, val)
+    pretty_printers_dict[re.compile('^std::basic_string<wchar_t,.*>$')] = lambda val: StdStringPrinter(target_wide_charset, val)
+    pretty_printers_dict[re.compile('^std::basic_string<char16_t,.*>$')] = lambda val: StdStringPrinter('UTF-16', val)
+    pretty_printers_dict[re.compile('^std::basic_string<char32_t,.*>$')] = lambda val: StdStringPrinter('UTF-32', val)
+    pretty_printers_dict[re.compile('^std::bitset<.*>$')] = StdBitsetPrinter
+    pretty_printers_dict[re.compile('^std::deque<.*>$')] = StdDequePrinter
+    pretty_printers_dict[re.compile('^std::list<.*>$')] = StdListPrinter
+    pretty_printers_dict[re.compile('^std::map<.*>$')] = lambda val: StdMapPrinter("std::map", val)
+    pretty_printers_dict[re.compile('^std::multimap<.*>$')] = lambda val: StdMapPrinter("std::multimap", val)
+    pretty_printers_dict[re.compile('^std::multiset<.*>$')] = lambda val: StdSetPrinter("std::multiset", val)
+    pretty_printers_dict[re.compile('^std::priority_queue<.*>$')] = lambda val: StdStackOrQueuePrinter("std::priority_queue", val)
+    pretty_printers_dict[re.compile('^std::queue<.*>$')] = lambda val: StdStackOrQueuePrinter("std::queue", val)
+    pretty_printers_dict[re.compile('^std::set<.*>$')] = lambda val: StdSetPrinter("std::set", val)
+    pretty_printers_dict[re.compile('^std::stack<.*>$')] = lambda val: StdStackOrQueuePrinter("std::stack", val)
+    pretty_printers_dict[re.compile('^std::vector<.*>$')] = StdVectorPrinter
     # vector<bool>
 
     # C++0x stuff.
     # array - the default seems reasonable
     # smart_ptr?  seems to only be in boost right now
-    obj.pretty_printers['^std::tr1::shared_ptr<.*>$'] = lambda val: StdPointerPrinter ('std::shared_ptr', val)
-    obj.pretty_printers['^std::tr1::weak_ptr<.*>$'] = lambda val: StdPointerPrinter ('std::weak_ptr', val)
-    obj.pretty_printers['^std::tr1::unique_ptr<.*>$'] = UniquePointerPrinter
-    obj.pretty_printers['^std::tr1::unordered_map<.*>$'] = lambda val: Tr1UnorderedMapPrinter ('std::tr1::unordered_map', val)
-    obj.pretty_printers['^std::tr1::unordered_set<.*>$'] = lambda val: Tr1UnorderedSetPrinter ('std::tr1::unordered_set', val)
-    obj.pretty_printers['^std::tr1::unordered_multimap<.*>$'] = lambda val: Tr1UnorderedMapPrinter ('std::tr1::unordered_multimap', val)
-    obj.pretty_printers['^std::tr1::unordered_multiset<.*>$'] = lambda val: Tr1UnorderedSetPrinter ('std::tr1::unordered_multiset', val)
+    pretty_printers_dict[re.compile('^std::tr1::shared_ptr<.*>$')] = lambda val: StdPointerPrinter ('std::shared_ptr', val)
+    pretty_printers_dict[re.compile('^std::tr1::weak_ptr<.*>$')] = lambda val: StdPointerPrinter ('std::weak_ptr', val)
+    pretty_printers_dict[re.compile('^std::tr1::unique_ptr<.*>$')] = UniquePointerPrinter
+    pretty_printers_dict[re.compile('^std::tr1::unordered_map<.*>$')] = lambda val: Tr1UnorderedMapPrinter ('std::tr1::unordered_map', val)
+    pretty_printers_dict[re.compile('^std::tr1::unordered_set<.*>$')] = lambda val: Tr1UnorderedSetPrinter ('std::tr1::unordered_set', val)
+    pretty_printers_dict[re.compile('^std::tr1::unordered_multimap<.*>$')] = lambda val: Tr1UnorderedMapPrinter ('std::tr1::unordered_multimap', val)
+    pretty_printers_dict[re.compile('^std::tr1::unordered_multiset<.*>$')] = lambda val: Tr1UnorderedSetPrinter ('std::tr1::unordered_multiset', val)
 
     # Extensions.
-    obj.pretty_printers['^__gnu_cxx::slist<.*>$'] = StdSlistPrinter
+    pretty_printers_dict[re.compile('^__gnu_cxx::slist<.*>$')] = StdSlistPrinter
 
     if True:
         # These shouldn't be necessary, if GDB "print *i" worked.
         # But it often doesn't, so here they are.
-        obj.pretty_printers['^std::_List_iterator<.*>$'] = lambda val: StdListIteratorPrinter(val)
-        obj.pretty_printers['^std::_List_const_iterator<.*>$'] = lambda val: StdListIteratorPrinter(val)
-        obj.pretty_printers['^std::_Rb_tree_iterator<.*>$'] = lambda val: StdRbtreeIteratorPrinter(val)
-        obj.pretty_printers['^std::_Rb_tree_const_iterator<.*>$'] = lambda val: StdRbtreeIteratorPrinter(val)
-        obj.pretty_printers['^std::_Deque_iterator<.*>$'] = lambda val: StdDequeIteratorPrinter(val)
-        obj.pretty_printers['^std::_Deque_const_iterator<.*>$'] = lambda val: StdDequeIteratorPrinter(val)
-        obj.pretty_printers['^__gnu_cxx::__normal_iterator<.*>$'] = lambda val: StdVectorIteratorPrinter(val)
-        obj.pretty_printers['^__gnu_cxx::_Slist_iterator<.*>$'] = lambda val: StdSlistIteratorPrinter(val)
-
+        pretty_printers_dict[re.compile('^std::_List_iterator<.*>$')] = lambda val: StdListIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^std::_List_const_iterator<.*>$')] = lambda val: StdListIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^std::_Rb_tree_iterator<.*>$')] = lambda val: StdRbtreeIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^std::_Rb_tree_const_iterator<.*>$')] = lambda val: StdRbtreeIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^std::_Deque_iterator<.*>$')] = lambda val: StdDequeIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^std::_Deque_const_iterator<.*>$')] = lambda val: StdDequeIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^__gnu_cxx::__normal_iterator<.*>$')] = lambda val: StdVectorIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^__gnu_cxx::_Slist_iterator<.*>$')] = lambda val: StdSlistIteratorPrinter(val)
+
+pretty_printers_dict = {}
+
+build_libstdcxx_dictionary ()
 register_libstdcxx_printers (gdb.current_objfile())
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index afdf821..83e5a4b 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -161,7 +161,7 @@ int gdbpy_is_string (PyObject *obj);
    other pretty-printer functions, because they refer to PyObject.  */
 char *apply_varobj_pretty_printer (PyObject *print_obj, struct value *value,
 				   struct value **replacement);
-PyObject *gdbpy_get_varobj_pretty_printer (struct type *type);
+PyObject *gdbpy_get_varobj_pretty_printer (struct value *value);
 PyObject *gdbpy_instantiate_printer (PyObject *cons, struct value *value);
 char *gdbpy_get_display_hint (PyObject *printer);
 
diff --git a/gdb/python/python-objfile.c b/gdb/python/python-objfile.c
index a225409..45e53cc 100644
--- a/gdb/python/python-objfile.c
+++ b/gdb/python/python-objfile.c
@@ -1,6 +1,6 @@
 /* Python interface to objfiles.
 
-   Copyright (C) 2008 Free Software Foundation, Inc.
+   Copyright (C) 2008, 2009 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -29,7 +29,7 @@ typedef struct
   /* The corresponding objfile.  */
   struct objfile *objfile;
 
-  /* The pretty-printer dictionary.  */
+  /* The pretty-printer list of function.  */
   PyObject *printers;
 } objfile_object;
 
@@ -66,7 +66,7 @@ objfpy_new (PyTypeObject *type, PyObject *args, PyObject *keywords)
     {
       self->objfile = NULL;
 
-      self->printers = PyDict_New ();
+      self->printers = PyList_New (0);
       if (!self->printers)
 	{
 	  Py_DECREF (self);
@@ -95,10 +95,10 @@ objfpy_set_printers (PyObject *o, PyObject *value, void *ignore)
       return -1;
     }
 
-  if (! PyDict_Check (value))
+  if (! PyList_Check (value))
     {
       PyErr_SetString (PyExc_TypeError,
-		       "the pretty_printers attribute must be a dictionary");
+		       "the pretty_printers attribute must be a list");
       return -1;
     }
 
@@ -139,7 +139,7 @@ objfile_to_objfile_object (struct objfile *objfile)
 
 	  object->objfile = objfile;
 
-	  object->printers = PyDict_New ();
+	  object->printers = PyList_New (0);
 	  if (!object->printers)
 	    {
 	      Py_DECREF (object);
diff --git a/gdb/python/python-type.c b/gdb/python/python-type.c
index 5aa82f6..857d4c7 100644
--- a/gdb/python/python-type.c
+++ b/gdb/python/python-type.c
@@ -1,6 +1,6 @@
 /* Python interface to types.
 
-   Copyright (C) 2008 Free Software Foundation, Inc.
+   Copyright (C) 2008, 2009 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -233,6 +233,15 @@ typy_tag (PyObject *self, PyObject *args)
   return PyString_FromString (TYPE_TAG_NAME (type));
 }
 
+/* Return the type, stripped of TypeDefs */
+static PyObject *
+typy_strip_typedefs (PyObject *self, PyObject *args)
+{
+  struct type *type = ((type_object *) self)->type;
+
+  return type_to_type_object (CHECK_TYPEDEF (type));
+}
+
 /* Return a Type object which represents a pointer to SELF.  */
 static PyObject *
 typy_pointer (PyObject *self, PyObject *args)
@@ -710,6 +719,8 @@ Each field is a dictionary." },
     "Return the size of this type, in bytes" },
   { "tag", typy_tag, METH_NOARGS,
     "Return the tag name for this type, or None." },
+  { "strip_typedefs", typy_strip_typedefs, METH_NOARGS,
+    "Return a type stripped of typedefs"},
   { "target", typy_target, METH_NOARGS,
     "Return the target type of this type" },
   { "template_argument", typy_template_argument, METH_VARARGS,
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 8677da6..45c05db 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -842,52 +842,37 @@ get_type (struct type *type)
 }
 
 /* Helper function for find_pretty_printer which iterates over a
-   dictionary and tries to find a match.  */
+   list, calls each function and inspects output.  */
 static PyObject *
-search_pp_dictionary (PyObject *dict, char *type_name)
+search_pp_list (PyObject *list, struct value *value)
 {
-  Py_ssize_t iter;
-  PyObject *key, *func, *found = NULL;
-
-  /* See if the type matches a pretty-printer regexp.  */
-  iter = 0;
-  while (! found && PyDict_Next (dict, &iter, &key, &func))
+  Py_ssize_t pp_list_size, list_index;
+  PyObject *function, *printer = NULL;
+  
+  pp_list_size = PyList_Size (list);
+  for (list_index = 0; list_index < pp_list_size; list_index++)
     {
-      char *rx_str;
-
-      if (! PyString_Check (key))
-	continue;
-      rx_str = PyString_AsString (key);
-      if (re_comp (rx_str) == NULL && re_exec (type_name) == 1)
-	found = func;
+      function = PyList_GetItem (list, list_index);
+      printer = gdbpy_instantiate_printer (function, value);
+      if (PyErr_Occurred ())
+	gdbpy_print_stack ();
+      if ((printer) && (printer != Py_None))
+	return printer;      
     }
 
-  return found;
+  return Py_None;
 }
 
 /* Find the pretty-printing constructor function for TYPE.  If no
    pretty-printer exists, return NULL.  If one exists, return a new
    reference.  */
 static PyObject *
-find_pretty_printer (struct type *type)
+find_pretty_printer (struct value *value)
 {
-  PyObject *dict, *found = NULL;
-  char *type_name = NULL;
-  struct objfile *obj;
-  volatile struct gdb_exception except;
 
-  /* Get the name of the type.  */
-  TRY_CATCH (except, RETURN_MASK_ALL)
-    {
-      /* If we have a reference, use the referenced type.  */
-      if (TYPE_CODE (type) == TYPE_CODE_REF)
-	type = TYPE_TARGET_TYPE (type);
-      /* Strip off any qualifiers from the type.  */
-      type = make_cv_type (0, 0, type, NULL);
-      type_name = get_type (type);
-    }
-  if (except.reason < 0)
-    return NULL;
+  PyObject *pp_list = NULL;
+  PyObject *function = NULL;
+  struct objfile *obj;
 
   /* Look at the pretty-printer dictionary for each objfile.  */
   ALL_OBJFILES (obj)
@@ -896,34 +881,33 @@ find_pretty_printer (struct type *type)
     if (!objf)
       continue;
 
-    dict = objfpy_get_printers (objf, NULL);
-    found = search_pp_dictionary (dict, type_name);
-    if (found)
+    pp_list = objfpy_get_printers (objf, NULL);
+    function = search_pp_list (pp_list, value);
+    if (function != Py_None)
       goto done;
 
-    Py_DECREF (dict);
+    Py_DECREF (pp_list);
   }
 
   /* Fetch the global pretty printer dictionary.  */
-  dict = NULL;
+  pp_list = NULL;
   if (! PyObject_HasAttrString (gdb_module, "pretty_printers"))
     goto done;
-  dict = PyObject_GetAttrString (gdb_module, "pretty_printers");
-  if (! dict)
+  pp_list = PyObject_GetAttrString (gdb_module, "pretty_printers");
+  if (! pp_list)
     goto done;
-  if (! PyDict_Check (dict) || ! PyDict_Size (dict))
+  if (! PyList_Check (pp_list) || ! PyList_Size (pp_list))
     goto done;
 
-  found = search_pp_dictionary (dict, type_name);
+  function = search_pp_list (pp_list, value);
 
  done:
-  xfree (type_name);
-
-  if (found)
-    Py_INCREF (found);
-  Py_XDECREF (dict);
+  if (! function)
+      function = Py_None;
+  Py_XDECREF (pp_list);
+  Py_INCREF (function);
 
-  return found;
+  return function;
 }
 
 /* Pretty-print a single value, via the printer object PRINTER.  If
@@ -1174,6 +1158,8 @@ print_children (PyObject *printer, const char *hint,
 
       if (! item)
 	{
+	  if (PyErr_Occurred ())
+	    gdbpy_print_stack ();
 	  /* Set a flag so we can know whether we printed all the
 	     available elements.  */
 	  done_flag = 1;
@@ -1296,7 +1282,7 @@ apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
 			  const struct value_print_options *options,
 			  const struct language_defn *language)
 {
-  PyObject *func, *printer;
+  PyObject *printer = NULL;
   struct value *value;
   char *hint = NULL;
   struct cleanup *cleanups;
@@ -1306,36 +1292,26 @@ apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
   state = PyGILState_Ensure ();
   cleanups = make_cleanup_py_restore_gil (&state);
 
-  /* Find the constructor.  */
-  func = find_pretty_printer (type);
-  if (! func)
-    goto done;
-
   /* Instantiate the printer.  */
   if (valaddr)
     valaddr += embedded_offset;
   value = value_from_contents_and_address (type, valaddr, address);
-  printer = gdbpy_instantiate_printer (func, value);
-  Py_DECREF (func);
 
-  if (!printer)
-    {
-      gdbpy_print_stack ();
-      goto done;
-    }
+  /* Find the constructor.  */
+  printer = find_pretty_printer (value);
+  make_cleanup_py_decref (printer);
+  if (printer == Py_None)
+    goto done;
 
   /* If we are printing a map, we want some special formatting.  */
   hint = gdbpy_get_display_hint (printer);
   make_cleanup (free_current_contents, &hint);
 
-  make_cleanup_py_decref (printer);
-  if (printer != Py_None)
-    {
-      print_string_repr (printer, hint, stream, recurse, options, language);
-      print_children (printer, hint, stream, recurse, options, language);
+  /* Print the section */
+  print_string_repr (printer, hint, stream, recurse, options, language);
+  print_children (printer, hint, stream, recurse, options, language);
+  result = 1;
 
-      result = 1;
-    }
 
  done:
   do_cleanups (cleanups);
@@ -1371,9 +1347,9 @@ apply_varobj_pretty_printer (PyObject *printer_obj, struct value *value,
    is the type of the varobj for which a printer should be
    returned.  */
 PyObject *
-gdbpy_get_varobj_pretty_printer (struct type *type)
+gdbpy_get_varobj_pretty_printer (struct value *value)
 {
-  return find_pretty_printer (type);
+  return find_pretty_printer (value);
 }
 
 /* A Python function which wraps find_pretty_printer and instantiates
@@ -1396,24 +1372,8 @@ gdbpy_default_visualizer (PyObject *self, PyObject *args)
       return NULL;
     }
 
-  cons = find_pretty_printer (value_type (value));
-  if (cons)
-    {
-      /* While it is a bit lame to pass value here and make a new
-	 Value, it is probably better to share the instantiation
-	 code.  */
-      printer = gdbpy_instantiate_printer (cons, value);
-      Py_DECREF (cons);
-    }
-
-  if (!printer)
-    {
-      PyErr_Clear ();
-      printer = Py_None;
-      Py_INCREF (printer);
-    }
-
-  return printer;
+  cons = find_pretty_printer (value);
+  return cons      ;
 }
 
 #else /* HAVE_PYTHON */
@@ -1566,7 +1526,7 @@ Enables or disables auto-loading of Python code when an object is opened."),
   gdbpy_initialize_membuf ();
 
   PyRun_SimpleString ("import gdb");
-  PyRun_SimpleString ("gdb.pretty_printers = {}");
+  PyRun_SimpleString ("gdb.pretty_printers = []");
 
   observer_attach_new_objfile (gdbpy_new_objfile);
 
diff --git a/gdb/testsuite/gdb.python/python-prettyprint.py b/gdb/testsuite/gdb.python/python-prettyprint.py
index ecd4fc6..352024c 100644
--- a/gdb/testsuite/gdb.python/python-prettyprint.py
+++ b/gdb/testsuite/gdb.python/python-prettyprint.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2008 Free Software Foundation, Inc.
+# Copyright (C) 2008, 2009 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
@@ -16,6 +16,8 @@
 # This file is part of the GDB testsuite.  It tests python pretty
 # printers.
 
+import re
+
 # Test returning a Value from a printer.
 class string_print:
     def __init__(self, val):
@@ -76,21 +78,56 @@ class pp_sss:
     def to_string(self):
         return "a=<" + str(self.val['a']) + "> b=<" + str(self.val["b"]) + ">"
 
-gdb.pretty_printers['^struct s$']   = pp_s
-gdb.pretty_printers['^s$']   = pp_s
-gdb.pretty_printers['^S$']   = pp_s
-
-gdb.pretty_printers['^struct ss$']  = pp_ss
-gdb.pretty_printers['^ss$']  = pp_ss
-
-gdb.pretty_printers['^const S &$']   = pp_s
-gdb.pretty_printers['^SSS$']  = pp_sss
-
-# Note that we purposely omit the typedef names here.
-# Printer lookup is based on canonical name.
-# However, we do need both tagged and untagged variants, to handle
-# both the C and C++ cases.
-gdb.pretty_printers['^struct string_repr$'] = string_print
-gdb.pretty_printers['^struct container$'] = ContainerPrinter
-gdb.pretty_printers['^string_repr$'] = string_print
-gdb.pretty_printers['^container$'] = ContainerPrinter
+def lookup_function (val):
+    "Look-up and return a pretty-printer that can print val."
+
+    # Get the type.
+    type = val.type ();
+
+    # If it points to a reference, get the reference
+    if type.code () == gdb.TYPE_CODE_REF:
+        type = type.target ()
+
+    # Get the unqualified type, stripped of type defs
+    type = type.unqualified ().strip_typedefs ()
+
+    # Get the type name    
+    typename = type.tag ()
+    if (typename == None):
+        return None
+
+    # Iterate over local dictionary of types to determine
+    # if a printer is registered for that type. Return an
+    # instantiation of the printer if found.
+    for function in pretty_printers_dict:
+        if function.match (typename):
+            return pretty_printers_dict[function] (val)
+        
+    # Cannot find a pretty printer. Return None.
+
+    return None
+
+
+def register_pretty_printers ():
+    pretty_printers_dict[re.compile ('^struct s$')]   = pp_s
+    pretty_printers_dict[re.compile ('^s$')]   = pp_s
+    pretty_printers_dict[re.compile ('^S$')]   = pp_s
+
+    pretty_printers_dict[re.compile ('^struct ss$')]  = pp_ss
+    pretty_printers_dict[re.compile ('^ss$')]  = pp_ss
+    pretty_printers_dict[re.compile ('^const S &$')]   = pp_s
+    pretty_printers_dict[re.compile ('^SSS$')]  = pp_sss
+    
+    # Note that we purposely omit the typedef names here.
+    # Printer lookup is based on canonical name.
+    # However, we do need both tagged and untagged variants, to handle
+    # both the C and C++ cases.
+    pretty_printers_dict[re.compile ('^struct string_repr$')] = string_print
+    pretty_printers_dict[re.compile ('^struct container$')] = ContainerPrinter
+    pretty_printers_dict[re.compile ('^string_repr$')] = string_print
+    pretty_printers_dict[re.compile ('^container$')] = ContainerPrinter
+    
+pretty_printers_dict = {}
+
+register_pretty_printers ()
+gdb.pretty_printers.append (lookup_function)
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 7f77902..5eeb26c 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -181,10 +181,6 @@ struct varobj
   int from;
   int to;
 
-  /* If NULL, no pretty printer is installed.  If not NULL, the
-     constructor to call to get a pretty-printer object.  */
-  PyObject *constructor;
-
   /* The pretty-printer that has been constructed.  If NULL, then a
      new printer object is needed, and one will be constructed.  */
   PyObject *pretty_printer;
@@ -713,23 +709,17 @@ varobj_delete (struct varobj *var, char ***dellist, int only_children)
 
 /* Instantiate a pretty-printer for a given value.  */
 static PyObject *
-instantiate_pretty_printer (struct varobj *var, struct value *value)
+instantiate_pretty_printer (PyObject *constructor, struct value *value)
 {
 #if HAVE_PYTHON
-  if (var->constructor)
-    {
-      PyObject *printer = gdbpy_instantiate_printer (var->constructor, value);
-      if (printer == Py_None)
-	{
-	  Py_DECREF (printer);
-	  printer = NULL;
-	}
-      return printer;
-    }
+
+  PyObject *printer = gdbpy_instantiate_printer (constructor, value);
+  return printer;
 #endif
   return NULL;
 }
 
+
 /* Set/Get variable object display format */
 
 enum varobj_display_formats
@@ -1224,7 +1214,7 @@ install_new_value (struct varobj *var, struct value *value, int initial)
   /* If the type has custom visualizer, we consider it to be always
      changeable. FIXME: need to make sure this behaviour will not
      mess up read-sensitive values.  */
-  if (var->constructor)
+  if (var->pretty_printer)
     changeable = 1;
 
   need_to_fetch = changeable;
@@ -1278,20 +1268,6 @@ install_new_value (struct varobj *var, struct value *value, int initial)
 	}
     }
 
-#if HAVE_PYTHON
-  {
-    PyGILState_STATE state = PyGILState_Ensure ();
-    if (var->pretty_printer)
-      {
-	Py_DECREF (var->pretty_printer);
-      }
-    if (value)
-      var->pretty_printer = instantiate_pretty_printer (var, value);
-    else
-      var->pretty_printer = NULL;
-    PyGILState_Release (state);
-  }
-#endif
 
   /* Below, we'll be comparing string rendering of old and new
      values.  Don't get string rendering if the value is
@@ -1408,11 +1384,9 @@ install_visualizer (struct varobj *var, PyObject *visualizer)
   varobj_delete (var, NULL, 1 /* children only */);
   var->num_children = -1;
 
-  Py_XDECREF (var->constructor);
-  var->constructor = visualizer;
-
   Py_XDECREF (var->pretty_printer);
-  var->pretty_printer = NULL;
+  var->pretty_printer = visualizer;
+
 
   install_new_value (var, var->value, 1);
 
@@ -1433,14 +1407,21 @@ install_default_visualizer (struct varobj *var)
 #if HAVE_PYTHON
   struct cleanup *cleanup;
   PyGILState_STATE state;
-  PyObject *constructor = NULL;
+  PyObject *pretty_printer = NULL;
 
   state = PyGILState_Ensure ();
   cleanup = make_cleanup_py_restore_gil (&state);
 
-  if (var->type)
-    constructor = gdbpy_get_varobj_pretty_printer (var->type);
-  install_visualizer (var, constructor);
+  if (var->value)
+      pretty_printer = gdbpy_get_varobj_pretty_printer (var->value);
+
+  if (pretty_printer == Py_None)
+    {
+      Py_DECREF (pretty_printer);
+      pretty_printer = NULL;
+    }
+  
+  install_visualizer (var, pretty_printer);
 
   do_cleanups (cleanup);
 #else
@@ -1452,7 +1433,7 @@ void
 varobj_set_visualizer (struct varobj *var, const char *visualizer)
 {
 #if HAVE_PYTHON
-  PyObject *mainmod, *globals, *constructor;
+  PyObject *mainmod, *globals, *pretty_printer, *constructor;
   struct cleanup *back_to;
   PyGILState_STATE state;
 
@@ -1465,21 +1446,28 @@ varobj_set_visualizer (struct varobj *var, const char *visualizer)
   make_cleanup_py_decref (globals);
 
   constructor = PyRun_String (visualizer, Py_eval_input, globals, globals);
+  
+  // Do not instantiate NoneType.
+  if (constructor == Py_None)
+    pretty_printer = Py_None;
+  else
+    pretty_printer = instantiate_pretty_printer(constructor, var->value);
 
-  if (! constructor)
+  if (! pretty_printer)
     {
       gdbpy_print_stack ();
       error ("Could not evaluate visualizer expression: %s", visualizer);
     }
 
-  if (constructor == Py_None)
+  if (pretty_printer == Py_None)
     {
-      Py_DECREF (constructor);
-      constructor = NULL;
+      Py_DECREF (pretty_printer);
+      pretty_printer = NULL;
     }
 
-  install_visualizer (var, constructor);
+  install_visualizer (var, pretty_printer);
 
+  Py_DECREF(constructor);
   do_cleanups (back_to);
 #else
   error ("Python support required");
@@ -1917,7 +1905,6 @@ new_variable (void)
   var->children_requested = 0;
   var->from = -1;
   var->to = -1;
-  var->constructor = 0;
   var->pretty_printer = 0;
 
   return var;
@@ -1954,7 +1941,6 @@ free_variable (struct varobj *var)
 #if HAVE_PYTHON
   {
     PyGILState_STATE state = PyGILState_Ensure ();
-    Py_XDECREF (var->constructor);
     Py_XDECREF (var->pretty_printer);
     PyGILState_Release (state);
   }
@@ -2753,7 +2739,7 @@ c_value_of_variable (struct varobj *var, enum varobj_display_formats format)
 
   /* If we have a custom formatter, return whatever string it has
      produced.  */
-  if (var->constructor && var->print_value)
+  if (var->pretty_printer && var->print_value)
     return xstrdup (var->print_value);
   
   /* Strip top-level references. */

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

* Re: [python] [rfc] Patch for pretty-printers registration API change
  2009-02-17 14:03 [python] [rfc] Patch for pretty-printers registration API change Phil Muldoon
@ 2009-02-17 18:52 ` Tom Tromey
  2009-02-18 14:40   ` Phil Muldoon
  0 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2009-02-17 18:52 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: Project Archer

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> Please do let me know  your comments and thoughts on this change. I
Phil> look forward to hearing them.

This is looking pretty good.

I have lots of little nits.  Don't let them get you down, the most
important parts of this patch are ok.

Phil> +The Python list @code{gdb.pretty_printers} contains an array of
Phil> +functions that have been registered via addition as a
Phil> +pretty-printer. Each function will be called with a @code{gdb.Value}

GNU follows the American style and puts two spaces after a period.

Phil> +to be pretty-printed.  Each @code{gdb.Objfile} also contains a
Phil> +@code{pretty_printers} attribute. In both contexts, these functions
Phil> +takes a single @code{gdb.Value} argument and returns the output of the
Phil> +pretty-printer conforming to the interface definition above, or if the
Phil> +pretty-printer cannot print the value, returns the Python value: None.

This sentence reads strangely.  How about:

  A function on one of these lists takes a single @code{gdb.Value}
  argument and returns a pretty-printer object conforming to the
  interface definition above.  If this function cannot create a
  pretty-printer for the value, it should return @code{None}.

Phil> +@code{gdb.Objfile} and iteratively calls each function in the list for
Phil> +that @code{gdb.Objfile} until it receives pretty-printed output.

I think:  ... until it receives a pretty-printer object.

Phil> +After these @code{gdb.Objfile} have been exhausted, it tries the global
Phil> +@code{gdb.pretty-printers} list, again calling each function until
Phil> +output is generated.
 
"until an object is returned".

Phil> +Similarly, the order in which the functions are called in each list
Phil> +is not specified.
 
I think we do want to specify the order in the list case.  We should
always invoke the functions starting from the head of the list.  With
dictionaries there is no natural ordering; but with lists there is.

Phil> +How printers are selected is defined by the user. @value{GDBN}
Phil> +provides the value, and from this a decision returned whether the
Phil> +printer can print the value.

The grammar is weird here.  Also, avoid the passive voice, upstream
will ding you for this.

Phil> +the type. If it is a type the printer is interested in, return an
Phil> +instance of the printer. If not, return: None.

"return @code{None}"

Phil> +    if (lookup_tag  == None):

No parens here.  Also there is an extra space before the ==.
I know this is just an example in the docs but I think we want to
exhibit typical Python style.

Phil> -        self.n_buckets = hash['_M_bucket_count']
Phil> +        self.n_buckets = hash['_M_element_count']

I think we ought to leave this hunk out and then put it back in a
later patch -- one that fixes all the bugs in this iterator.

Phil> +    # If it points to a reference, get the reference

In the GNU style, a comment must be a full sentence, starting with a
capital letter and ending with a period.  This one misses the period,
but others have other problems.

Phil> +    # Get the unqualified type, stripped of type defs

Just "typedefs".  Also the period.

Phil> +    # Iterate over local dictionary of types to determine
Phil> +    # if a printer is registered for that type. Return an

Two spaces after a period in comments, too.

Phil> +/* Return the type, stripped of TypeDefs */

Just "typedefs".  And the elusive period.

Phil> +static PyObject *
Phil> +typy_strip_typedefs (PyObject *self, PyObject *args)
[...]
Phil> +  return type_to_type_object (CHECK_TYPEDEF (type));

CHECK_TYPEDEF is a macro typically called for side effects.
Here it is better to call check_typedef.

Phil> +search_pp_list (PyObject *list, struct value *value)

By taking a struct value, this function creates a new gdb.Value for
each printer.  If the printer returns None (which will be the most
common case), then the new Value is immediately destroyed.

Instead, I think it would be better to instantiate the Value once,
before looping over all the printers.  This means updating this to
take a PyObject and also updating gdbpy_instantiate_printer.

Phil> +      function = PyList_GetItem (list, list_index);

Needs a check for failure.

Phil> +      if (PyErr_Occurred ())
Phil> +	gdbpy_print_stack ();

I suspect that we should just propagate errors that occur while
calling functions from the list.  My thinking is that any error here
represents a bug in some Python code, and we shouldn't proceed past
that point.

However, I'm not certain about this.

Phil> +      if ((printer) && (printer != Py_None))

Too many parens.

Phil> +  return Py_None;

The function's introductory comment claims that it returns a new
reference.  So, you need Py_RETURN_NONE here.

Phil> +find_pretty_printer (struct value *value)
[...]

Phil> +  if (! PyList_Check (pp_list) || ! PyList_Size (pp_list))

I don't think we need the PyList_Size check here.
If it is zero, search_pp_list will just return.
 
Phil> +  if (! function)
Phil> +      function = Py_None;

Indentation looks wrong on the second line.

Phil> +  Py_INCREF (function);
 
If search_pp_list does return a new reference, then this is not needed.
Instead it has to be hoisted into the 'function = Py_None' line, above.

Phil> @@ -1174,6 +1158,8 @@ print_children (PyObject *printer, const char *hint,
 
Phil>        if (! item)
Phil>  	{
Phil> +	  if (PyErr_Occurred ())
Phil> +	    gdbpy_print_stack ();
Phil>  	  /* Set a flag so we can know whether we printed all the
Phil>  	     available elements.  */
Phil>  	  done_flag = 1;

I am not sure but I suspect this should be:

   if (PyErr_Occurred ())
     gdbpy_print_stack ();
   else
     {
        ...
     }

IOW, do we want to set done_flag if the iterator failed?

Maybe it doesn't really matter.

Phil> +  return cons      ;

Excess spaces.

Phil> +  if (var->value)
Phil> +      pretty_printer = gdbpy_get_varobj_pretty_printer (var->value);

The second line looks it is indented too far.

Phil> +  Py_DECREF(constructor);

Missing space before the "(".

thanks,
Tom

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

* Re: [python] [rfc] Patch for pretty-printers registration API change
  2009-02-17 18:52 ` Tom Tromey
@ 2009-02-18 14:40   ` Phil Muldoon
  2009-02-18 19:15     ` Tom Tromey
  0 siblings, 1 reply; 8+ messages in thread
From: Phil Muldoon @ 2009-02-18 14:40 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Project Archer

[-- Attachment #1: Type: text/plain, Size: 6133 bytes --]

Tom Tromey wrote:
> Phil> Please do let me know  your comments and thoughts on this change. I
> Phil> look forward to hearing them.
>
> This is looking pretty good.
>   


Thanks for the swift review. Attached patch with corrections. I've 
replied in-line to some of your comments.

> Phil> +The Python list @code{gdb.pretty_printers} contains an array of
> Phil> +functions that have been registered via addition as a
> Phil> +pretty-printer. Each function will be called with a @code{gdb.Value}
>
> GNU follows the American style and puts two spaces after a period.
>
>   

...

I've fixed all the noted documentation issues. Please scan the 
documentation again and let me know if they need some further polishing.


> Phil> -        self.n_buckets = hash['_M_bucket_count']
> Phil> +        self.n_buckets = hash['_M_element_count']
>
> I think we ought to leave this hunk out and then put it back in a
> later patch -- one that fixes all the bugs in this iterator.
>   


Good catch -- this one snuck into the patch. Was not even supposed to be 
in the submission today. I've reverted the change in the libstdcxx 
printers and removed the entry from the ChangeLog.


> Phil> +    # If it points to a reference, get the reference
>
> In the GNU style, a comment must be a full sentence, starting with a
> capital letter and ending with a period.  This one misses the period,
> but others have other problems.
>   


....

I fixed all the comments issues here. I also fixed: 
gdb.python/python-testsuite.py as it had the same issues (that I 
inflicted on it ;))


> Phil> +static PyObject *
> Phil> +typy_strip_typedefs (PyObject *self, PyObject *args)
> [...]
> Phil> +  return type_to_type_object (CHECK_TYPEDEF (type));
>
> CHECK_TYPEDEF is a macro typically called for side effects.
> Here it is better to call check_typedef.
>   


Done.


> Phil> +search_pp_list (PyObject *list, struct value *value)
>
> By taking a struct value, this function creates a new gdb.Value for
> each printer.  If the printer returns None (which will be the most
> common case), then the new Value is immediately destroyed.I've
>
> Instead, I think it would be better to instantiate the Value once,
> before looping over all the printers.  This means updating this to
> take a PyObject and also updating gdbpy_instantiate_printer.\\
>   


I've made the change so that the Value object is created only once; 
after being copied and converted it is destroyed after a printer is 
found, or the search for a printer has been exhausted. I ended up having 
to do this in two places: once in the MI code, and another time in the 
Python code (as both have places that can instantiate a printer). I 
thought about making a convenience function to do the copy/convert but 
decided against it -- I would have to export the function from python.c. 
We can can change it to a convenience function if people think it better 
that way.


> Phil> +      function = PyList_GetItem (list, list_index);
>
> Needs a check for failure.
>
> Phil> +      if (PyErr_Occurred ())
> Phil> +	gdbpy_print_stack ();
>
> I suspect that we should just propagate errors that occur while
> calling functions from the list.  My thinking is that any error here
> represents a bug in some Python code, and we shouldn't proceed past
> that point.
>
> However, I'm not certain about this.
>   


These two actually occur in the same loop. I was (and still am!) not 
quite sure what to do in this instance. As the code is iterating through 
a list, there is a possibility that multiple exceptions may occur that 
will overwrite each other? When at last the exception is printed out as 
it propagates upwards it might be the nth exception, that may mislead 
the user? I'm just not sure. I'll defer to expertise in this matter. For 
now I removed the PyErr_Occurred () -> gdbpy_print_stack () condition. I 
also decided to return Py_None if the PyList_GetItem fails. Maybe at the 
point directly after the GetItem call we should have an exception check.


> Phil> +      if ((printer) && (printer != Py_None))
>
> Too many parens.
>
> Phil> +  return Py_None;
>
> The function's introductory comment claims that it returns a new
> reference.  So, you need Py_RETURN_NONE here.
>
> Phil> +find_pretty_printer (struct value *value)
> [...]
>
> Phil> +  if (! PyList_Check (pp_list) || ! PyList_Size (pp_list))
>
> I don't think we need the PyList_Size check here.
> If it is zero, search_pp_list will just return.r
>  
> Phil> +  if (! function)
> Phil> +      function = Py_None;
>
> Indentation looks wrong on the second line.
>   


Fixed all of the above.


> Phil> +  Py_INCREF (function);
>  
> If search_pp_list does return a new reference, then this is not needed.
> Instead it has to be hoisted into the 'function = Py_None' line, above.
>   


Fixed. (That is, I moved the INCREF into the "if statement" above, to 
increment the count to Py_None)


> Phil> @@ -1174,6 +1158,8 @@ print_children (PyObject *printer, const char *hint,
>  
> Phil>        if (! item)
> Phil>  	{
> Phil> +	  if (PyErr_Occurred ())
> Phil> +	    gdbpy_print_stack ();
> Phil>  	  /* Set a flag so we can know whether we printed all the
> Phil>  	     available elements.  */
> Phil>  	  done_flag = 1;
>
> I am not sure but I suspect this should be:
>
>    if (PyErr_Occurred ())
>      gdbpy_print_stack ();
>    else
>      {
>         ...
>      }
> IOW, do we want to set done_flag if the iterator failed?
>
> Maybe it doesn't really matter.
>   


I do not think it matters -- the error is printed, and at that point the 
loop ends anyway. But from a correctness point of view, I believe you 
are correct here. Fixed as above


> Phil> +  return cons      ;
>
> Excess spaces.
>
> Phil> +  if (var->value)
> Phil> +      pretty_printer = gdbpy_get_varobj_pretty_printer (var->value);
>
> The second line looks it is indented too far.
>
> Phil> +  Py_DECREF(constructor);
>
> Missing space before the "("

Fixed.

I've updated the ChangeLogs but as they are essentially the same, I've 
omitted them from this in-review mail. Will generate them again when we 
have something final.

Thanks Phil

[-- Attachment #2: pretty_printer_reg_2.patch --]
[-- Type: text/x-patch, Size: 34333 bytes --]

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 4408635..4371672 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18531,6 +18531,11 @@ target's @code{char} type will be an 8-bit byte.  However, on some
 unusual platforms, this type may have a different size.
 @end defmethod
 
+@defmethod Type strip_typedefs
+Return a new @code{gdb.Type} that represents the real type,
+after removing all layers of typedefs.
+@end defmethod
+
 @defmethod Type tag
 Return the tag name for this type.  The tag name is the name after
 @code{struct}, @code{union}, or @code{enum} in C; not all languages
@@ -18791,30 +18796,27 @@ convertible to @code{gdb.Value}, an exception is raised.
 
 @subsubsection Selecting Pretty-Printers
 
-The Python dictionary @code{gdb.pretty_printers} maps regular
-expressions (strings) onto constructors.  Each @code{gdb.Objfile} also
-contains a @code{pretty_printers} attribute.  A constructor, in this
-context, is a function which takes a single @code{gdb.Value} argument
-and returns a a pretty-printer conforming to the interface definition
-above.
+The Python list @code{gdb.pretty_printers} contains an array of
+functions that have been registered via addition as a pretty-printer.
+Each function will be called with a @code{gdb.Value} to be
+pretty-printed.  Each @code{gdb.Objfile} also contains a
+@code{pretty_printers} attribute.  A function on one of these lists
+takes a single @code{gdb.Value} argument and returns a pretty-printer
+object conforming to the interface definition above.  If this function
+cannot create a pretty-printer for the value, it should return
+@code{None}.
 
-When printing a value, @value{GDBN} first computes the value's
-canonical type by following typedefs, following a reference type to
-its referenced type, and removing qualifiers, such as @code{const} or
-@code{volatile}.
-
-Then, @value{GDBN} tests each regular expression against this type name.
 @value{GDBN} first checks the @code{pretty_printers} attribute of each
-@code{gdb.Objfile}; after these have been exhausted it tries the
-global @code{gdb.pretty_printers}.
-
-If a regular expression matches, then the corresponding pretty-printer
-is invoked with a @code{gdb.Value} representing the value to be
-printed.
+@code{gdb.Objfile} and iteratively calls each function in the list for
+that @code{gdb.Objfile} until it receives a pretty-printer object.
+After these @code{gdb.Objfile} have been exhausted, it tries the
+global @code{gdb.pretty-printers} list, again calling each function
+until an object is returned.
 
 The order in which the objfiles are searched is not specified.
-Similarly, the order in which the regular expressions in a given
-dictionary are tried is not specified.
+Functions are always invoked from the head of the
+@code{gdb.pretty-printers} list, and iterated over sequentially until
+the end of the list, or a printer object is returned.
 
 Here is an example showing how a @code{std::string} printer might be
 written:
@@ -18823,13 +18825,34 @@ written:
 class StdStringPrinter:
     "Print a std::string"
 
-    def __init__(self, val):
+    def __init__ (self, val):
         self.val = val
 
-    def to_string(self):
+    def to_string (self):
         return self.val['_M_dataplus']['_M_p']
 @end smallexample
 
+And here is an example showing how a lookup function for
+the printer example above might be written. 
+
+@smallexample
+def str_lookup_function (val):
+
+    lookup_tag = val.type ().tag ()
+    regex = re.compile ("^std::basic_string<char,.*>$")
+    if lookup_tag == None:
+        return None
+    if regex.match (lookup_tag):
+        return StdStringPrinter (val)
+    
+    return None
+@end smallexample
+
+The example lookup function extracts the value's type, and attempts to
+match it to a type that it can pretty-print.  If it is a type the
+printer can pretty-print, it will return a printer object.  If not, it
+returns: @code{None}.
+
 We recommend that you put your core pretty-printers into a versioned
 python package, and then restrict your auto-loaded code to idempotent
 behavior -- for example, just @code{import}s of your printer modules,
@@ -18842,7 +18865,7 @@ For example, in addition to the above, this code might appear in
 
 @smallexample
 def register_printers (objfile):
-    objfile.pretty_printers['^std::basic_string<char.*>$'] = StdStringPrinter
+    objfile.pretty_printers.add (str_lookup_function)
 @end smallexample
 
 And then the corresponding contents of the auto-load file would be:
diff --git a/gdb/python/lib/gdb/libstdcxx/v6/printers.py b/gdb/python/lib/gdb/libstdcxx/v6/printers.py
index c9c8543..b8c6d6f 100644
--- a/gdb/python/lib/gdb/libstdcxx/v6/printers.py
+++ b/gdb/python/lib/gdb/libstdcxx/v6/printers.py
@@ -1,6 +1,6 @@
 # Pretty-printers for libstc++.
 
-# Copyright (C) 2008 Free Software Foundation, Inc.
+# Copyright (C) 2008, 2009 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
@@ -17,6 +17,7 @@
 
 import gdb
 import itertools
+import re
 
 class StdPointerPrinter:
     "Print a smart pointer of some kind"
@@ -546,56 +547,90 @@ class Tr1UnorderedMapPrinter:
     def display_hint (self):
         return 'map'
 
-def register_libstdcxx_printers(obj):
+def register_libstdcxx_printers (obj):
     "Register libstdc++ pretty-printers with objfile Obj."
 
     if obj == None:
         obj = gdb
 
+    obj.pretty_printers.append (lookup_function)
+
+def lookup_function (val):
+    "Look-up and return a pretty-printer that can print val."
+
+    # Get the type.
+    type = val.type ();
+
+    # If it points to a reference, get the reference.
+    if type.code () == gdb.TYPE_CODE_REF:
+        type = type.target ()
+
+    # Get the unqualified type, stripped of typedefs.
+    type = type.unqualified ().strip_typedefs ()
+
+    # Get the type name.    
+    typename = type.tag ()
+    if typename == None:
+        return None
+
+    # Iterate over local dictionary of types to determine
+    # if a printer is registered for that type.  Return an
+    # instantiation of the printer if found.
+    for function in pretty_printers_dict:
+        if function.search (typename):
+            return pretty_printers_dict[function] (val)
+        
+    # Cannot find a pretty printer.  Return None.
+    return None
+
+def build_libstdcxx_dictionary ():
     # libstdc++ objects requiring pretty-printing.
     # In order from:
     # http://gcc.gnu.org/onlinedocs/libstdc++/latest-doxygen/a01847.html
-    obj.pretty_printers['^std::basic_string<char,.*>$'] = lambda val: StdStringPrinter(None, val)
-    obj.pretty_printers['^std::basic_string<wchar_t,.*>$'] = lambda val: StdStringPrinter(target_wide_charset, val)
-    obj.pretty_printers['^std::basic_string<char16_t,.*>$'] = lambda val: StdStringPrinter('UTF-16', val)
-    obj.pretty_printers['^std::basic_string<char32_t,.*>$'] = lambda val: StdStringPrinter('UTF-32', val)
-    obj.pretty_printers['^std::bitset<.*>$'] = StdBitsetPrinter
-    obj.pretty_printers['^std::deque<.*>$'] = StdDequePrinter
-    obj.pretty_printers['^std::list<.*>$'] = StdListPrinter
-    obj.pretty_printers['^std::map<.*>$'] = lambda val: StdMapPrinter("std::map", val)
-    obj.pretty_printers['^std::multimap<.*>$'] = lambda val: StdMapPrinter("std::multimap", val)
-    obj.pretty_printers['^std::multiset<.*>$'] = lambda val: StdSetPrinter("std::multiset", val)
-    obj.pretty_printers['^std::priority_queue<.*>$'] = lambda val: StdStackOrQueuePrinter("std::priority_queue", val)
-    obj.pretty_printers['^std::queue<.*>$'] = lambda val: StdStackOrQueuePrinter("std::queue", val)
-    obj.pretty_printers['^std::set<.*>$'] = lambda val: StdSetPrinter("std::set", val)
-    obj.pretty_printers['^std::stack<.*>$'] = lambda val: StdStackOrQueuePrinter("std::stack", val)
-    obj.pretty_printers['^std::vector<.*>$'] = StdVectorPrinter
+    pretty_printers_dict[re.compile('^std::basic_string<char,.*>$')] = lambda val: StdStringPrinter(None, val)
+    pretty_printers_dict[re.compile('^std::basic_string<wchar_t,.*>$')] = lambda val: StdStringPrinter(target_wide_charset, val)
+    pretty_printers_dict[re.compile('^std::basic_string<char16_t,.*>$')] = lambda val: StdStringPrinter('UTF-16', val)
+    pretty_printers_dict[re.compile('^std::basic_string<char32_t,.*>$')] = lambda val: StdStringPrinter('UTF-32', val)
+    pretty_printers_dict[re.compile('^std::bitset<.*>$')] = StdBitsetPrinter
+    pretty_printers_dict[re.compile('^std::deque<.*>$')] = StdDequePrinter
+    pretty_printers_dict[re.compile('^std::list<.*>$')] = StdListPrinter
+    pretty_printers_dict[re.compile('^std::map<.*>$')] = lambda val: StdMapPrinter("std::map", val)
+    pretty_printers_dict[re.compile('^std::multimap<.*>$')] = lambda val: StdMapPrinter("std::multimap", val)
+    pretty_printers_dict[re.compile('^std::multiset<.*>$')] = lambda val: StdSetPrinter("std::multiset", val)
+    pretty_printers_dict[re.compile('^std::priority_queue<.*>$')] = lambda val: StdStackOrQueuePrinter("std::priority_queue", val)
+    pretty_printers_dict[re.compile('^std::queue<.*>$')] = lambda val: StdStackOrQueuePrinter("std::queue", val)
+    pretty_printers_dict[re.compile('^std::set<.*>$')] = lambda val: StdSetPrinter("std::set", val)
+    pretty_printers_dict[re.compile('^std::stack<.*>$')] = lambda val: StdStackOrQueuePrinter("std::stack", val)
+    pretty_printers_dict[re.compile('^std::vector<.*>$')] = StdVectorPrinter
     # vector<bool>
 
     # C++0x stuff.
     # array - the default seems reasonable
     # smart_ptr?  seems to only be in boost right now
-    obj.pretty_printers['^std::tr1::shared_ptr<.*>$'] = lambda val: StdPointerPrinter ('std::shared_ptr', val)
-    obj.pretty_printers['^std::tr1::weak_ptr<.*>$'] = lambda val: StdPointerPrinter ('std::weak_ptr', val)
-    obj.pretty_printers['^std::tr1::unique_ptr<.*>$'] = UniquePointerPrinter
-    obj.pretty_printers['^std::tr1::unordered_map<.*>$'] = lambda val: Tr1UnorderedMapPrinter ('std::tr1::unordered_map', val)
-    obj.pretty_printers['^std::tr1::unordered_set<.*>$'] = lambda val: Tr1UnorderedSetPrinter ('std::tr1::unordered_set', val)
-    obj.pretty_printers['^std::tr1::unordered_multimap<.*>$'] = lambda val: Tr1UnorderedMapPrinter ('std::tr1::unordered_multimap', val)
-    obj.pretty_printers['^std::tr1::unordered_multiset<.*>$'] = lambda val: Tr1UnorderedSetPrinter ('std::tr1::unordered_multiset', val)
+    pretty_printers_dict[re.compile('^std::tr1::shared_ptr<.*>$')] = lambda val: StdPointerPrinter ('std::shared_ptr', val)
+    pretty_printers_dict[re.compile('^std::tr1::weak_ptr<.*>$')] = lambda val: StdPointerPrinter ('std::weak_ptr', val)
+    pretty_printers_dict[re.compile('^std::tr1::unique_ptr<.*>$')] = UniquePointerPrinter
+    pretty_printers_dict[re.compile('^std::tr1::unordered_map<.*>$')] = lambda val: Tr1UnorderedMapPrinter ('std::tr1::unordered_map', val)
+    pretty_printers_dict[re.compile('^std::tr1::unordered_set<.*>$')] = lambda val: Tr1UnorderedSetPrinter ('std::tr1::unordered_set', val)
+    pretty_printers_dict[re.compile('^std::tr1::unordered_multimap<.*>$')] = lambda val: Tr1UnorderedMapPrinter ('std::tr1::unordered_multimap', val)
+    pretty_printers_dict[re.compile('^std::tr1::unordered_multiset<.*>$')] = lambda val: Tr1UnorderedSetPrinter ('std::tr1::unordered_multiset', val)
 
     # Extensions.
-    obj.pretty_printers['^__gnu_cxx::slist<.*>$'] = StdSlistPrinter
+    pretty_printers_dict[re.compile('^__gnu_cxx::slist<.*>$')] = StdSlistPrinter
 
     if True:
         # These shouldn't be necessary, if GDB "print *i" worked.
         # But it often doesn't, so here they are.
-        obj.pretty_printers['^std::_List_iterator<.*>$'] = lambda val: StdListIteratorPrinter(val)
-        obj.pretty_printers['^std::_List_const_iterator<.*>$'] = lambda val: StdListIteratorPrinter(val)
-        obj.pretty_printers['^std::_Rb_tree_iterator<.*>$'] = lambda val: StdRbtreeIteratorPrinter(val)
-        obj.pretty_printers['^std::_Rb_tree_const_iterator<.*>$'] = lambda val: StdRbtreeIteratorPrinter(val)
-        obj.pretty_printers['^std::_Deque_iterator<.*>$'] = lambda val: StdDequeIteratorPrinter(val)
-        obj.pretty_printers['^std::_Deque_const_iterator<.*>$'] = lambda val: StdDequeIteratorPrinter(val)
-        obj.pretty_printers['^__gnu_cxx::__normal_iterator<.*>$'] = lambda val: StdVectorIteratorPrinter(val)
-        obj.pretty_printers['^__gnu_cxx::_Slist_iterator<.*>$'] = lambda val: StdSlistIteratorPrinter(val)
-
+        pretty_printers_dict[re.compile('^std::_List_iterator<.*>$')] = lambda val: StdListIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^std::_List_const_iterator<.*>$')] = lambda val: StdListIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^std::_Rb_tree_iterator<.*>$')] = lambda val: StdRbtreeIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^std::_Rb_tree_const_iterator<.*>$')] = lambda val: StdRbtreeIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^std::_Deque_iterator<.*>$')] = lambda val: StdDequeIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^std::_Deque_const_iterator<.*>$')] = lambda val: StdDequeIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^__gnu_cxx::__normal_iterator<.*>$')] = lambda val: StdVectorIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^__gnu_cxx::_Slist_iterator<.*>$')] = lambda val: StdSlistIteratorPrinter(val)
+
+pretty_printers_dict = {}
+
+build_libstdcxx_dictionary ()
 register_libstdcxx_printers (gdb.current_objfile())
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index afdf821..e2fc9f1 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -161,8 +161,8 @@ int gdbpy_is_string (PyObject *obj);
    other pretty-printer functions, because they refer to PyObject.  */
 char *apply_varobj_pretty_printer (PyObject *print_obj, struct value *value,
 				   struct value **replacement);
-PyObject *gdbpy_get_varobj_pretty_printer (struct type *type);
-PyObject *gdbpy_instantiate_printer (PyObject *cons, struct value *value);
+PyObject *gdbpy_get_varobj_pretty_printer (struct value *value);
+PyObject *gdbpy_instantiate_printer (PyObject *cons, PyObject *value);
 char *gdbpy_get_display_hint (PyObject *printer);
 
 extern PyObject *gdbpy_children_cst;
diff --git a/gdb/python/python-objfile.c b/gdb/python/python-objfile.c
index a225409..e97d3a2 100644
--- a/gdb/python/python-objfile.c
+++ b/gdb/python/python-objfile.c
@@ -1,6 +1,6 @@
 /* Python interface to objfiles.
 
-   Copyright (C) 2008 Free Software Foundation, Inc.
+   Copyright (C) 2008, 2009 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -29,7 +29,7 @@ typedef struct
   /* The corresponding objfile.  */
   struct objfile *objfile;
 
-  /* The pretty-printer dictionary.  */
+  /* The pretty-printer list of functions.  */
   PyObject *printers;
 } objfile_object;
 
@@ -66,7 +66,7 @@ objfpy_new (PyTypeObject *type, PyObject *args, PyObject *keywords)
     {
       self->objfile = NULL;
 
-      self->printers = PyDict_New ();
+      self->printers = PyList_New (0);
       if (!self->printers)
 	{
 	  Py_DECREF (self);
@@ -95,10 +95,10 @@ objfpy_set_printers (PyObject *o, PyObject *value, void *ignore)
       return -1;
     }
 
-  if (! PyDict_Check (value))
+  if (! PyList_Check (value))
     {
       PyErr_SetString (PyExc_TypeError,
-		       "the pretty_printers attribute must be a dictionary");
+		       "the pretty_printers attribute must be a list");
       return -1;
     }
 
@@ -139,7 +139,7 @@ objfile_to_objfile_object (struct objfile *objfile)
 
 	  object->objfile = objfile;
 
-	  object->printers = PyDict_New ();
+	  object->printers = PyList_New (0);
 	  if (!object->printers)
 	    {
 	      Py_DECREF (object);
diff --git a/gdb/python/python-type.c b/gdb/python/python-type.c
index 5aa82f6..d336456 100644
--- a/gdb/python/python-type.c
+++ b/gdb/python/python-type.c
@@ -1,6 +1,6 @@
 /* Python interface to types.
 
-   Copyright (C) 2008 Free Software Foundation, Inc.
+   Copyright (C) 2008, 2009 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -233,6 +233,15 @@ typy_tag (PyObject *self, PyObject *args)
   return PyString_FromString (TYPE_TAG_NAME (type));
 }
 
+/* Return the type, stripped of typedefs. */
+static PyObject *
+typy_strip_typedefs (PyObject *self, PyObject *args)
+{
+  struct type *type = ((type_object *) self)->type;
+
+  return type_to_type_object (check_typedef (type));
+}
+
 /* Return a Type object which represents a pointer to SELF.  */
 static PyObject *
 typy_pointer (PyObject *self, PyObject *args)
@@ -710,6 +719,8 @@ Each field is a dictionary." },
     "Return the size of this type, in bytes" },
   { "tag", typy_tag, METH_NOARGS,
     "Return the tag name for this type, or None." },
+  { "strip_typedefs", typy_strip_typedefs, METH_NOARGS,
+    "Return a type stripped of typedefs"},
   { "target", typy_target, METH_NOARGS,
     "Return the target type of this type" },
   { "template_argument", typy_template_argument, METH_VARARGS,
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 8677da6..0321a70 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -842,52 +842,45 @@ get_type (struct type *type)
 }
 
 /* Helper function for find_pretty_printer which iterates over a
-   dictionary and tries to find a match.  */
+   list, calls each function and inspects output.  */
 static PyObject *
-search_pp_dictionary (PyObject *dict, char *type_name)
+search_pp_list (PyObject *list, PyObject *value)
 {
-  Py_ssize_t iter;
-  PyObject *key, *func, *found = NULL;
-
-  /* See if the type matches a pretty-printer regexp.  */
-  iter = 0;
-  while (! found && PyDict_Next (dict, &iter, &key, &func))
+  Py_ssize_t pp_list_size, list_index;
+  PyObject *function, *printer = NULL;
+  
+  pp_list_size = PyList_Size (list);
+  for (list_index = 0; list_index < pp_list_size; list_index++)
     {
-      char *rx_str;
-
-      if (! PyString_Check (key))
-	continue;
-      rx_str = PyString_AsString (key);
-      if (re_comp (rx_str) == NULL && re_exec (type_name) == 1)
-	found = func;
+      function = PyList_GetItem (list, list_index);
+      if (function == NULL)
+	Py_RETURN_NONE;
+      printer = gdbpy_instantiate_printer (function, value);
+      if (printer && (printer != Py_None))
+	return printer;      
     }
 
-  return found;
+  Py_RETURN_NONE;
 }
 
 /* Find the pretty-printing constructor function for TYPE.  If no
    pretty-printer exists, return NULL.  If one exists, return a new
    reference.  */
 static PyObject *
-find_pretty_printer (struct type *type)
+find_pretty_printer (struct value *value)
 {
-  PyObject *dict, *found = NULL;
-  char *type_name = NULL;
+  PyObject *pp_list = NULL;
+  PyObject *function = NULL;
+  PyObject *val_obj = NULL;
   struct objfile *obj;
   volatile struct gdb_exception except;
 
-  /* Get the name of the type.  */
   TRY_CATCH (except, RETURN_MASK_ALL)
     {
-      /* If we have a reference, use the referenced type.  */
-      if (TYPE_CODE (type) == TYPE_CODE_REF)
-	type = TYPE_TARGET_TYPE (type);
-      /* Strip off any qualifiers from the type.  */
-      type = make_cv_type (0, 0, type, NULL);
-      type_name = get_type (type);
+      value = value_copy (value);
+      val_obj = value_to_value_object (value);
     }
-  if (except.reason < 0)
-    return NULL;
+  GDB_PY_HANDLE_EXCEPTION (except);
 
   /* Look at the pretty-printer dictionary for each objfile.  */
   ALL_OBJFILES (obj)
@@ -896,34 +889,36 @@ find_pretty_printer (struct type *type)
     if (!objf)
       continue;
 
-    dict = objfpy_get_printers (objf, NULL);
-    found = search_pp_dictionary (dict, type_name);
-    if (found)
+    pp_list = objfpy_get_printers (objf, NULL);
+    function = search_pp_list (pp_list, val_obj);
+    if (function != Py_None)
       goto done;
 
-    Py_DECREF (dict);
+    Py_DECREF (pp_list);
   }
 
   /* Fetch the global pretty printer dictionary.  */
-  dict = NULL;
+  pp_list = NULL;
   if (! PyObject_HasAttrString (gdb_module, "pretty_printers"))
     goto done;
-  dict = PyObject_GetAttrString (gdb_module, "pretty_printers");
-  if (! dict)
+  pp_list = PyObject_GetAttrString (gdb_module, "pretty_printers");
+  if (! pp_list)
     goto done;
-  if (! PyDict_Check (dict) || ! PyDict_Size (dict))
+  if (! PyList_Check (pp_list))
     goto done;
 
-  found = search_pp_dictionary (dict, type_name);
+  function = search_pp_list (pp_list, val_obj);
 
  done:
-  xfree (type_name);
-
-  if (found)
-    Py_INCREF (found);
-  Py_XDECREF (dict);
-
-  return found;
+  if (! function) 
+    {
+      function = Py_None;
+      Py_INCREF (function);
+    }
+  Py_XDECREF (pp_list);
+  Py_DECREF (val_obj);
+  
+  return function;
 }
 
 /* Pretty-print a single value, via the printer object PRINTER.  If
@@ -966,22 +961,11 @@ pretty_print_one_value (PyObject *printer, struct value **out_value)
 /* Instantiate a pretty-printer given a constructor, CONS, and a
    value, VAL.  Return NULL on error.  */
 PyObject *
-gdbpy_instantiate_printer (PyObject *cons, struct value *value)
+gdbpy_instantiate_printer (PyObject *cons, PyObject *value)
 {
-  PyObject *val_obj = NULL, *result;
-  volatile struct gdb_exception except;
-
-  TRY_CATCH (except, RETURN_MASK_ALL)
-    {
-      /* FIXME: memory management here.  Why are values so
-	 funny?  */
-      value = value_copy (value);
-      val_obj = value_to_value_object (value);
-    }
-  GDB_PY_HANDLE_EXCEPTION (except);
+  PyObject *result;
 
-  result = PyObject_CallFunctionObjArgs (cons, val_obj, NULL);
-  Py_DECREF (val_obj);
+  result = PyObject_CallFunctionObjArgs (cons, value, NULL);
   return result;
 }
 
@@ -1174,9 +1158,12 @@ print_children (PyObject *printer, const char *hint,
 
       if (! item)
 	{
+	  if (PyErr_Occurred ())
+	    gdbpy_print_stack ();
 	  /* Set a flag so we can know whether we printed all the
 	     available elements.  */
-	  done_flag = 1;
+	  else	  
+	    done_flag = 1;
 	  break;
 	}
 
@@ -1296,7 +1283,7 @@ apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
 			  const struct value_print_options *options,
 			  const struct language_defn *language)
 {
-  PyObject *func, *printer;
+  PyObject *printer = NULL;
   struct value *value;
   char *hint = NULL;
   struct cleanup *cleanups;
@@ -1306,36 +1293,26 @@ apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
   state = PyGILState_Ensure ();
   cleanups = make_cleanup_py_restore_gil (&state);
 
-  /* Find the constructor.  */
-  func = find_pretty_printer (type);
-  if (! func)
-    goto done;
-
   /* Instantiate the printer.  */
   if (valaddr)
     valaddr += embedded_offset;
   value = value_from_contents_and_address (type, valaddr, address);
-  printer = gdbpy_instantiate_printer (func, value);
-  Py_DECREF (func);
 
-  if (!printer)
-    {
-      gdbpy_print_stack ();
-      goto done;
-    }
+  /* Find the constructor.  */
+  printer = find_pretty_printer (value);
+  make_cleanup_py_decref (printer);
+  if (printer == Py_None)
+    goto done;
 
   /* If we are printing a map, we want some special formatting.  */
   hint = gdbpy_get_display_hint (printer);
   make_cleanup (free_current_contents, &hint);
 
-  make_cleanup_py_decref (printer);
-  if (printer != Py_None)
-    {
-      print_string_repr (printer, hint, stream, recurse, options, language);
-      print_children (printer, hint, stream, recurse, options, language);
+  /* Print the section */
+  print_string_repr (printer, hint, stream, recurse, options, language);
+  print_children (printer, hint, stream, recurse, options, language);
+  result = 1;
 
-      result = 1;
-    }
 
  done:
   do_cleanups (cleanups);
@@ -1371,9 +1348,9 @@ apply_varobj_pretty_printer (PyObject *printer_obj, struct value *value,
    is the type of the varobj for which a printer should be
    returned.  */
 PyObject *
-gdbpy_get_varobj_pretty_printer (struct type *type)
+gdbpy_get_varobj_pretty_printer (struct value *value)
 {
-  return find_pretty_printer (type);
+  return find_pretty_printer (value);
 }
 
 /* A Python function which wraps find_pretty_printer and instantiates
@@ -1396,24 +1373,8 @@ gdbpy_default_visualizer (PyObject *self, PyObject *args)
       return NULL;
     }
 
-  cons = find_pretty_printer (value_type (value));
-  if (cons)
-    {
-      /* While it is a bit lame to pass value here and make a new
-	 Value, it is probably better to share the instantiation
-	 code.  */
-      printer = gdbpy_instantiate_printer (cons, value);
-      Py_DECREF (cons);
-    }
-
-  if (!printer)
-    {
-      PyErr_Clear ();
-      printer = Py_None;
-      Py_INCREF (printer);
-    }
-
-  return printer;
+  cons = find_pretty_printer (value);
+  return cons;
 }
 
 #else /* HAVE_PYTHON */
@@ -1566,7 +1527,7 @@ Enables or disables auto-loading of Python code when an object is opened."),
   gdbpy_initialize_membuf ();
 
   PyRun_SimpleString ("import gdb");
-  PyRun_SimpleString ("gdb.pretty_printers = {}");
+  PyRun_SimpleString ("gdb.pretty_printers = []");
 
   observer_attach_new_objfile (gdbpy_new_objfile);
 
diff --git a/gdb/testsuite/gdb.python/python-prettyprint.py b/gdb/testsuite/gdb.python/python-prettyprint.py
index ecd4fc6..0d9cb87 100644
--- a/gdb/testsuite/gdb.python/python-prettyprint.py
+++ b/gdb/testsuite/gdb.python/python-prettyprint.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2008 Free Software Foundation, Inc.
+# Copyright (C) 2008, 2009 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
@@ -16,6 +16,8 @@
 # This file is part of the GDB testsuite.  It tests python pretty
 # printers.
 
+import re
+
 # Test returning a Value from a printer.
 class string_print:
     def __init__(self, val):
@@ -76,21 +78,57 @@ class pp_sss:
     def to_string(self):
         return "a=<" + str(self.val['a']) + "> b=<" + str(self.val["b"]) + ">"
 
-gdb.pretty_printers['^struct s$']   = pp_s
-gdb.pretty_printers['^s$']   = pp_s
-gdb.pretty_printers['^S$']   = pp_s
-
-gdb.pretty_printers['^struct ss$']  = pp_ss
-gdb.pretty_printers['^ss$']  = pp_ss
-
-gdb.pretty_printers['^const S &$']   = pp_s
-gdb.pretty_printers['^SSS$']  = pp_sss
-
-# Note that we purposely omit the typedef names here.
-# Printer lookup is based on canonical name.
-# However, we do need both tagged and untagged variants, to handle
-# both the C and C++ cases.
-gdb.pretty_printers['^struct string_repr$'] = string_print
-gdb.pretty_printers['^struct container$'] = ContainerPrinter
-gdb.pretty_printers['^string_repr$'] = string_print
-gdb.pretty_printers['^container$'] = ContainerPrinter
+def lookup_function (val):
+    "Look-up and return a pretty-printer that can print val."
+
+    # Get the type.
+    type = val.type ();
+
+    # If it points to a reference, get the reference.
+    if type.code () == gdb.TYPE_CODE_REF:
+        type = type.target ()
+
+    # Get the unqualified type, stripped of typedefs.
+    type = type.unqualified ().strip_typedefs ()
+
+    # Get the type name.    
+    typename = type.tag ()
+
+    if typename == None:
+        return None
+
+    # Iterate over local dictionary of types to determine
+    # if a printer is registered for that type.  Return an
+    # instantiation of the printer if found.
+    for function in pretty_printers_dict:
+        if function.match (typename):
+            return pretty_printers_dict[function] (val)
+        
+    # Cannot find a pretty printer.  Return None.
+
+    return None
+
+
+def register_pretty_printers ():
+    pretty_printers_dict[re.compile ('^struct s$')]   = pp_s
+    pretty_printers_dict[re.compile ('^s$')]   = pp_s
+    pretty_printers_dict[re.compile ('^S$')]   = pp_s
+
+    pretty_printers_dict[re.compile ('^struct ss$')]  = pp_ss
+    pretty_printers_dict[re.compile ('^ss$')]  = pp_ss
+    pretty_printers_dict[re.compile ('^const S &$')]   = pp_s
+    pretty_printers_dict[re.compile ('^SSS$')]  = pp_sss
+    
+    # Note that we purposely omit the typedef names here.
+    # Printer lookup is based on canonical name.
+    # However, we do need both tagged and untagged variants, to handle
+    # both the C and C++ cases.
+    pretty_printers_dict[re.compile ('^struct string_repr$')] = string_print
+    pretty_printers_dict[re.compile ('^struct container$')] = ContainerPrinter
+    pretty_printers_dict[re.compile ('^string_repr$')] = string_print
+    pretty_printers_dict[re.compile ('^container$')] = ContainerPrinter
+    
+pretty_printers_dict = {}
+
+register_pretty_printers ()
+gdb.pretty_printers.append (lookup_function)
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 7f77902..4d1a9d6 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -181,10 +181,6 @@ struct varobj
   int from;
   int to;
 
-  /* If NULL, no pretty printer is installed.  If not NULL, the
-     constructor to call to get a pretty-printer object.  */
-  PyObject *constructor;
-
   /* The pretty-printer that has been constructed.  If NULL, then a
      new printer object is needed, and one will be constructed.  */
   PyObject *pretty_printer;
@@ -713,19 +709,22 @@ varobj_delete (struct varobj *var, char ***dellist, int only_children)
 
 /* Instantiate a pretty-printer for a given value.  */
 static PyObject *
-instantiate_pretty_printer (struct varobj *var, struct value *value)
+instantiate_pretty_printer (PyObject *constructor, struct value *value)
 {
 #if HAVE_PYTHON
-  if (var->constructor)
+  PyObject *val_obj = NULL; 
+  PyObject *printer;
+  volatile struct gdb_exception except;
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
     {
-      PyObject *printer = gdbpy_instantiate_printer (var->constructor, value);
-      if (printer == Py_None)
-	{
-	  Py_DECREF (printer);
-	  printer = NULL;
-	}
-      return printer;
+      value = value_copy (value);
+      val_obj = value_to_value_object (value);
     }
+  GDB_PY_HANDLE_EXCEPTION (except);
+  printer = gdbpy_instantiate_printer (constructor, val_obj);
+  Py_DECREF (val_obj);
+  return printer;
 #endif
   return NULL;
 }
@@ -1224,7 +1223,7 @@ install_new_value (struct varobj *var, struct value *value, int initial)
   /* If the type has custom visualizer, we consider it to be always
      changeable. FIXME: need to make sure this behaviour will not
      mess up read-sensitive values.  */
-  if (var->constructor)
+  if (var->pretty_printer)
     changeable = 1;
 
   need_to_fetch = changeable;
@@ -1278,20 +1277,6 @@ install_new_value (struct varobj *var, struct value *value, int initial)
 	}
     }
 
-#if HAVE_PYTHON
-  {
-    PyGILState_STATE state = PyGILState_Ensure ();
-    if (var->pretty_printer)
-      {
-	Py_DECREF (var->pretty_printer);
-      }
-    if (value)
-      var->pretty_printer = instantiate_pretty_printer (var, value);
-    else
-      var->pretty_printer = NULL;
-    PyGILState_Release (state);
-  }
-#endif
 
   /* Below, we'll be comparing string rendering of old and new
      values.  Don't get string rendering if the value is
@@ -1408,11 +1393,8 @@ install_visualizer (struct varobj *var, PyObject *visualizer)
   varobj_delete (var, NULL, 1 /* children only */);
   var->num_children = -1;
 
-  Py_XDECREF (var->constructor);
-  var->constructor = visualizer;
-
   Py_XDECREF (var->pretty_printer);
-  var->pretty_printer = NULL;
+  var->pretty_printer = visualizer;
 
   install_new_value (var, var->value, 1);
 
@@ -1433,14 +1415,21 @@ install_default_visualizer (struct varobj *var)
 #if HAVE_PYTHON
   struct cleanup *cleanup;
   PyGILState_STATE state;
-  PyObject *constructor = NULL;
+  PyObject *pretty_printer = NULL;
 
   state = PyGILState_Ensure ();
   cleanup = make_cleanup_py_restore_gil (&state);
 
-  if (var->type)
-    constructor = gdbpy_get_varobj_pretty_printer (var->type);
-  install_visualizer (var, constructor);
+  if (var->value)
+    pretty_printer = gdbpy_get_varobj_pretty_printer (var->value);
+
+  if (pretty_printer == Py_None)
+    {
+      Py_DECREF (pretty_printer);
+      pretty_printer = NULL;
+    }
+  
+  install_visualizer (var, pretty_printer);
 
   do_cleanups (cleanup);
 #else
@@ -1452,7 +1441,7 @@ void
 varobj_set_visualizer (struct varobj *var, const char *visualizer)
 {
 #if HAVE_PYTHON
-  PyObject *mainmod, *globals, *constructor;
+  PyObject *mainmod, *globals, *pretty_printer, *constructor;
   struct cleanup *back_to;
   PyGILState_STATE state;
 
@@ -1465,21 +1454,28 @@ varobj_set_visualizer (struct varobj *var, const char *visualizer)
   make_cleanup_py_decref (globals);
 
   constructor = PyRun_String (visualizer, Py_eval_input, globals, globals);
+  
+  // Do not instantiate NoneType.
+  if (constructor == Py_None)
+    pretty_printer = Py_None;
+  else
+    pretty_printer = instantiate_pretty_printer (constructor, var->value);
 
-  if (! constructor)
+  if (! pretty_printer)
     {
       gdbpy_print_stack ();
       error ("Could not evaluate visualizer expression: %s", visualizer);
     }
 
-  if (constructor == Py_None)
+  if (pretty_printer == Py_None)
     {
-      Py_DECREF (constructor);
-      constructor = NULL;
+      Py_DECREF (pretty_printer);
+      pretty_printer = NULL;
     }
 
-  install_visualizer (var, constructor);
+  install_visualizer (var, pretty_printer);
 
+  Py_DECREF (constructor);
   do_cleanups (back_to);
 #else
   error ("Python support required");
@@ -1917,7 +1913,6 @@ new_variable (void)
   var->children_requested = 0;
   var->from = -1;
   var->to = -1;
-  var->constructor = 0;
   var->pretty_printer = 0;
 
   return var;
@@ -1954,7 +1949,6 @@ free_variable (struct varobj *var)
 #if HAVE_PYTHON
   {
     PyGILState_STATE state = PyGILState_Ensure ();
-    Py_XDECREF (var->constructor);
     Py_XDECREF (var->pretty_printer);
     PyGILState_Release (state);
   }
@@ -2753,7 +2747,7 @@ c_value_of_variable (struct varobj *var, enum varobj_display_formats format)
 
   /* If we have a custom formatter, return whatever string it has
      produced.  */
-  if (var->constructor && var->print_value)
+  if (var->pretty_printer && var->print_value)
     return xstrdup (var->print_value);
   
   /* Strip top-level references. */

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

* Re: [python] [rfc] Patch for pretty-printers registration API change
  2009-02-18 14:40   ` Phil Muldoon
@ 2009-02-18 19:15     ` Tom Tromey
  2009-02-20 15:50       ` Phil Muldoon
  0 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2009-02-18 19:15 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: Project Archer

Thanks Phil.

Phil> +      function = PyList_GetItem (list, list_index);
Phil> +      if (function == NULL)
Phil> +	Py_RETURN_NONE;

I think it is not valid to ignore Python errors like this.  Instead, I
think you either have to propagate the exception (return NULL) or
clear it.  I suspect propagation is the better answer.

Phil> +      printer = gdbpy_instantiate_printer (function, value);
Phil> +      if (printer && (printer != Py_None))

Still too many parens :)

Also, the same comment holds about error propagation.

Phil> +      val_obj = value_to_value_object (value);

This can fail, returning NULL.  I think this particular line should
probably be moved out of the TRY_CATCH and after the
GDB_PY_HANDLE_EXCEPTION.  Then, it needs an error check.

(Though see below, I think some of this is probably redundant.)

Phil> +  Py_DECREF (val_obj);

I'm guessing you'll want Py_XDECREF here.

Phil>    value = value_from_contents_and_address (type, valaddr, address);
[...]
Phil> +  printer = find_pretty_printer (value);

In this case we know we don't need to copy the value -- we made a new
one and we can just use it directly.  So, I think the value_copy logic
should probably be removed from find_pretty_printer and pushed into
the callers that need it.  What do you think about that?

Phil> +  // Do not instantiate NoneType.
Phil> +  if (constructor == Py_None)
Phil> +    pretty_printer = Py_None;
Phil> +  else
Phil> +    pretty_printer = instantiate_pretty_printer (constructor, var->value);
 
Phil> -  if (! constructor)
Phil> +  if (! pretty_printer)
Phil>      {
Phil>        gdbpy_print_stack ();
Phil>        error ("Could not evaluate visualizer expression: %s", visualizer);
Phil>      }
 
Phil> -  if (constructor == Py_None)
Phil> +  if (pretty_printer == Py_None)
Phil>      {
Phil> -      Py_DECREF (constructor);
Phil> -      constructor = NULL;
Phil> +      Py_DECREF (pretty_printer);
Phil> +      pretty_printer = NULL;
Phil>      }

The refcount logic for "None" here is not correct.  I think the
simplest fix is to add an incref to the true branch of the first if.

Tom

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

* Re: [python] [rfc] Patch for pretty-printers registration API change
  2009-02-18 19:15     ` Tom Tromey
@ 2009-02-20 15:50       ` Phil Muldoon
  2009-02-20 19:38         ` Tom Tromey
  0 siblings, 1 reply; 8+ messages in thread
From: Phil Muldoon @ 2009-02-20 15:50 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Project Archer

[-- Attachment #1: Type: text/plain, Size: 2159 bytes --]

Tom Tromey wrote:

Hi Tom,

> Phil> +      function = PyList_GetItem (list, list_index);
> Phil> +      if (function == NULL)
> Phil> +	Py_RETURN_NONE;
>
> I think it is not valid to ignore Python errors like this.  Instead, I
> think you either have to propagate the exception (return NULL) or
> clear it.  I suspect propagation is the better answer.
>   


I elected propagation. I was not terribly sure in the CLI case if I 
should start applying the gdb error () and gdbpy_print_stack () inside 
apply_val_pretty_printer () .. or allow the error to propagate further. 
I elected to let them continue as that was the case beforehand. We can 
easily change this. What do you think?

> Phil> +      printer = gdbpy_instantiate_printer (function, value);
> Phil> +      if (printer && (printer != Py_None))
>
> Still too many parens :)
>   


Fixed ;)


> Also, the same comment holds about error propagation.
>
> Phil> +      val_obj = value_to_value_object (value);
>
> This can fail, returning NULL.  I think this particular line should
> probably be moved out of the TRY_CATCH and after the
> GDB_PY_HANDLE_EXCEPTION.  Then, it needs an error check.
>   


Fixed these NULL cases ... and ...

> So, I think the value_copy logic
> should probably be removed from find_pretty_printer and pushed into
> the callers that need it.  What do you think about that?
>   


I think we should only copy it when needed per you suggestion. It adds a 
bit more code, but cuts down on memory-clutter. I did this in the cases 
I thought it was needed (mainly when I get passed a varobj). In any 
case, there are only 3 in total. Please check and let me know if you 
disagree in the copy cases and we'll fix it up.


> Phil> +  // Do not instantiate NoneType.
> Phil> +  if (constructor == Py_None)
>
>   

...

> The refcount logic for "None" here is not correct.  I think the
> simplest fix is to add an incref to the true branch of the first if


Fixed, and as a bonus I fixed a case in search_pp_list and 
find_pretty_printer where they was not decrementing Py_None while 
searching for printers (Py_RETURN_NONE as part of the macro incref's 
Py_None).

Regards

Phil

[-- Attachment #2: pretty_printer_lookup.patch --]
[-- Type: text/x-patch, Size: 36194 bytes --]

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 4408635..4371672 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18531,6 +18531,11 @@ target's @code{char} type will be an 8-bit byte.  However, on some
 unusual platforms, this type may have a different size.
 @end defmethod
 
+@defmethod Type strip_typedefs
+Return a new @code{gdb.Type} that represents the real type,
+after removing all layers of typedefs.
+@end defmethod
+
 @defmethod Type tag
 Return the tag name for this type.  The tag name is the name after
 @code{struct}, @code{union}, or @code{enum} in C; not all languages
@@ -18791,30 +18796,27 @@ convertible to @code{gdb.Value}, an exception is raised.
 
 @subsubsection Selecting Pretty-Printers
 
-The Python dictionary @code{gdb.pretty_printers} maps regular
-expressions (strings) onto constructors.  Each @code{gdb.Objfile} also
-contains a @code{pretty_printers} attribute.  A constructor, in this
-context, is a function which takes a single @code{gdb.Value} argument
-and returns a a pretty-printer conforming to the interface definition
-above.
+The Python list @code{gdb.pretty_printers} contains an array of
+functions that have been registered via addition as a pretty-printer.
+Each function will be called with a @code{gdb.Value} to be
+pretty-printed.  Each @code{gdb.Objfile} also contains a
+@code{pretty_printers} attribute.  A function on one of these lists
+takes a single @code{gdb.Value} argument and returns a pretty-printer
+object conforming to the interface definition above.  If this function
+cannot create a pretty-printer for the value, it should return
+@code{None}.
 
-When printing a value, @value{GDBN} first computes the value's
-canonical type by following typedefs, following a reference type to
-its referenced type, and removing qualifiers, such as @code{const} or
-@code{volatile}.
-
-Then, @value{GDBN} tests each regular expression against this type name.
 @value{GDBN} first checks the @code{pretty_printers} attribute of each
-@code{gdb.Objfile}; after these have been exhausted it tries the
-global @code{gdb.pretty_printers}.
-
-If a regular expression matches, then the corresponding pretty-printer
-is invoked with a @code{gdb.Value} representing the value to be
-printed.
+@code{gdb.Objfile} and iteratively calls each function in the list for
+that @code{gdb.Objfile} until it receives a pretty-printer object.
+After these @code{gdb.Objfile} have been exhausted, it tries the
+global @code{gdb.pretty-printers} list, again calling each function
+until an object is returned.
 
 The order in which the objfiles are searched is not specified.
-Similarly, the order in which the regular expressions in a given
-dictionary are tried is not specified.
+Functions are always invoked from the head of the
+@code{gdb.pretty-printers} list, and iterated over sequentially until
+the end of the list, or a printer object is returned.
 
 Here is an example showing how a @code{std::string} printer might be
 written:
@@ -18823,13 +18825,34 @@ written:
 class StdStringPrinter:
     "Print a std::string"
 
-    def __init__(self, val):
+    def __init__ (self, val):
         self.val = val
 
-    def to_string(self):
+    def to_string (self):
         return self.val['_M_dataplus']['_M_p']
 @end smallexample
 
+And here is an example showing how a lookup function for
+the printer example above might be written. 
+
+@smallexample
+def str_lookup_function (val):
+
+    lookup_tag = val.type ().tag ()
+    regex = re.compile ("^std::basic_string<char,.*>$")
+    if lookup_tag == None:
+        return None
+    if regex.match (lookup_tag):
+        return StdStringPrinter (val)
+    
+    return None
+@end smallexample
+
+The example lookup function extracts the value's type, and attempts to
+match it to a type that it can pretty-print.  If it is a type the
+printer can pretty-print, it will return a printer object.  If not, it
+returns: @code{None}.
+
 We recommend that you put your core pretty-printers into a versioned
 python package, and then restrict your auto-loaded code to idempotent
 behavior -- for example, just @code{import}s of your printer modules,
@@ -18842,7 +18865,7 @@ For example, in addition to the above, this code might appear in
 
 @smallexample
 def register_printers (objfile):
-    objfile.pretty_printers['^std::basic_string<char.*>$'] = StdStringPrinter
+    objfile.pretty_printers.add (str_lookup_function)
 @end smallexample
 
 And then the corresponding contents of the auto-load file would be:
diff --git a/gdb/python/lib/gdb/libstdcxx/v6/printers.py b/gdb/python/lib/gdb/libstdcxx/v6/printers.py
index c9c8543..b8c6d6f 100644
--- a/gdb/python/lib/gdb/libstdcxx/v6/printers.py
+++ b/gdb/python/lib/gdb/libstdcxx/v6/printers.py
@@ -1,6 +1,6 @@
 # Pretty-printers for libstc++.
 
-# Copyright (C) 2008 Free Software Foundation, Inc.
+# Copyright (C) 2008, 2009 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
@@ -17,6 +17,7 @@
 
 import gdb
 import itertools
+import re
 
 class StdPointerPrinter:
     "Print a smart pointer of some kind"
@@ -546,56 +547,90 @@ class Tr1UnorderedMapPrinter:
     def display_hint (self):
         return 'map'
 
-def register_libstdcxx_printers(obj):
+def register_libstdcxx_printers (obj):
     "Register libstdc++ pretty-printers with objfile Obj."
 
     if obj == None:
         obj = gdb
 
+    obj.pretty_printers.append (lookup_function)
+
+def lookup_function (val):
+    "Look-up and return a pretty-printer that can print val."
+
+    # Get the type.
+    type = val.type ();
+
+    # If it points to a reference, get the reference.
+    if type.code () == gdb.TYPE_CODE_REF:
+        type = type.target ()
+
+    # Get the unqualified type, stripped of typedefs.
+    type = type.unqualified ().strip_typedefs ()
+
+    # Get the type name.    
+    typename = type.tag ()
+    if typename == None:
+        return None
+
+    # Iterate over local dictionary of types to determine
+    # if a printer is registered for that type.  Return an
+    # instantiation of the printer if found.
+    for function in pretty_printers_dict:
+        if function.search (typename):
+            return pretty_printers_dict[function] (val)
+        
+    # Cannot find a pretty printer.  Return None.
+    return None
+
+def build_libstdcxx_dictionary ():
     # libstdc++ objects requiring pretty-printing.
     # In order from:
     # http://gcc.gnu.org/onlinedocs/libstdc++/latest-doxygen/a01847.html
-    obj.pretty_printers['^std::basic_string<char,.*>$'] = lambda val: StdStringPrinter(None, val)
-    obj.pretty_printers['^std::basic_string<wchar_t,.*>$'] = lambda val: StdStringPrinter(target_wide_charset, val)
-    obj.pretty_printers['^std::basic_string<char16_t,.*>$'] = lambda val: StdStringPrinter('UTF-16', val)
-    obj.pretty_printers['^std::basic_string<char32_t,.*>$'] = lambda val: StdStringPrinter('UTF-32', val)
-    obj.pretty_printers['^std::bitset<.*>$'] = StdBitsetPrinter
-    obj.pretty_printers['^std::deque<.*>$'] = StdDequePrinter
-    obj.pretty_printers['^std::list<.*>$'] = StdListPrinter
-    obj.pretty_printers['^std::map<.*>$'] = lambda val: StdMapPrinter("std::map", val)
-    obj.pretty_printers['^std::multimap<.*>$'] = lambda val: StdMapPrinter("std::multimap", val)
-    obj.pretty_printers['^std::multiset<.*>$'] = lambda val: StdSetPrinter("std::multiset", val)
-    obj.pretty_printers['^std::priority_queue<.*>$'] = lambda val: StdStackOrQueuePrinter("std::priority_queue", val)
-    obj.pretty_printers['^std::queue<.*>$'] = lambda val: StdStackOrQueuePrinter("std::queue", val)
-    obj.pretty_printers['^std::set<.*>$'] = lambda val: StdSetPrinter("std::set", val)
-    obj.pretty_printers['^std::stack<.*>$'] = lambda val: StdStackOrQueuePrinter("std::stack", val)
-    obj.pretty_printers['^std::vector<.*>$'] = StdVectorPrinter
+    pretty_printers_dict[re.compile('^std::basic_string<char,.*>$')] = lambda val: StdStringPrinter(None, val)
+    pretty_printers_dict[re.compile('^std::basic_string<wchar_t,.*>$')] = lambda val: StdStringPrinter(target_wide_charset, val)
+    pretty_printers_dict[re.compile('^std::basic_string<char16_t,.*>$')] = lambda val: StdStringPrinter('UTF-16', val)
+    pretty_printers_dict[re.compile('^std::basic_string<char32_t,.*>$')] = lambda val: StdStringPrinter('UTF-32', val)
+    pretty_printers_dict[re.compile('^std::bitset<.*>$')] = StdBitsetPrinter
+    pretty_printers_dict[re.compile('^std::deque<.*>$')] = StdDequePrinter
+    pretty_printers_dict[re.compile('^std::list<.*>$')] = StdListPrinter
+    pretty_printers_dict[re.compile('^std::map<.*>$')] = lambda val: StdMapPrinter("std::map", val)
+    pretty_printers_dict[re.compile('^std::multimap<.*>$')] = lambda val: StdMapPrinter("std::multimap", val)
+    pretty_printers_dict[re.compile('^std::multiset<.*>$')] = lambda val: StdSetPrinter("std::multiset", val)
+    pretty_printers_dict[re.compile('^std::priority_queue<.*>$')] = lambda val: StdStackOrQueuePrinter("std::priority_queue", val)
+    pretty_printers_dict[re.compile('^std::queue<.*>$')] = lambda val: StdStackOrQueuePrinter("std::queue", val)
+    pretty_printers_dict[re.compile('^std::set<.*>$')] = lambda val: StdSetPrinter("std::set", val)
+    pretty_printers_dict[re.compile('^std::stack<.*>$')] = lambda val: StdStackOrQueuePrinter("std::stack", val)
+    pretty_printers_dict[re.compile('^std::vector<.*>$')] = StdVectorPrinter
     # vector<bool>
 
     # C++0x stuff.
     # array - the default seems reasonable
     # smart_ptr?  seems to only be in boost right now
-    obj.pretty_printers['^std::tr1::shared_ptr<.*>$'] = lambda val: StdPointerPrinter ('std::shared_ptr', val)
-    obj.pretty_printers['^std::tr1::weak_ptr<.*>$'] = lambda val: StdPointerPrinter ('std::weak_ptr', val)
-    obj.pretty_printers['^std::tr1::unique_ptr<.*>$'] = UniquePointerPrinter
-    obj.pretty_printers['^std::tr1::unordered_map<.*>$'] = lambda val: Tr1UnorderedMapPrinter ('std::tr1::unordered_map', val)
-    obj.pretty_printers['^std::tr1::unordered_set<.*>$'] = lambda val: Tr1UnorderedSetPrinter ('std::tr1::unordered_set', val)
-    obj.pretty_printers['^std::tr1::unordered_multimap<.*>$'] = lambda val: Tr1UnorderedMapPrinter ('std::tr1::unordered_multimap', val)
-    obj.pretty_printers['^std::tr1::unordered_multiset<.*>$'] = lambda val: Tr1UnorderedSetPrinter ('std::tr1::unordered_multiset', val)
+    pretty_printers_dict[re.compile('^std::tr1::shared_ptr<.*>$')] = lambda val: StdPointerPrinter ('std::shared_ptr', val)
+    pretty_printers_dict[re.compile('^std::tr1::weak_ptr<.*>$')] = lambda val: StdPointerPrinter ('std::weak_ptr', val)
+    pretty_printers_dict[re.compile('^std::tr1::unique_ptr<.*>$')] = UniquePointerPrinter
+    pretty_printers_dict[re.compile('^std::tr1::unordered_map<.*>$')] = lambda val: Tr1UnorderedMapPrinter ('std::tr1::unordered_map', val)
+    pretty_printers_dict[re.compile('^std::tr1::unordered_set<.*>$')] = lambda val: Tr1UnorderedSetPrinter ('std::tr1::unordered_set', val)
+    pretty_printers_dict[re.compile('^std::tr1::unordered_multimap<.*>$')] = lambda val: Tr1UnorderedMapPrinter ('std::tr1::unordered_multimap', val)
+    pretty_printers_dict[re.compile('^std::tr1::unordered_multiset<.*>$')] = lambda val: Tr1UnorderedSetPrinter ('std::tr1::unordered_multiset', val)
 
     # Extensions.
-    obj.pretty_printers['^__gnu_cxx::slist<.*>$'] = StdSlistPrinter
+    pretty_printers_dict[re.compile('^__gnu_cxx::slist<.*>$')] = StdSlistPrinter
 
     if True:
         # These shouldn't be necessary, if GDB "print *i" worked.
         # But it often doesn't, so here they are.
-        obj.pretty_printers['^std::_List_iterator<.*>$'] = lambda val: StdListIteratorPrinter(val)
-        obj.pretty_printers['^std::_List_const_iterator<.*>$'] = lambda val: StdListIteratorPrinter(val)
-        obj.pretty_printers['^std::_Rb_tree_iterator<.*>$'] = lambda val: StdRbtreeIteratorPrinter(val)
-        obj.pretty_printers['^std::_Rb_tree_const_iterator<.*>$'] = lambda val: StdRbtreeIteratorPrinter(val)
-        obj.pretty_printers['^std::_Deque_iterator<.*>$'] = lambda val: StdDequeIteratorPrinter(val)
-        obj.pretty_printers['^std::_Deque_const_iterator<.*>$'] = lambda val: StdDequeIteratorPrinter(val)
-        obj.pretty_printers['^__gnu_cxx::__normal_iterator<.*>$'] = lambda val: StdVectorIteratorPrinter(val)
-        obj.pretty_printers['^__gnu_cxx::_Slist_iterator<.*>$'] = lambda val: StdSlistIteratorPrinter(val)
-
+        pretty_printers_dict[re.compile('^std::_List_iterator<.*>$')] = lambda val: StdListIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^std::_List_const_iterator<.*>$')] = lambda val: StdListIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^std::_Rb_tree_iterator<.*>$')] = lambda val: StdRbtreeIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^std::_Rb_tree_const_iterator<.*>$')] = lambda val: StdRbtreeIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^std::_Deque_iterator<.*>$')] = lambda val: StdDequeIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^std::_Deque_const_iterator<.*>$')] = lambda val: StdDequeIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^__gnu_cxx::__normal_iterator<.*>$')] = lambda val: StdVectorIteratorPrinter(val)
+        pretty_printers_dict[re.compile('^__gnu_cxx::_Slist_iterator<.*>$')] = lambda val: StdSlistIteratorPrinter(val)
+
+pretty_printers_dict = {}
+
+build_libstdcxx_dictionary ()
 register_libstdcxx_printers (gdb.current_objfile())
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index afdf821..e2fc9f1 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -161,8 +161,8 @@ int gdbpy_is_string (PyObject *obj);
    other pretty-printer functions, because they refer to PyObject.  */
 char *apply_varobj_pretty_printer (PyObject *print_obj, struct value *value,
 				   struct value **replacement);
-PyObject *gdbpy_get_varobj_pretty_printer (struct type *type);
-PyObject *gdbpy_instantiate_printer (PyObject *cons, struct value *value);
+PyObject *gdbpy_get_varobj_pretty_printer (struct value *value);
+PyObject *gdbpy_instantiate_printer (PyObject *cons, PyObject *value);
 char *gdbpy_get_display_hint (PyObject *printer);
 
 extern PyObject *gdbpy_children_cst;
diff --git a/gdb/python/python-objfile.c b/gdb/python/python-objfile.c
index a225409..e97d3a2 100644
--- a/gdb/python/python-objfile.c
+++ b/gdb/python/python-objfile.c
@@ -1,6 +1,6 @@
 /* Python interface to objfiles.
 
-   Copyright (C) 2008 Free Software Foundation, Inc.
+   Copyright (C) 2008, 2009 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -29,7 +29,7 @@ typedef struct
   /* The corresponding objfile.  */
   struct objfile *objfile;
 
-  /* The pretty-printer dictionary.  */
+  /* The pretty-printer list of functions.  */
   PyObject *printers;
 } objfile_object;
 
@@ -66,7 +66,7 @@ objfpy_new (PyTypeObject *type, PyObject *args, PyObject *keywords)
     {
       self->objfile = NULL;
 
-      self->printers = PyDict_New ();
+      self->printers = PyList_New (0);
       if (!self->printers)
 	{
 	  Py_DECREF (self);
@@ -95,10 +95,10 @@ objfpy_set_printers (PyObject *o, PyObject *value, void *ignore)
       return -1;
     }
 
-  if (! PyDict_Check (value))
+  if (! PyList_Check (value))
     {
       PyErr_SetString (PyExc_TypeError,
-		       "the pretty_printers attribute must be a dictionary");
+		       "the pretty_printers attribute must be a list");
       return -1;
     }
 
@@ -139,7 +139,7 @@ objfile_to_objfile_object (struct objfile *objfile)
 
 	  object->objfile = objfile;
 
-	  object->printers = PyDict_New ();
+	  object->printers = PyList_New (0);
 	  if (!object->printers)
 	    {
 	      Py_DECREF (object);
diff --git a/gdb/python/python-type.c b/gdb/python/python-type.c
index 5aa82f6..d336456 100644
--- a/gdb/python/python-type.c
+++ b/gdb/python/python-type.c
@@ -1,6 +1,6 @@
 /* Python interface to types.
 
-   Copyright (C) 2008 Free Software Foundation, Inc.
+   Copyright (C) 2008, 2009 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -233,6 +233,15 @@ typy_tag (PyObject *self, PyObject *args)
   return PyString_FromString (TYPE_TAG_NAME (type));
 }
 
+/* Return the type, stripped of typedefs. */
+static PyObject *
+typy_strip_typedefs (PyObject *self, PyObject *args)
+{
+  struct type *type = ((type_object *) self)->type;
+
+  return type_to_type_object (check_typedef (type));
+}
+
 /* Return a Type object which represents a pointer to SELF.  */
 static PyObject *
 typy_pointer (PyObject *self, PyObject *args)
@@ -710,6 +719,8 @@ Each field is a dictionary." },
     "Return the size of this type, in bytes" },
   { "tag", typy_tag, METH_NOARGS,
     "Return the tag name for this type, or None." },
+  { "strip_typedefs", typy_strip_typedefs, METH_NOARGS,
+    "Return a type stripped of typedefs"},
   { "target", typy_target, METH_NOARGS,
     "Return the target type of this type" },
   { "template_argument", typy_template_argument, METH_VARARGS,
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 8677da6..3897132 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -842,53 +842,47 @@ get_type (struct type *type)
 }
 
 /* Helper function for find_pretty_printer which iterates over a
-   dictionary and tries to find a match.  */
+   list, calls each function and inspects output.  */
 static PyObject *
-search_pp_dictionary (PyObject *dict, char *type_name)
+search_pp_list (PyObject *list, PyObject *value)
 {
-  Py_ssize_t iter;
-  PyObject *key, *func, *found = NULL;
-
-  /* See if the type matches a pretty-printer regexp.  */
-  iter = 0;
-  while (! found && PyDict_Next (dict, &iter, &key, &func))
+  Py_ssize_t pp_list_size, list_index;
+  PyObject *function, *printer = NULL;
+  
+  pp_list_size = PyList_Size (list);
+  for (list_index = 0; list_index < pp_list_size; list_index++)
     {
-      char *rx_str;
+      function = PyList_GetItem (list, list_index);
+      if (! function)
+	return NULL;
+
+      /* gdbpy_instantiate_printer can return three possible return
+	 values:  NULL on error;  Py_None if the pretty-printer
+	 in the list cannot print the value; or a printer instance if
+	 the printer can print the value.  */
+      printer = gdbpy_instantiate_printer (function, value);
+      if (! printer)
+	return NULL;
+      else if (printer != Py_None)
+	return printer;
 
-      if (! PyString_Check (key))
-	continue;
-      rx_str = PyString_AsString (key);
-      if (re_comp (rx_str) == NULL && re_exec (type_name) == 1)
-	found = func;
+      Py_DECREF (Py_None);
     }
 
-  return found;
+  Py_RETURN_NONE;
 }
 
 /* Find the pretty-printing constructor function for TYPE.  If no
    pretty-printer exists, return NULL.  If one exists, return a new
    reference.  */
 static PyObject *
-find_pretty_printer (struct type *type)
+find_pretty_printer (PyObject *value)
 {
-  PyObject *dict, *found = NULL;
-  char *type_name = NULL;
+  PyObject *pp_list = NULL;
+  PyObject *function = NULL;
   struct objfile *obj;
   volatile struct gdb_exception except;
 
-  /* Get the name of the type.  */
-  TRY_CATCH (except, RETURN_MASK_ALL)
-    {
-      /* If we have a reference, use the referenced type.  */
-      if (TYPE_CODE (type) == TYPE_CODE_REF)
-	type = TYPE_TARGET_TYPE (type);
-      /* Strip off any qualifiers from the type.  */
-      type = make_cv_type (0, 0, type, NULL);
-      type_name = get_type (type);
-    }
-  if (except.reason < 0)
-    return NULL;
-
   /* Look at the pretty-printer dictionary for each objfile.  */
   ALL_OBJFILES (obj)
   {
@@ -896,34 +890,43 @@ find_pretty_printer (struct type *type)
     if (!objf)
       continue;
 
-    dict = objfpy_get_printers (objf, NULL);
-    found = search_pp_dictionary (dict, type_name);
-    if (found)
-      goto done;
+    pp_list = objfpy_get_printers (objf, NULL);
+    function = search_pp_list (pp_list, value);
+
+    /* If there is an error in any objfile list, abort the search and
+       exit.  */
+    if (! function)
+      {
+	Py_XDECREF (pp_list);
+	return NULL;
+      }
 
-    Py_DECREF (dict);
+    if (function && function != Py_None)
+      goto done;
+    
+    /* In this loop, if function is not an instantiation of a
+    pretty-printer, and it is not null, then it is a return of
+    Py_RETURN_NONE, which must be decremented.  */
+    Py_DECREF (function);
+    Py_XDECREF (pp_list);
   }
 
+  pp_list = NULL;
   /* Fetch the global pretty printer dictionary.  */
-  dict = NULL;
   if (! PyObject_HasAttrString (gdb_module, "pretty_printers"))
     goto done;
-  dict = PyObject_GetAttrString (gdb_module, "pretty_printers");
-  if (! dict)
+  pp_list = PyObject_GetAttrString (gdb_module, "pretty_printers");
+  if (! pp_list)
     goto done;
-  if (! PyDict_Check (dict) || ! PyDict_Size (dict))
+  if (! PyList_Check (pp_list))
     goto done;
 
-  found = search_pp_dictionary (dict, type_name);
+  function = search_pp_list (pp_list, value);
 
  done:
-  xfree (type_name);
-
-  if (found)
-    Py_INCREF (found);
-  Py_XDECREF (dict);
-
-  return found;
+  Py_XDECREF (pp_list);
+  
+  return function;
 }
 
 /* Pretty-print a single value, via the printer object PRINTER.  If
@@ -964,24 +967,13 @@ pretty_print_one_value (PyObject *printer, struct value **out_value)
 }
 
 /* Instantiate a pretty-printer given a constructor, CONS, and a
-   value, VAL.  Return NULL on error.  */
+   value, VAL.  Return NULL on error.  Ownership of the object
+   instance is transferred to the reciever */
 PyObject *
-gdbpy_instantiate_printer (PyObject *cons, struct value *value)
+gdbpy_instantiate_printer (PyObject *cons, PyObject *value)
 {
-  PyObject *val_obj = NULL, *result;
-  volatile struct gdb_exception except;
-
-  TRY_CATCH (except, RETURN_MASK_ALL)
-    {
-      /* FIXME: memory management here.  Why are values so
-	 funny?  */
-      value = value_copy (value);
-      val_obj = value_to_value_object (value);
-    }
-  GDB_PY_HANDLE_EXCEPTION (except);
-
-  result = PyObject_CallFunctionObjArgs (cons, val_obj, NULL);
-  Py_DECREF (val_obj);
+  PyObject *result;
+  result = PyObject_CallFunctionObjArgs (cons, value, NULL);
   return result;
 }
 
@@ -1174,9 +1166,12 @@ print_children (PyObject *printer, const char *hint,
 
       if (! item)
 	{
+	  if (PyErr_Occurred ())
+	    gdbpy_print_stack ();
 	  /* Set a flag so we can know whether we printed all the
 	     available elements.  */
-	  done_flag = 1;
+	  else	  
+	    done_flag = 1;
 	  break;
 	}
 
@@ -1296,7 +1291,8 @@ apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
 			  const struct value_print_options *options,
 			  const struct language_defn *language)
 {
-  PyObject *func, *printer;
+  PyObject *printer = NULL;
+  PyObject *val_obj = NULL;
   struct value *value;
   char *hint = NULL;
   struct cleanup *cleanups;
@@ -1306,36 +1302,30 @@ apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
   state = PyGILState_Ensure ();
   cleanups = make_cleanup_py_restore_gil (&state);
 
-  /* Find the constructor.  */
-  func = find_pretty_printer (type);
-  if (! func)
-    goto done;
-
   /* Instantiate the printer.  */
   if (valaddr)
     valaddr += embedded_offset;
   value = value_from_contents_and_address (type, valaddr, address);
-  printer = gdbpy_instantiate_printer (func, value);
-  Py_DECREF (func);
 
-  if (!printer)
-    {
-      gdbpy_print_stack ();
-      goto done;
-    }
+  val_obj = value_to_value_object (value);
+  if (! val_obj)
+    goto done;
+  
+  /* Find the constructor.  */
+  printer = find_pretty_printer (val_obj);
+  make_cleanup_py_decref (printer);
+  if (! printer || printer == Py_None)
+    goto done;
 
   /* If we are printing a map, we want some special formatting.  */
   hint = gdbpy_get_display_hint (printer);
   make_cleanup (free_current_contents, &hint);
 
-  make_cleanup_py_decref (printer);
-  if (printer != Py_None)
-    {
-      print_string_repr (printer, hint, stream, recurse, options, language);
-      print_children (printer, hint, stream, recurse, options, language);
+  /* Print the section */
+  print_string_repr (printer, hint, stream, recurse, options, language);
+  print_children (printer, hint, stream, recurse, options, language);
+  result = 1;
 
-      result = 1;
-    }
 
  done:
   do_cleanups (cleanups);
@@ -1367,13 +1357,29 @@ apply_varobj_pretty_printer (PyObject *printer_obj, struct value *value,
 }
 
 /* Find a pretty-printer object for the varobj module.  Returns a new
-   reference to the object if successful; returns NULL if not.  TYPE
-   is the type of the varobj for which a printer should be
-   returned.  */
+   reference to the object if successful; returns NULL if not.  VALUE
+   is the value for which a printer tests to determine if it 
+   can pretty-print the value.  */
 PyObject *
-gdbpy_get_varobj_pretty_printer (struct type *type)
+gdbpy_get_varobj_pretty_printer (struct value *value)
 {
-  return find_pretty_printer (type);
+  PyObject *val_obj;
+  PyObject *pretty_printer = NULL;
+  volatile struct gdb_exception except;
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      value = value_copy (value);
+    }
+  GDB_PY_HANDLE_EXCEPTION (except);
+  
+  val_obj = value_to_value_object (value);
+  if (! val_obj)
+    return NULL;
+
+  pretty_printer = find_pretty_printer (val_obj);
+  Py_DECREF (val_obj);
+  return pretty_printer;
 }
 
 /* A Python function which wraps find_pretty_printer and instantiates
@@ -1396,24 +1402,8 @@ gdbpy_default_visualizer (PyObject *self, PyObject *args)
       return NULL;
     }
 
-  cons = find_pretty_printer (value_type (value));
-  if (cons)
-    {
-      /* While it is a bit lame to pass value here and make a new
-	 Value, it is probably better to share the instantiation
-	 code.  */
-      printer = gdbpy_instantiate_printer (cons, value);
-      Py_DECREF (cons);
-    }
-
-  if (!printer)
-    {
-      PyErr_Clear ();
-      printer = Py_None;
-      Py_INCREF (printer);
-    }
-
-  return printer;
+  cons = find_pretty_printer (val_obj);
+  return cons;
 }
 
 #else /* HAVE_PYTHON */
@@ -1566,7 +1556,7 @@ Enables or disables auto-loading of Python code when an object is opened."),
   gdbpy_initialize_membuf ();
 
   PyRun_SimpleString ("import gdb");
-  PyRun_SimpleString ("gdb.pretty_printers = {}");
+  PyRun_SimpleString ("gdb.pretty_printers = []");
 
   observer_attach_new_objfile (gdbpy_new_objfile);
 
diff --git a/gdb/testsuite/gdb.python/python-prettyprint.py b/gdb/testsuite/gdb.python/python-prettyprint.py
index ecd4fc6..0d9cb87 100644
--- a/gdb/testsuite/gdb.python/python-prettyprint.py
+++ b/gdb/testsuite/gdb.python/python-prettyprint.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2008 Free Software Foundation, Inc.
+# Copyright (C) 2008, 2009 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
@@ -16,6 +16,8 @@
 # This file is part of the GDB testsuite.  It tests python pretty
 # printers.
 
+import re
+
 # Test returning a Value from a printer.
 class string_print:
     def __init__(self, val):
@@ -76,21 +78,57 @@ class pp_sss:
     def to_string(self):
         return "a=<" + str(self.val['a']) + "> b=<" + str(self.val["b"]) + ">"
 
-gdb.pretty_printers['^struct s$']   = pp_s
-gdb.pretty_printers['^s$']   = pp_s
-gdb.pretty_printers['^S$']   = pp_s
-
-gdb.pretty_printers['^struct ss$']  = pp_ss
-gdb.pretty_printers['^ss$']  = pp_ss
-
-gdb.pretty_printers['^const S &$']   = pp_s
-gdb.pretty_printers['^SSS$']  = pp_sss
-
-# Note that we purposely omit the typedef names here.
-# Printer lookup is based on canonical name.
-# However, we do need both tagged and untagged variants, to handle
-# both the C and C++ cases.
-gdb.pretty_printers['^struct string_repr$'] = string_print
-gdb.pretty_printers['^struct container$'] = ContainerPrinter
-gdb.pretty_printers['^string_repr$'] = string_print
-gdb.pretty_printers['^container$'] = ContainerPrinter
+def lookup_function (val):
+    "Look-up and return a pretty-printer that can print val."
+
+    # Get the type.
+    type = val.type ();
+
+    # If it points to a reference, get the reference.
+    if type.code () == gdb.TYPE_CODE_REF:
+        type = type.target ()
+
+    # Get the unqualified type, stripped of typedefs.
+    type = type.unqualified ().strip_typedefs ()
+
+    # Get the type name.    
+    typename = type.tag ()
+
+    if typename == None:
+        return None
+
+    # Iterate over local dictionary of types to determine
+    # if a printer is registered for that type.  Return an
+    # instantiation of the printer if found.
+    for function in pretty_printers_dict:
+        if function.match (typename):
+            return pretty_printers_dict[function] (val)
+        
+    # Cannot find a pretty printer.  Return None.
+
+    return None
+
+
+def register_pretty_printers ():
+    pretty_printers_dict[re.compile ('^struct s$')]   = pp_s
+    pretty_printers_dict[re.compile ('^s$')]   = pp_s
+    pretty_printers_dict[re.compile ('^S$')]   = pp_s
+
+    pretty_printers_dict[re.compile ('^struct ss$')]  = pp_ss
+    pretty_printers_dict[re.compile ('^ss$')]  = pp_ss
+    pretty_printers_dict[re.compile ('^const S &$')]   = pp_s
+    pretty_printers_dict[re.compile ('^SSS$')]  = pp_sss
+    
+    # Note that we purposely omit the typedef names here.
+    # Printer lookup is based on canonical name.
+    # However, we do need both tagged and untagged variants, to handle
+    # both the C and C++ cases.
+    pretty_printers_dict[re.compile ('^struct string_repr$')] = string_print
+    pretty_printers_dict[re.compile ('^struct container$')] = ContainerPrinter
+    pretty_printers_dict[re.compile ('^string_repr$')] = string_print
+    pretty_printers_dict[re.compile ('^container$')] = ContainerPrinter
+    
+pretty_printers_dict = {}
+
+register_pretty_printers ()
+gdb.pretty_printers.append (lookup_function)
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 7f77902..150d8f8 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -181,10 +181,6 @@ struct varobj
   int from;
   int to;
 
-  /* If NULL, no pretty printer is installed.  If not NULL, the
-     constructor to call to get a pretty-printer object.  */
-  PyObject *constructor;
-
   /* The pretty-printer that has been constructed.  If NULL, then a
      new printer object is needed, and one will be constructed.  */
   PyObject *pretty_printer;
@@ -711,21 +707,29 @@ varobj_delete (struct varobj *var, char ***dellist, int only_children)
   return delcount;
 }
 
-/* Instantiate a pretty-printer for a given value.  */
+/* Convenience function for varobj_set_visualizer.  Instantiate a
+   pretty-printer for a given value.  */
 static PyObject *
-instantiate_pretty_printer (struct varobj *var, struct value *value)
+instantiate_pretty_printer (PyObject *constructor, struct value *value)
 {
 #if HAVE_PYTHON
-  if (var->constructor)
+  PyObject *val_obj = NULL; 
+  PyObject *printer;
+  volatile struct gdb_exception except;
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
     {
-      PyObject *printer = gdbpy_instantiate_printer (var->constructor, value);
-      if (printer == Py_None)
-	{
-	  Py_DECREF (printer);
-	  printer = NULL;
-	}
-      return printer;
+      value = value_copy (value);
     }
+  GDB_PY_HANDLE_EXCEPTION (except);
+  val_obj = value_to_value_object (value);
+
+  if (! val_obj)
+    return NULL;
+
+  printer = gdbpy_instantiate_printer (constructor, val_obj);
+  Py_DECREF (val_obj);
+  return printer;
 #endif
   return NULL;
 }
@@ -1224,7 +1228,7 @@ install_new_value (struct varobj *var, struct value *value, int initial)
   /* If the type has custom visualizer, we consider it to be always
      changeable. FIXME: need to make sure this behaviour will not
      mess up read-sensitive values.  */
-  if (var->constructor)
+  if (var->pretty_printer)
     changeable = 1;
 
   need_to_fetch = changeable;
@@ -1278,20 +1282,6 @@ install_new_value (struct varobj *var, struct value *value, int initial)
 	}
     }
 
-#if HAVE_PYTHON
-  {
-    PyGILState_STATE state = PyGILState_Ensure ();
-    if (var->pretty_printer)
-      {
-	Py_DECREF (var->pretty_printer);
-      }
-    if (value)
-      var->pretty_printer = instantiate_pretty_printer (var, value);
-    else
-      var->pretty_printer = NULL;
-    PyGILState_Release (state);
-  }
-#endif
 
   /* Below, we'll be comparing string rendering of old and new
      values.  Don't get string rendering if the value is
@@ -1408,11 +1398,8 @@ install_visualizer (struct varobj *var, PyObject *visualizer)
   varobj_delete (var, NULL, 1 /* children only */);
   var->num_children = -1;
 
-  Py_XDECREF (var->constructor);
-  var->constructor = visualizer;
-
   Py_XDECREF (var->pretty_printer);
-  var->pretty_printer = NULL;
+  var->pretty_printer = visualizer;
 
   install_new_value (var, var->value, 1);
 
@@ -1433,15 +1420,28 @@ install_default_visualizer (struct varobj *var)
 #if HAVE_PYTHON
   struct cleanup *cleanup;
   PyGILState_STATE state;
-  PyObject *constructor = NULL;
+  PyObject *pretty_printer = NULL;
 
   state = PyGILState_Ensure ();
   cleanup = make_cleanup_py_restore_gil (&state);
 
-  if (var->type)
-    constructor = gdbpy_get_varobj_pretty_printer (var->type);
-  install_visualizer (var, constructor);
-
+  if (var->value)
+    {
+      pretty_printer = gdbpy_get_varobj_pretty_printer (var->value);
+      if (! pretty_printer)
+	{
+	  gdbpy_print_stack ();
+	  error (_("Cannot instantiate printer for default visualizer"));
+	}
+    }
+      
+  if (pretty_printer == Py_None)
+    {
+      Py_DECREF (pretty_printer);
+      pretty_printer = NULL;
+    }
+  
+  install_visualizer (var, pretty_printer);
   do_cleanups (cleanup);
 #else
   error ("Python support required");
@@ -1452,10 +1452,11 @@ void
 varobj_set_visualizer (struct varobj *var, const char *visualizer)
 {
 #if HAVE_PYTHON
-  PyObject *mainmod, *globals, *constructor;
-  struct cleanup *back_to;
+  PyObject *mainmod, *globals, *pretty_printer, *constructor;
+  struct cleanup *back_to, *value;
   PyGILState_STATE state;
 
+
   state = PyGILState_Ensure ();
   back_to = make_cleanup_py_restore_gil (&state);
 
@@ -1465,20 +1466,31 @@ varobj_set_visualizer (struct varobj *var, const char *visualizer)
   make_cleanup_py_decref (globals);
 
   constructor = PyRun_String (visualizer, Py_eval_input, globals, globals);
+  
+  /* Do not instantiate NoneType. */
+  if (constructor == Py_None)
+    {
+      pretty_printer = Py_None;
+      Py_INCREF (pretty_printer);
+    }
+  else
+    pretty_printer = instantiate_pretty_printer (constructor, var->value);
 
-  if (! constructor)
+  Py_XDECREF (constructor);
+
+  if (! pretty_printer)
     {
       gdbpy_print_stack ();
       error ("Could not evaluate visualizer expression: %s", visualizer);
     }
 
-  if (constructor == Py_None)
+  if (pretty_printer == Py_None)
     {
-      Py_DECREF (constructor);
-      constructor = NULL;
+      Py_DECREF (pretty_printer);
+      pretty_printer = NULL;
     }
 
-  install_visualizer (var, constructor);
+  install_visualizer (var, pretty_printer);
 
   do_cleanups (back_to);
 #else
@@ -1917,7 +1929,6 @@ new_variable (void)
   var->children_requested = 0;
   var->from = -1;
   var->to = -1;
-  var->constructor = 0;
   var->pretty_printer = 0;
 
   return var;
@@ -1954,7 +1965,6 @@ free_variable (struct varobj *var)
 #if HAVE_PYTHON
   {
     PyGILState_STATE state = PyGILState_Ensure ();
-    Py_XDECREF (var->constructor);
     Py_XDECREF (var->pretty_printer);
     PyGILState_Release (state);
   }
@@ -2753,7 +2763,7 @@ c_value_of_variable (struct varobj *var, enum varobj_display_formats format)
 
   /* If we have a custom formatter, return whatever string it has
      produced.  */
-  if (var->constructor && var->print_value)
+  if (var->pretty_printer && var->print_value)
     return xstrdup (var->print_value);
   
   /* Strip top-level references. */

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

* Re: [python] [rfc] Patch for pretty-printers registration API change
  2009-02-20 15:50       ` Phil Muldoon
@ 2009-02-20 19:38         ` Tom Tromey
  2009-02-24 11:30           ` Phil Muldoon
  0 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2009-02-20 19:38 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: Project Archer

Phil> I elected propagation. I was not terribly sure in the CLI case if I
Phil> should start applying the gdb error () and gdbpy_print_stack () inside
Phil> apply_val_pretty_printer () .. or allow the error to propagate
Phil> further. I elected to let them continue as that was the case
Phil> beforehand. We can easily change this. What do you think?

I think it is probably fine to just print the exception trace and
continue here.

Phil> +      printer = gdbpy_instantiate_printer (function, value);
Phil> +      if (! printer)
Phil> +	return NULL;
Phil> +      else if (printer != Py_None)
Phil> +	return printer;
[...]
Phil> +      Py_DECREF (Py_None);

I think it is much clearer to write Py_DECREF (printer) here.
Decreffing Py_None looks very weird to me.

Phil> +    if (! function)
Phil> +      {
Phil> +	Py_XDECREF (pp_list);
Phil> +	return NULL;
Phil> +      }
 
Phil> -    Py_DECREF (dict);
Phil> +    if (function && function != Py_None)
Phil> +      goto done;

I think the !function case above means that you don't have to test
function in the second if.

The function get_type is now unused, please delete it.

The 'value' argument to apply_varobj_pretty_printer is also unused --
remove it as well.

This patch is ok with the above changes.

thanks,
Tom

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

* Re: [python] [rfc] Patch for pretty-printers registration API change
  2009-02-20 19:38         ` Tom Tromey
@ 2009-02-24 11:30           ` Phil Muldoon
  2009-02-24 16:27             ` Tom Tromey
  0 siblings, 1 reply; 8+ messages in thread
From: Phil Muldoon @ 2009-02-24 11:30 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Project Archer

Tom Tromey wrote:
> This patch is ok with the above changes.
>   

Thanks. I updated the changes as requested. I have now checked the patch 
into archer-tromey-python. I have closed the 
archer-pmuldoon-pretty-printers-lookup branch and updated the wiki to 
indicate its closed status. One small note before we sign off on this ...

> Phil> I elected propagation. I was not terribly sure in the CLI case if I
> Phil> should start applying the gdb error () and gdbpy_print_stack () inside
> Phil> apply_val_pretty_printer () .. or allow the error to propagate
> Phil> further. I elected to let them continue as that was the case
> Phil> beforehand. We can easily change this. What do you think?
>
> I think it is probably fine to just print the exception trace and
> continue here.
>   


I was unsure of what you meant here. In the code, I elected to check for 
an exception within the "done" section of apply_val_pretty_printer; if 
one had occurred, I dumped stack and continued. If this is not what you 
meant we can change it.

Regards

Phil


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

* Re: [python] [rfc] Patch for pretty-printers registration API change
  2009-02-24 11:30           ` Phil Muldoon
@ 2009-02-24 16:27             ` Tom Tromey
  0 siblings, 0 replies; 8+ messages in thread
From: Tom Tromey @ 2009-02-24 16:27 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: Project Archer

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

>> I think it is probably fine to just print the exception trace and
>> continue here.

Phil> I was unsure of what you meant here. In the code, I elected to check
Phil> for an exception within the "done" section of
Phil> apply_val_pretty_printer; if one had occurred, I dumped stack and
Phil> continued. If this is not what you meant we can change it.

I think the code currently on the branch looks reasonable.

Tom

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

end of thread, other threads:[~2009-02-24 16:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-17 14:03 [python] [rfc] Patch for pretty-printers registration API change Phil Muldoon
2009-02-17 18:52 ` Tom Tromey
2009-02-18 14:40   ` Phil Muldoon
2009-02-18 19:15     ` Tom Tromey
2009-02-20 15:50       ` Phil Muldoon
2009-02-20 19:38         ` Tom Tromey
2009-02-24 11:30           ` Phil Muldoon
2009-02-24 16:27             ` Tom Tromey

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).