This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB 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: [patch] pr10659 pretty-printing: Display vectors of vectors as matrix--rev 2


On 05/27/10 17:14, Tom Tromey wrote:
"Chris" == Chris Moller<cmoller@redhat.com> writes:

Chris> This patch adds the mechanism for gdb to respond to "matrix" hints Chris> from printers.py.

Thanks.

Chris>  (Just to exercise the testcase, the patch includes
Chris>  a patched version of printers.py to src/gdb/testsuite/gdb.python.  It
Chris>  can be removed once the patched printers.py is generally available.)

This is not ok. This part of the test suite ought to be independent
from the C++ compiler in use. Instead, write some new code (C is fine)
and an associated printer to exercise the new code in GDB. There are
other examples of this in gdb.python.

I'm not altogether sure what you meant by this, but what I did was rename printers.py to pr10659.py and strip out everything but the vector/matrix code and tinker pr10659.exp to match. This seems to be consistent with what py-prettyprint.* does--if I've guessed wrong, let me know.


(The real new printers.py has been moved to the right place in svn gcc and I'm creating the patch for that even as I type.)

Chris>  Question:  pr10659.exp yields:
Chris>     +ERROR: no fileid for qcore
Chris>     +ERROR: Couldn't send python print 'test' to GDB.
Chris>     +UNRESOLVED: gdb.python/pr10659.exp: verify python support
Chris>  The actual tests pass, but I haven't the slightest idea how to get rid
Chris>  of those errors or even if it's necessary to do so.

skip_python_tests requires a running gdb, so it must come after
prepare_for_testing.

done


Chris>  +  pretty = (is_array || is_matrix)
Chris>  +    ? options->prettyprint_arrays : options->pretty;

Wrong formatting.

Chris>  +  if (is_matrix&&  recurse == 0)
Chris>  +    fputs_filtered ("\n", stream);

I suspect this is not correct in the !pretty case.
Maybe for !pretty the matrix stuff should just be disabled, I don't see
what it would add.

A "matrix" hint turns on pretty mode--this seems to be consistent with what an "array" hint does.


Chris> + else if (is_matrix) ; /* Force a do-nothing. */

Formatting. Plus it is better to use empty braces with a comment
between, to emphasize the emptiness.

done


Chris> + static int is_matrix = 0;

There has to be a better way.  Static locals like this are generally
bad.

One idea would be a new field in value_print_options. This may be
slightly difficult depending on how recursion is done in some cases.

done, using an alloca copy of the options


Chris>  -  is_py_none = print_string_repr (printer, hint, stream, recurse,
Chris>  -				  options, language, gdbarch);
Chris>  +  is_py_none = is_matrix ? 1 : print_string_repr (printer, hint, stream,
Chris>  +						  recurse, options,
Chris>  +						  language, gdbarch);

What is the rationale for this change?

print_string_repr() is what prints the "std::vector of length..." stuff which I need to suppress for printing matrices, and matrix formatting needs is_py_none set to 1.



Chris> + if (recurse == 0) is_matrix = 0;

Formatting. Note also that this approach is not error-safe. You need a
cleanup. (Though with the value_print_options approach you won't,
because you can make a new local copy.)

see above--replaced by an alloca copy of options and a new matrix flag.
Chris>  RCS file: testsuite/gdb.python/printers.py
Chris>  diff -N testsuite/gdb.python/printers.py
[...]
Chris>  +print "in printers.py"

Debugging code.

done


Chris>  +    def display_hint(self):
Chris>  +        itype0  = self.val.type.template_argument(0)
Chris>  +        rc = 'array'
Chris>  +        if itype0.tag:
Chris>  +            if -1 != itype0.tag.find('vector'):

This is not a suitable test.  Instead extract the regular expression
from the recognizer and use that.  Ideally, stuff the compiled form in a
global and use it in both places:

Chris> + pretty_printers_dict[re.compile('^std::vector<.*>$')] = StdVectorPrinter

done, but it's part of this patch only as testcase code--the real thing is going into gcc. also, the latest version of printers.py from svn gcc was a little different in th pretty_printers_dict stuff and in StdVectorPrinter.__init__. my patch is with respect to that.


Tom

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.11858
diff -u -r1.11858 ChangeLog
--- ChangeLog	31 May 2010 07:00:40 -0000	1.11858
+++ ChangeLog	31 May 2010 18:35:08 -0000
@@ -1,3 +1,9 @@
+2010-05-31  Chris Moller  <cmoller@redhat.com>
+
+	* python/py-prettyprint.c (print_children): Add formatting for
+	matrices. (apply_val_pretty_printer): Detect and deal with matrix
+	hints. 
+
 2010-05-31  Pierre Muller  <muller@ics.u-strasbg.fr>
 
 	* windows-nat.c (GetConsoleFontSize, GetCurrentConsoleFont):
Index: valprint.h
===================================================================
RCS file: /cvs/src/src/gdb/valprint.h,v
retrieving revision 1.25
diff -u -r1.25 valprint.h
--- valprint.h	1 Jan 2010 07:31:43 -0000	1.25
+++ valprint.h	31 May 2010 18:35:09 -0000
@@ -31,6 +31,9 @@
   /* Controls pretty printing of arrays.  */
   int prettyprint_arrays;
 
+  /* Affects pretty printing of matrices.  */
+  int prettyprint_matrix;
+
   /* Controls pretty printing of structures.  */
   int prettyprint_structs;
 
Index: python/py-prettyprint.c
===================================================================
RCS file: /cvs/src/src/gdb/python/py-prettyprint.c,v
retrieving revision 1.10
diff -u -r1.10 py-prettyprint.c
--- python/py-prettyprint.c	17 May 2010 21:23:25 -0000	1.10
+++ python/py-prettyprint.c	31 May 2010 18:35:10 -0000
@@ -433,7 +433,8 @@
 
   /* Use the prettyprint_arrays option if we are printing an array,
      and the pretty option otherwise.  */
-  pretty = is_array ? options->prettyprint_arrays : options->pretty;
+  pretty = (is_array || options->prettyprint_matrix) ?
+    options->prettyprint_arrays : options->pretty;
 
   /* Manufacture a dummy Python frame to work around Python 2.4 bug,
      where it insists on having a non-NULL tstate->frame when
@@ -445,6 +446,9 @@
       goto done;
     }
   make_cleanup_py_decref (frame);
+  
+  if (options->prettyprint_matrix && recurse == 0)
+    fputs_filtered ("\n", stream);
 
   done_flag = 0;
   for (i = 0; i < options->print_max; ++i)
@@ -479,12 +483,23 @@
 	 3. Other.  Always print a ",".  */
       if (i == 0)
 	{
-         if (is_py_none)
-           fputs_filtered ("{", stream);
-         else
-           fputs_filtered (" = {", stream);
+	  if (options->prettyprint_matrix && recurse == 0)
+	    print_spaces_filtered (2 + 2 * recurse, stream);
+	  if (is_py_none)
+	    {
+	      if (options->prettyprint_matrix && strcmp (hint, "array"))
+		{
+		  fputs_filtered ("{\n", stream);
+		  print_spaces_filtered (4 + 2 * recurse, stream);
+		}
+	      else
+		fputs_filtered ("{", stream);
+	    }
+	  else
+	    fputs_filtered (" = {", stream);
        }
-
+      else if (options->prettyprint_matrix)
+	print_spaces_filtered (4 + 2 * recurse, stream);
       else if (! is_map || i % 2 == 0)
 	fputs_filtered (pretty ? "," : ", ", stream);
 
@@ -513,6 +528,10 @@
 
       if (is_map && i % 2 == 0)
 	fputs_filtered ("[", stream);
+      else if (options->prettyprint_matrix)
+	{
+	  /* Force a do-nothing.  */
+	}
       else if (is_array)
 	{
 	  /* We print the index, not whatever the child method
@@ -587,7 +606,12 @@
 	  fputs_filtered ("\n", stream);
 	  print_spaces_filtered (2 * recurse, stream);
 	}
-      fputs_filtered ("}", stream);
+      if (options->prettyprint_matrix)
+      {
+	print_spaces_filtered (4 * recurse, stream);
+	fputs_filtered ("}\n", stream);
+      }
+      else fputs_filtered ("}", stream);
     }
 
  done:
@@ -609,6 +633,7 @@
   struct cleanup *cleanups;
   int result = 0;
   int is_py_none = 0;
+  struct value_print_options *options_copy;
   cleanups = ensure_python_env (gdbarch, language);
 
   /* Instantiate the printer.  */
@@ -630,12 +655,23 @@
 
   /* If we are printing a map, we want some special formatting.  */
   hint = gdbpy_get_display_hint (printer);
+  
+  if (recurse == 0)
+    {
+      options_copy = alloca (sizeof (struct value_print_options));
+      memcpy (options_copy, options, sizeof (struct value_print_options));
+      options_copy->prettyprint_matrix = hint && !strcmp (hint, "matrix");
+    }
+  else options_copy = (struct value_print_options *)options;
+
   make_cleanup (free_current_contents, &hint);
 
   /* Print the section */
-  is_py_none = print_string_repr (printer, hint, stream, recurse,
-				  options, language, gdbarch);
-  print_children (printer, hint, stream, recurse, options, language,
+  is_py_none = options_copy->prettyprint_matrix ?
+    1 : print_string_repr (printer, hint, stream,
+			   recurse, options_copy,
+			   language, gdbarch);
+  print_children (printer, hint, stream, recurse, options_copy, language,
 		  is_py_none);
 
   result = 1;
Index: testsuite/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
retrieving revision 1.2294
diff -u -r1.2294 ChangeLog
--- testsuite/ChangeLog	31 May 2010 03:31:16 -0000	1.2294
+++ testsuite/ChangeLog	31 May 2010 18:35:26 -0000
@@ -1,3 +1,10 @@
+2010-05-31  Chris Moller  <cmoller@redhat.com>
+
+	* gdb.python/Makefile.in (EXECUTABLES):  Added pr10659.
+	* gdb.python/pr10659.cc:  New file.
+	* gdb.python/pr10659.exp.  New file.
+	* gdb.python/pr10659.py: New file.
+
 2010-05-31  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	Accept the new Linux kernel "t (tracing stop)" string.
Index: testsuite/gdb.python/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.python/Makefile.in,v
retrieving revision 1.6
diff -u -r1.6 Makefile.in
--- testsuite/gdb.python/Makefile.in	9 Apr 2010 09:41:43 -0000	1.6
+++ testsuite/gdb.python/Makefile.in	31 May 2010 18:35:28 -0000
@@ -2,7 +2,7 @@
 srcdir = @srcdir@
 
 EXECUTABLES = py-type py-value py-prettyprint py-template py-block \
-		py-symbol py-mi py-breakpoint
+		py-symbol py-mi py-breakpoint pr10659
 
 all info install-info dvi install uninstall installcheck check:
 	@echo "Nothing to be done for $@..."
Index: testsuite/gdb.python/pr10659.cc
===================================================================
RCS file: testsuite/gdb.python/pr10659.cc
diff -N testsuite/gdb.python/pr10659.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.python/pr10659.cc	31 May 2010 18:35:28 -0000
@@ -0,0 +1,43 @@
+#include <list>
+#include <vector>  // /usr/include/c++/4.4.1/bits/vector.tcc
+#include <iostream>
+
+using namespace std;
+
+int use_windows = 9999;
+
+int
+main(){
+  vector<int> test1(2,0);
+  test1[0]=8;
+  test1[1]=9;
+  
+  vector< vector<int> > test2(3, vector<int>(2,0));
+  test2[0][0]=0;
+  test2[0][1]=1;
+  test2[1][0]=2;
+  test2[1][1]=3;
+  test2[2][0]=4;
+  test2[2][1]=5;
+
+#define NR_ROWS    2
+#define NR_COLS    3
+#define NR_PLANES  4
+  vector<int> rows(NR_ROWS, 0);
+  vector< vector<int> > columns(NR_COLS, rows);
+  vector< vector < vector<int> > > test3(NR_PLANES, columns);
+
+  cout << "rows.size() = " << rows.size()
+       << ", columns.size() = " << columns.size()
+       << ", test3.size() = " << test3.size() << "\n";
+
+  for (int i = 0; i < rows.size(); i++) {
+    for (int j = 0; j < columns.size(); j++) {
+      for (int k = 0; k < test3.size(); k++) {
+	test3[k][j][i] = k * 100 + j * 10 + i;
+      }
+    }
+  }
+  
+  return 0;  // break
+}
Index: testsuite/gdb.python/pr10659.exp
===================================================================
RCS file: testsuite/gdb.python/pr10659.exp
diff -N testsuite/gdb.python/pr10659.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.python/pr10659.exp	31 May 2010 18:35:28 -0000
@@ -0,0 +1,82 @@
+#Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set nl             "\[\r\n\]+"
+
+set testfile pr10659
+set srcfile ${testfile}.cc
+if [prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}] {
+    return -1
+}
+
+if { [skip_python_tests] } { continue }
+
+gdb_test "python execfile(\"$srcdir/$subdir/pr10659.py\")" ""
+gdb_test "python gdb.pretty_printers = \[lookup_function\]" ""
+
+if ![runto_main] then {
+    fail "Can't run to main"
+    return
+}
+
+gdb_breakpoint [gdb_get_line_number "break"]
+gdb_continue_to_breakpoint "break"
+
+gdb_test "p test1" "vector of length 2, capacity 2 =.*"
+
+gdb_test "p test2" "= $nl  {$nl    {.*"
+
+# Complete result is:
+#
+# (gdb) p test2
+# $2 =
+#   {
+#     {0      1    }
+#     {2      3    }
+#     {4      5    }
+#  }
+
+
+gdb_test "p test3" "= $nl  {$nl    {$nl      {.*"
+
+# Complete result is:
+#
+# (gdb) p test3
+# $3 =
+#   {
+#     {
+#       {0        1        }
+#       {10        11        }
+#       {20        21        }
+#     }
+#     {
+#       {100        101        }
+#       {110        111        }
+#       {120        121        }
+#     }
+#     {
+#       {200        201        }
+#       {210        211        }
+#       {220        221        }
+#     }
+#     {
+#       {300        301        }
+#       {310        311        }
+#       {320        321        }
+#     }
+#  }
+# 
+
+
Index: testsuite/gdb.python/pr10659.py
===================================================================
RCS file: testsuite/gdb.python/pr10659.py
diff -N testsuite/gdb.python/pr10659.py
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.python/pr10659.py	31 May 2010 18:35:28 -0000
@@ -0,0 +1,109 @@
+# 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
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import gdb
+import itertools
+import re
+
+vector_sig = 'std::vector'
+vector_regex = re.compile('^' + vector_sig + '<.*>$')
+
+class FakeVectorPrinter:
+    "Print a std::vector"
+
+    class _iterator:
+        def __init__ (self, start, finish):
+            self.item = start
+            self.finish = finish
+            self.count = 0
+
+        def __iter__(self):
+            return self
+
+        def next(self):
+            if self.item == self.finish:
+                raise StopIteration
+            count = self.count
+            self.count = self.count + 1
+            elt = self.item.dereference()
+            self.item = self.item + 1
+            return ('[%d]' % count, elt)
+
+    def __init__(self, typename, val):
+        self.typename = typename
+        self.val = val
+
+    def children(self):
+        return self._iterator(self.val['_M_impl']['_M_start'],
+                              self.val['_M_impl']['_M_finish'])
+
+    def to_string(self):
+        start = self.val['_M_impl']['_M_start']
+        finish = self.val['_M_impl']['_M_finish']
+        end = self.val['_M_impl']['_M_end_of_storage']
+        return ('std::vector of length %d, capacity %d'
+                % (int (finish - start), int (end - start)))
+
+    def display_hint(self):
+        itype0  = self.val.type.template_argument(0)
+        itag = itype0.tag
+        if itag and re.match(vector_regex, itag):
+            rc = 'matrix'
+        else:
+            rc = 'array'
+        return rc
+
+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 fake_pretty_printers_dict:
+        if function.search (typename):
+            return fake_pretty_printers_dict[function] (val)
+        
+    # Cannot find a pretty printer.  Return None.
+    return None
+
+def build_libfakecxx_dictionary ():
+    fake_pretty_printers_dict[vector_regex] = lambda val: FakeVectorPrinter(vector_sig, val)
+
+fake_pretty_printers_dict = {}
+
+build_libfakecxx_dictionary ()

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