From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21669 invoked by alias); 27 May 2010 21:03:35 -0000 Received: (qmail 21556 invoked by uid 22791); 27 May 2010 21:03:34 -0000 X-SWARE-Spam-Status: No, hits=-5.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,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; Thu, 27 May 2010 21:03:30 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o4RL3Sm7022229 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 27 May 2010 17:03:28 -0400 Received: from localhost.localdomain (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o4RL3R71028325; Thu, 27 May 2010 17:03:27 -0400 Message-ID: <4BFEDE1E.2080008@redhat.com> Date: Thu, 27 May 2010 21:14:00 -0000 From: Phil Muldoon User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100430 Fedora/3.0.4-2.fc12 Lightning/1.0b2pre Thunderbird/3.0.4 MIME-Version: 1.0 To: Chris Moller CC: "gdb-patches@sourceware.org" Subject: Re: [patch] pr10659 pretty-printing: Display vectors of vectors as matrix References: <4BFEA148.7000409@redhat.com> In-Reply-To: <4BFEA148.7000409@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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/msg00664.txt.bz2 On 05/27/2010 05:43 PM, Chris Moller wrote: > This patch adds the mechanism for gdb to respond to "matrix" hints from > printers.py. (Just to exercise the testcase, the patch includes a > patched version of printers.py to src/gdb/testsuite/gdb.python. It can > be removed once the patched printers.py is generally available.) This is not an official review, I just have some questions. > @@ -397,7 +397,7 @@ > struct ui_file *stream, int recurse, > const struct value_print_options *options, > const struct language_defn *language, > - int is_py_none) > + int is_py_none, int is_matrix) > { I'm not sure why it is necessary to pass the hint in here. All of the other hints are parsed internally within the function? > - if (is_py_none) > - fputs_filtered ("{", stream); > - else > - fputs_filtered (" = {", stream); > + if (is_matrix && recurse == 0) > + print_spaces_filtered (2 + 2 * recurse, stream); > + if (is_py_none) > + { > + if (is_matrix && strcmp (hint, "array")) > + { I looked at this, and I'm not sure what this means. Either a hint is a matrix or an array? Is this trying to deal with children? I looked at the modified vector printer and it can return either "array" or "matrix" depending on the template argument. > +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 {.*" > +gdb_test "p test3" "= $nl {$nl {$nl {.*" > + I cannot deduce from this test what the actual content of a vector printed with the matrix hint should look like beyond the expected bracket structure. To be really picky, I'd at least like to see an example of the output in comments or a more comprehensive test. I've really gotten into the idea that tests should also serve as an example of usage somehow. > Index: testsuite/gdb.python/printers.py > =================================================================== > RCS file: testsuite/gdb.python/printers.py > diff -N testsuite/gdb.python/printers.py This file cannot exist in the GDB testsuite. It is already hosted in GCC. You should post any changes to the vector printer there. For your testcase, I would write the test so it does not rely on this file. Cheers, Phil