From: Chris Moller <cmoller@redhat.com>
To: tromey@redhat.com
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [patch] pr10659 pretty-printing: Display vectors of vectors as matrix--rev 2
Date: Mon, 31 May 2010 23:21:00 -0000 [thread overview]
Message-ID: <4C0407E4.3020604@redhat.com> (raw)
In-Reply-To: <m31vcxf3oi.fsf@fleche.redhat.com>
[-- Attachment #1: Type: text/plain, Size: 4375 bytes --]
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
>
[-- Attachment #2: pr10659.patch --]
[-- Type: text/x-patch, Size: 13490 bytes --]
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 ()
next prev parent reply other threads:[~2010-05-31 19:03 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-27 17:46 [patch] pr10659 pretty-printing: Display vectors of vectors as matrix Chris Moller
2010-05-27 21:14 ` Phil Muldoon
2010-05-27 22:25 ` Chris Moller
2010-05-27 21:20 ` Tom Tromey
2010-05-31 23:21 ` Chris Moller [this message]
2010-06-02 22:58 ` [patch] pr10659 pretty-printing: Display vectors of vectors as matrix--rev 2 Tom Tromey
2010-06-30 12:26 ` Jan Kratochvil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4C0407E4.3020604@redhat.com \
--to=cmoller@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=tromey@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).