From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20462 invoked by alias); 31 May 2010 19:03:12 -0000 Received: (qmail 19926 invoked by uid 22791); 31 May 2010 19:03:09 -0000 X-SWARE-Spam-Status: No, hits=-5.6 required=5.0 tests=AWL,BAYES_05,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_BJ,TW_CP,TW_QC,TW_SV,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 31 May 2010 19:03:03 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o4VJ31vn004386 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 31 May 2010 15:03:01 -0400 Received: from qcore.mollernet.net (vpn-227-10.phx2.redhat.com [10.3.227.10]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o4VJ30Ew026446; Mon, 31 May 2010 15:03:00 -0400 Message-ID: <4C0407E4.3020604@redhat.com> Date: Mon, 31 May 2010 23:21:00 -0000 From: Chris Moller User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3pre) Gecko/20090513 Fedora/3.0-2.3.beta2.fc11 Thunderbird/3.0b2 MIME-Version: 1.0 To: tromey@redhat.com CC: "gdb-patches@sourceware.org" Subject: Re: [patch] pr10659 pretty-printing: Display vectors of vectors as matrix--rev 2 References: <4BFEA148.7000409@redhat.com> In-Reply-To: Content-Type: multipart/mixed; boundary="------------010207090707090000010901" X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-05/txt/msg00698.txt.bz2 This is a multi-part message in MIME format. --------------010207090707090000010901 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 4375 On 05/27/10 17:14, Tom Tromey wrote: >>>>>> "Chris" == Chris Moller 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 > --------------010207090707090000010901 Content-Type: text/x-patch; name="pr10659.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="pr10659.patch" Content-length: 13490 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 + + * 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 * 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 + + * 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 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 +#include // /usr/include/c++/4.4.1/bits/vector.tcc +#include + +using namespace std; + +int use_windows = 9999; + +int +main(){ + vector test1(2,0); + test1[0]=8; + test1[1]=9; + + vector< vector > test2(3, vector(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 rows(NR_ROWS, 0); + vector< vector > columns(NR_COLS, rows); + vector< vector < vector > > 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 . + +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 . + +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 () --------------010207090707090000010901--