Re: [patch] PR 12999 info macros command.

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Matt" == Matt Rice <ratmice@gmail.com> 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