Re: [patch] PR 12999 info macros command.

[Matt Rice <ratmice at gmail dot com>]

On Tue, Jul 19, 2011 at 10:16 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "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.

Thanks for reviewing, updated.

> 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.

I used 'current' instead of 'indicated'. because the command does not
have an argument use to indicate,
but a 'current state'.

> 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.

Ahh, yes these also needed a 'falling rocks'.  warning.

> 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.

K, I updated the whole test like that, and also fixed some issues with it.

> 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.

Yeah, and if you are going to explicitly give it as a linespec, you
are probably staring right at the macro definition to get its line
number.  So its kind of an awkward corner case.

Attachment: foo.diff
Description: Binary data