RFA info macro [-at LOCATION,] (v2)
Commit Message
This adds a NEWS entry as requested by Eli and I believe addresses his
concerns with regard to gdb/doc/gdb.texinfo. The other two files
gdb/macrocmd.c and gdb/testsuite/gdb.base/info-macros.exp are unchanged
from the previous submission. I updated the gdb/ChangeLog entry to
include the gdb/NEWS addition..
A few releases ago it was possible to set a location that would be used
by 'info location' by typing something like:
list file.c:42
and then do:
info macro MACRONAME
. Sometime between release 7.1 and 7.8 that stopped working.
The following patch adds the option [-at LOCATION,] to 'info macro' to
enable the use of a user selected location as a documented feature.
Three files, three change log entries:
gdb/ChangeLog:
2014-11-10 David Taylor <dtaylor@emc.com>
* macrocmd.c (info_macro_command): Add support for new option '-at
LOCATION'. (_initialize_macrocmd): Update doc string for 'info
macro' command.
* NEWS: Add entry about --reverse option to info macro command.
gdb/doc/ChangeLog:
2014-11-10 David Taylor <dtaylor@emc.com>
* gdb.texinfo: Document new -at LOCATION option of the 'info
macro' command.
gdb/testsuite/ChangeLog:
2014-11-10 David Taylor <dtaylor@emc.com>
* gdb.base/info-macros.exp: Add testcases for new -at LOCATION
option of the 'info macro' command.
On an x86-64 Linux platform, there were no testsuite regressions and the
new tests passed.
The actual patch:
Comments
> cc: Eli Zaretskii <eliz@gnu.org>
> Date: Wed, 19 Nov 2014 16:00:26 -0500
> From: David Taylor <dtaylor@usendtaylorx2l.lss.emc.com>
>
> This adds a NEWS entry as requested by Eli and I believe addresses his
> concerns with regard to gdb/doc/gdb.texinfo. The other two files
> gdb/macrocmd.c and gdb/testsuite/gdb.base/info-macros.exp are unchanged
> from the previous submission. I updated the gdb/ChangeLog entry to
> include the gdb/NEWS addition..
OK for the documentation parts, with this one gotcha:
> 2014-11-10 David Taylor <dtaylor@emc.com>
>
> * gdb.texinfo: Document new -at LOCATION option of the 'info
> macro' command.
This log entry should name the node in which you made the change.
Thanks.
On Wed, Nov 19, 2014 at 1:00 PM, David Taylor
<dtaylor@usendtaylorx2l.lss.emc.com> wrote:
> This adds a NEWS entry as requested by Eli and I believe addresses his
> concerns with regard to gdb/doc/gdb.texinfo. The other two files
> gdb/macrocmd.c and gdb/testsuite/gdb.base/info-macros.exp are unchanged
> from the previous submission. I updated the gdb/ChangeLog entry to
> include the gdb/NEWS addition..
>
> A few releases ago it was possible to set a location that would be used
> by 'info location' by typing something like:
>
> list file.c:42
>
> and then do:
>
> info macro MACRONAME
>
> . Sometime between release 7.1 and 7.8 that stopped working.
>
> The following patch adds the option [-at LOCATION,] to 'info macro' to
> enable the use of a user selected location as a documented feature.
Hi.
The "," in "-at LOCATION," seems a bit random, relative to other commands.
Maybe it is the best way to go.
If so, I'd like to see the reasons why it exists documented in the code.
Can we just remove the , and require -- when necessary?
Doug Evans <xdje42@gmail.com> wrote:
> Hi.
> This didn't go through for some strange reason.
> I'm hoping this does.
Bizarre.
> Delivery to the following recipient failed permanently:
>
> dtaylor@usendtaylorx2l.lss.emc.com
>
> Technical details of permanent failure:
> DNS Error: Address resolution of usendtaylorx2l.lss.emc.com. failed:
> Domain name not found
Groan. IT must have changed something. usendtaylorx2l is my desktop.
Outgoing mail should show an address of either dtaylor@emc.com or
david.taylor@emc.com.
> Subject: Re: RFA info macro [-at LOCATION,] (v2)
> From: Doug Evans <xdje42@gmail.com>
>
> On Wed, Nov 19, 2014 at 1:00 PM, David Taylor
> <dtaylor@usendtaylorx2l.lss.emc.com> wrote:
> > The following patch adds the option [-at LOCATION,] to 'info macro' to
> > enable the use of a user selected location as a documented feature.
>
> Hi.
Hello.
> The "," in "-at LOCATION," seems a bit random, relative to other commands.
>
> Maybe it is the best way to go.
> If so, I'd like to see the reasons why it exists documented in the code.
>
> Can we just remove the , and require -- when necessary?
It marks the end of the location and the start of the macro. It is
patterned after
maint agent [-at location,] EXPRESSION
and
maint agent-eval [-at location,] EXPRESSION
I did it the same way for consistency.
It seems unnecessary, but the location parsing code stops at comma but
not at space. Presumably that is so that file names can contain spaces.
But, that is just a guess on my part. I personally dislike file names
containing spaces.
David
[+ keiths, since this is linespec related, and I know
how much he loves linespecs! :-)]
On Thu, Nov 20, 2014 at 6:57 AM, David Taylor
<dtaylor@usendtaylorx2l.lss.emc.com> wrote:
> Doug Evans <xdje42@gmail.com> wrote:
>> The "," in "-at LOCATION," seems a bit random, relative to other commands.
>>
>> Maybe it is the best way to go.
>> If so, I'd like to see the reasons why it exists documented in the code.
>>
>> Can we just remove the , and require -- when necessary?
>
> It marks the end of the location and the start of the macro. It is
> patterned after
>
> maint agent [-at location,] EXPRESSION
> and
> maint agent-eval [-at location,] EXPRESSION
>
> I did it the same way for consistency.
Ah. Can't fault that. :-)
Still, I'm more ok with u/i visible hacks in maint commands
than with normal commands.
Bubbling this up into non-maint commands means needing
to now worry about consistency with all the other commands.
> It seems unnecessary, but the location parsing code stops at comma but
> not at space. Presumably that is so that file names can contain spaces.
> But, that is just a guess on my part. I personally dislike file names
> containing spaces.
Such names need to be quoted.
OTOH c++ functions with parameters, for example,
don't (usually) need to be quoted.
linespec.c has this:
static const char * const linespec_keywords[] = { "if", "thread", "task" };
This may not be the best solution, and it might involve a few more
changes to linespec.c (or elsewhere) but that's one alternative:
static const char * const linespec_keywords[] = { "if", "thread",
"task", "--" };
It works, but "--" wasn't intended for this purpose (mark the
end of the linespec), so it's just a not-well-thought-out-idea.
I tried to think of a situation where using a comma here would
lead to u/i warts later, but couldn't.
E.g., if the comma gets used in other contexts
then will users start complaining that "b foo, thread 1" doesn't work?
If that day comes we *could* allow the comma in that context.
But it would be odd that a comma is *required* in some
contexts and not in others.
In the end, I'm ok with the patch, but
I think the docs (both offline and online) need to highlight the comma
as being a requirement for "-at" since it's not intuitive.
Plus I think we need to document in some linespec-related
place that "," is now required to be recognized as ending a linespec.
I don't know if that exists today.
[FAOD, a "," within a linespec is still ok,
e.g., "info macro -at foo(int, int), BAR"]
Ok with those changes.
===================================================================
RCS file: /home/cvsroot/GDB/gdb/NEWS,v
retrieving revision 1.1.1.3
@@ -1,6 +1,15 @@
What has changed in GDB?
(Organized release by release)
+*** Changes since GDB 7.8
+
+* New options
+
+The info macro command now takes an optional location ([-at LOCATION,])
+for determining which definition, if any, of the macro is in scope. If
+left unspecified it, as before, uses the source and line associated with
+the current program counter.
+
*** Changes in GDB 7.8
* New command line options
===================================================================
RCS file: /home/cvsroot/GDB/gdb/macrocmd.c,v
retrieving revision 1.1.1.3
@@ -206,21 +206,43 @@
info_macro_command (char *args, int from_tty)
{
struct macro_scope *ms = NULL;
- struct cleanup *cleanup_chain;
+ struct cleanup *cleanup_chain = NULL;
char *name;
int show_all_macros_named = 0;
char *arg_start = args;
int processing_args = 1;
- while (processing_args
- && arg_start && *arg_start == '-' && *arg_start != '\0')
- {
- char *p = skip_to_space (arg_start);
+ int location_specified = 0;
- if (strncmp (arg_start, "-a", p - arg_start) == 0
- || strncmp (arg_start, "-all", p - arg_start) == 0)
+ while (processing_args && args && *args == '-')
+ {
+ if (check_for_argument (&args, "-a", sizeof ("-a") - 1)
+ || check_for_argument (&args, "-all", sizeof ("-all") - 1))
show_all_macros_named = 1;
- else if (strncmp (arg_start, "--", p - arg_start) == 0)
+ else if (check_for_argument (&args, "-at", sizeof ("-at") - 1))
+ {
+ struct linespec_result canonical;
+
+ location_specified = 1;
+
+ args = skip_spaces (args);
+ init_linespec_result (&canonical);
+ decode_line_full (&args, DECODE_LINE_FUNFIRSTLINE,
+ (struct symtab *) NULL, 0, &canonical,
+ NULL, NULL);
+
+ cleanup_chain = make_cleanup_destroy_linespec_result (&canonical);
+ args = skip_spaces (args);
+ if (args[0] == ',')
+ {
+ args++;
+ args = skip_spaces (args);
+ }
+ ms = sal_macro_scope (canonical.sals->vec->sals.sals[0]);
+ do_cleanups (cleanup_chain);
+ cleanup_chain = make_cleanup (free_current_contents, &ms);
+ }
+ else if (check_for_argument (&args, "--", sizeof ("--") - 1))
/* Our macro support seems rather C specific but this would
seem necessary for languages allowing - in macro names.
e.g. Scheme's (defmacro ->foo () "bar\n") */
@@ -228,24 +250,26 @@
else
{
/* Relies on modified 'args' not making it in to history */
- *p = '\0';
error (_("Unrecognized option '%s' to info macro command. "
- "Try \"help info macro\"."), arg_start);
+ "Try \"help info macro\"."), args);
}
- arg_start = skip_spaces (p);
+ args = skip_spaces (args);
}
- name = arg_start;
+ name = skip_spaces (args);
if (! name || ! *name)
error (_("You must follow the `info macro' command with the name"
" of the macro\n"
"whose definition you want to see."));
- ms = default_macro_scope ();
- cleanup_chain = make_cleanup (free_current_contents, &ms);
-
+ if (! location_specified)
+ {
+ ms = default_macro_scope ();
+ cleanup_chain = make_cleanup (free_current_contents, &ms);
+ }
+
if (! ms)
macro_inform_no_debuginfo ();
else if (show_all_macros_named)
@@ -525,10 +549,11 @@
add_cmd ("macro", no_class, info_macro_command,
_("Show the definition of MACRO, and it's source location.\n\
-Usage: info macro [-a|-all] [--] MACRO\n\
+Usage: info macro [-a|-all] [-at LOCATION,] [--] MACRO\n\
Options: \n\
-a, --all Output all definitions of MACRO in the current compilation\
unit.\n\
+ -at Use LOCATION rather than the current PC for selecting macros.\n\
-- Specify the end of arguments and the beginning of the MACRO."),
&infolist);
===================================================================
RCS file: /home/cvsroot/GDB/gdb/doc/gdb.texinfo,v
retrieving revision 1.3
@@ -11412,12 +11412,15 @@
@cindex macro definition, showing
@cindex definition of a macro, showing
@cindex macros, from debug info
-@item info macro [-a|-all] [--] @var{macro}
+@item info macro [-a|-all] [-at LOCATION,] [--] @var{macro}
Show the current definition or all definitions of the named @var{macro},
and describe the source location or compiler command-line where that
-definition was established. The optional double dash is to signify the end of
-argument processing and the beginning of @var{macro} for non C-like macros where
-the macro may begin with a hyphen.
+definition was established. The optional double dash is to signify the
+end of argument processing and the beginning of @var{macro} for non
+C-like macros where the macro may begin with a hyphen. If the optional
+@var{location} is specified, it is used to determine which definition,
+if any, of the macro is in scope; otherwise, as before, it uses the
+source and line asociated with the current program counter.
@kindex info macros
@item info macros @var{linespec}
===================================================================
RCS file: /home/cvsroot/GDB/gdb/testsuite/gdb.base/info-macros.exp,v
retrieving revision 1.1.1.3
@@ -133,6 +133,30 @@
set testname "$test"
gdb_test "$test" "$r1$r2$r3$r4" "$testname"
+set test "info macro -at LOCATION, FOO"
+
+set r5 ".* has no definition .*\r\nat .*$srcfile:\[0-9\]+"
+set testname "$test 1"
+gdb_test "info macro -at info-macros.c:42, FOO" "$r1" "$testname"
+
+set testname "$test 2"
+gdb_test "info macro -at info-macros.c:46, FOO" "$r2" "$testname"
+
+set testname "$test 3"
+gdb_test "info macro -at info-macros.c:50, FOO" "$r3" "$testname"
+
+set testname "$test 4"
+gdb_test "info macro -at info-macros.c:54, FOO" "$r2" "$testname"
+
+set testname "$test 5"
+gdb_test "info macro -at info-macros.c:58, FOO" "$r1" "$testname"
+
+set testname "$test 6"
+gdb_test "info macro -at info-macros.c:62, FOO" "$r5" "$testname"
+
+set testname "$test 7"
+gdb_test "info macro -at info-macros.c:66, FOO" "$r4" "$testname"
+
set test "info macros"
set r1 ".*#define FOO \"hello\""
set r2 ".*#define ONE"