From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5062 invoked by alias); 27 May 2010 21:14:14 -0000 Received: (qmail 5053 invoked by uid 22791); 27 May 2010 21:14:13 -0000 X-SWARE-Spam-Status: No, hits=-5.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_QC,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:14:09 +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 o4RLE7nj019181 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 27 May 2010 17:14:07 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o4RLE6wr030498; Thu, 27 May 2010 17:14:07 -0400 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id o4RLE6P5000571; Thu, 27 May 2010 17:14:06 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id B1EEA3780A5; Thu, 27 May 2010 15:14:05 -0600 (MDT) From: Tom Tromey 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> Reply-To: tromey@redhat.com Date: Thu, 27 May 2010 21:20:00 -0000 In-Reply-To: <4BFEA148.7000409@redhat.com> (Chris Moller's message of "Thu, 27 May 2010 12:43:52 -0400") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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/msg00665.txt.bz2 >>>>> "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. 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. 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. 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. 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. 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? 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.) Chris> RCS file: testsuite/gdb.python/printers.py Chris> diff -N testsuite/gdb.python/printers.py [...] Chris> +print "in printers.py" Debugging code. 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 Tom