From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5604 invoked by alias); 19 Jul 2011 17:17:09 -0000 Received: (qmail 5590 invoked by uid 22791); 19 Jul 2011 17:17:07 -0000 X-SWARE-Spam-Status: No, hits=-7.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS 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; Tue, 19 Jul 2011 17:16:49 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p6JHGl3I017607 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 19 Jul 2011 13:16:47 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p6JHGkFh004989; Tue, 19 Jul 2011 13:16:46 -0400 Received: from barimba (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 p6JHGjcv007678; Tue, 19 Jul 2011 13:16:45 -0400 From: Tom Tromey To: Matt Rice Cc: gdb-patches@sourceware.org Subject: Re: [patch] PR 12999 info macros command. References: Date: Tue, 19 Jul 2011 17:29:00 -0000 In-Reply-To: (Matt Rice's message of "Mon, 18 Jul 2011 23:57:25 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain 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: 2011-07/txt/msg00481.txt.bz2 >>>>> "Matt" == Matt Rice writes: Matt> this patch adds 2 commands, Matt> info macros LINESPEC Matt> info definitions MACRO Matt> the PR only asks for the former, but the latter seemed possibly useful. Thanks, this is very nice. Matt> +@kindex info definitions Matt> +@cindex macro definition, showing Matt> +@cindex definition, showing a macro's Matt> +@item info definitions @var{macro} Matt> +Show all definitions of the macro named @var{macro}, in the current scope, Matt> +and describe the source location or compiler command-line where those Matt> +definitions were established. I didn't understand what this did from this explanation. I got confused because "in the current scope" would seem to restrict us to a single definition. I would reword it to say something about "all definitions of MACRO at or before the indicated line in the indicated source file", or something to that effect. Matt> +print_macro_definition (const char *name, Matt> + const struct macro_definition *d, Matt> + struct macro_source_file *file, Matt> + int line) Add an introductory comment. Matt> +static void Matt> +print_macro_callback (const char *name, const struct macro_definition *macro, Matt> + struct macro_source_file *source, int line, Matt> + void *user_data) Likewise. Matt> + add_cmd ("macros", no_class, info_macros_command, Matt> + _("Show the names of all macros at LINESPEC, or the current \ Matt> +scope."), Matt> + &infolist); Matt> + Matt> + add_cmd ("definitions", no_class, info_definitions_command, Matt> + _("Show all definitions of the macro named MACRO in the\n\ Matt> +current scope."), Matt> + &infolist); I think the "definitions" one shouldn't have a \n in the help string. The first sentence of the help is special, it should be on one line. Also the text for "definitions" needs updating, following the docs. I'd like it if the help for all new commands included a usage line, e.g.: Usage: info macros [LINESPEC] Matt> --- a/gdb/macrotab.h Matt> +++ b/gdb/macrotab.h Matt> @@ -311,6 +311,8 @@ struct macro_source_file *(macro_definition_location Matt> or macro_for_each_in_scope. */ Matt> typedef void (*macro_callback_fn) (const char *name, Matt> const struct macro_definition *definition, Matt> + struct macro_source_file *source, Matt> + int line, Matt> void *user_data); The comment just above this typedef needs an update. Matt> +if [test_compiler_info gcc*] { Matt> + lappend options additional_flags=-g3 Matt> +} else { Matt> + return -1 Matt> +} I think this should probably call 'untested' first. Matt> +set test "info definitions FOO" Matt> +gdb_test "$test" ".*#define FOO \"hello\".*" "info definitions FOO 1" Matt> +gdb_test "$test" ".*#define FOO \" \".*" "info definitions FOO 2" Matt> +gdb_test "$test" ".*#define FOO \"world\".*" "info definitions FOO 3" Matt> +gdb_test "$test" ".*#define FOO\\(a\\) foo = a.*" "info definitions FOO 4" As you mentioned on irc, this is weird. How about just a single test with one big regexp that matches all the output? You can easily construct the regexp in pieces if that makes it simpler. Matt> +# info macros on the line where the #define or #include is Matt> +# fails to find the macro defined (though it works on the next line.) Matt> +setup_kfail "gdb/NNNN" *-*-* Matt> +gdb_test "$test" ".*define ONE.*" "$test.2" I suppose technically the macro is not actually defined until the next line. I don't think it matters much though, since the #define line will never be executable anyhow. Tom