This is the mail archive of the archer@sourceware.org mailing list for the Archer project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [python] [rfc] Patch for pretty-printers registration API change


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
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. */

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]