RFA: info macro [-at LOCATION,]

RFA: info macro [-at LOCATION,]

[David Taylor]

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.

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:

Index: gdb/macrocmd.c
===================================================================
RCS file: /home/cvsroot/GDB/gdb/macrocmd.c,v
retrieving revision 1.1.1.3
diff -u -r1.1.1.3 macrocmd.c
--- gdb/macrocmd.c	21 Aug 2014 13:17:23 -0000	1.1.1.3
+++ gdb/macrocmd.c	10 Nov 2014 21:23:37 -0000
@@ -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);
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /home/cvsroot/GDB/gdb/doc/gdb.texinfo,v
retrieving revision 1.3
diff -u -r1.3 gdb.texinfo
--- gdb/doc/gdb.texinfo	21 Aug 2014 14:07:17 -0000	1.3
+++ gdb/doc/gdb.texinfo	10 Nov 2014 21:23:37 -0000
@@ -11412,12 +11412,13 @@
 @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.
+the macro may begin with a hyphen.  If the optional LOCATION is specified,
+it is used instead of the current location.
 
 @kindex info macros
 @item info macros @var{linespec}
Index: gdb/testsuite/gdb.base/info-macros.exp
===================================================================
RCS file: /home/cvsroot/GDB/gdb/testsuite/gdb.base/info-macros.exp,v
retrieving revision 1.1.1.3
diff -u -r1.1.1.3 info-macros.exp
--- gdb/testsuite/gdb.base/info-macros.exp	21 Aug 2014 13:17:27 -0000	1.1.1.3
+++ gdb/testsuite/gdb.base/info-macros.exp	10 Nov 2014 21:23:38 -0000
@@ -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"

Re: RFA: info macro [-at LOCATION,]

[Eli Zaretskii]

> From: David Taylor <dtaylor@emc.com>
> Date: Mon, 10 Nov 2014 16:41:33 -0500
> 
> 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:

Thanks.  I think this warrants a NEWS entry as well.

> gdb/doc/ChangeLog:
> 
> 2014-11-10  David Taylor  <dtaylor@emc.com>
> 
> 	* gdb.texinfo: Document new -at LOCATION option of the 'info
> 	macro' command.

Please state the node in which the change was made (as if it were a
function, i.e. in parentheses).

> -@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.
> +the macro may begin with a hyphen.  If the optional LOCATION is specified,
> +it is used instead of the current location.

First, "LOCATION" should be "@var{location}", as it is a parameter,
not a literal string.

And second, "it is used instead of the current location" leaves too
much unsaid.  The reader will wonder why does location matter for this
purpose.  I suggest to make that explicit in the text.

Re: RFA: info macro [-at LOCATION,]

[David Taylor]

Eli Zaretskii <eliz@gnu.org> wrote:

> > From: David Taylor <dtaylor@emc.com>
> > Date: Mon, 10 Nov 2014 16:41:33 -0500
> > 
> > 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:
> 
> Thanks.  I think this warrants a NEWS entry as well.

Thanks.  How are diffs for NEWS handled?  For the text, how about:

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

> > gdb/doc/ChangeLog:
> > 
> > 2014-11-10  David Taylor  <dtaylor@emc.com>
> > 
> > 	* gdb.texinfo: Document new -at LOCATION option of the 'info
> > 	macro' command.
> 
> Please state the node in which the change was made (as if it were a
> function, i.e. in parentheses).

Okay, how about:

2014-11-10  David Taylor  <dtaylor@emc.com>

	* gdb.texinfo (Macros): Document new -at LOCATION option of
	the 'info macro' command.

> > -@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.
> > +the macro may begin with a hyphen.  If the optional LOCATION is specified,
> > +it is used instead of the current location.
> 
> First, "LOCATION" should be "@var{location}", as it is a parameter,
> not a literal string.

Easy enough.  Done.

> And second, "it is used instead of the current location" leaves too
> much unsaid.  The reader will wonder why does location matter for this
> purpose.  I suggest to make that explicit in the text.

I'm not sure how to word it nor exactly what you are looking for.
How about the following for the gdb/doc/gdb.texinfo piece:

Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /home/cvsroot/GDB/gdb/doc/gdb.texinfo,v
retrieving revision 1.3
diff -u -r1.3 gdb.texinfo
--- gdb/doc/gdb.texinfo	21 Aug 2014 14:07:17 -0000	1.3
+++ gdb/doc/gdb.texinfo	11 Nov 2014 13:53:09 -0000
@@ -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}

Thanks for reviewing the doc changes so quickly.

Re: RFA: info macro [-at LOCATION,]

[Eli Zaretskii]

> cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> Date: Tue, 11 Nov 2014 09:01:15 -0500
> From: David Taylor <dtaylor@usendtaylorx2l.lss.emc.com>
> 
> > Thanks.  I think this warrants a NEWS entry as well.
> 
> Thanks.  How are diffs for NEWS handled?

Like any other file you modify.

> For the text, how about:
> 
> * 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.

Fine with me, thanks.

> > > 	* gdb.texinfo: Document new -at LOCATION option of the 'info
> > > 	macro' command.
> > 
> > Please state the node in which the change was made (as if it were a
> > function, i.e. in parentheses).
> 
> Okay, how about:
> 
> 2014-11-10  David Taylor  <dtaylor@emc.com>
> 
> 	* gdb.texinfo (Macros): Document new -at LOCATION option of
> 	the 'info macro' command.

OK.

> > And second, "it is used instead of the current location" leaves too
> > much unsaid.  The reader will wonder why does location matter for this
> > purpose.  I suggest to make that explicit in the text.
> 
> I'm not sure how to word it nor exactly what you are looking for.
> How about the following for the gdb/doc/gdb.texinfo piece:

Sounds good to me.

> Thanks for reviewing the doc changes so quickly.

Thanks for working on this in the first place.

RFA info macro [-at LOCATION,] (v2)

[David Taylor]

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:



Index: gdb/NEWS
===================================================================
RCS file: /home/cvsroot/GDB/gdb/NEWS,v
retrieving revision 1.1.1.3
diff -u -r1.1.1.3 NEWS
--- gdb/NEWS	21 Aug 2014 13:17:24 -0000	1.1.1.3
+++ gdb/NEWS	19 Nov 2014 20:51:59 -0000
@@ -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
Index: gdb/macrocmd.c
===================================================================
RCS file: /home/cvsroot/GDB/gdb/macrocmd.c,v
retrieving revision 1.1.1.3
diff -u -r1.1.1.3 macrocmd.c
--- gdb/macrocmd.c	21 Aug 2014 13:17:23 -0000	1.1.1.3
+++ gdb/macrocmd.c	19 Nov 2014 20:51:59 -0000
@@ -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);
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /home/cvsroot/GDB/gdb/doc/gdb.texinfo,v
retrieving revision 1.3
diff -u -r1.3 gdb.texinfo
--- gdb/doc/gdb.texinfo	21 Aug 2014 14:07:17 -0000	1.3
+++ gdb/doc/gdb.texinfo	19 Nov 2014 20:52:00 -0000
@@ -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}
Index: gdb/testsuite/gdb.base/info-macros.exp
===================================================================
RCS file: /home/cvsroot/GDB/gdb/testsuite/gdb.base/info-macros.exp,v
retrieving revision 1.1.1.3
diff -u -r1.1.1.3 info-macros.exp
--- gdb/testsuite/gdb.base/info-macros.exp	21 Aug 2014 13:17:27 -0000	1.1.1.3
+++ gdb/testsuite/gdb.base/info-macros.exp	19 Nov 2014 20:52:03 -0000
@@ -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"

Re: RFA info macro [-at LOCATION,] (v2)

[Eli Zaretskii]

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

Re: RFA info macro [-at LOCATION,] (v2)

[Doug Evans]

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?

Re: Fwd: Delivery Status Notification (Failure)

[David Taylor]

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

Re: Fwd: Delivery Status Notification (Failure)

[Doug Evans]

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

Re: Fwd: Delivery Status Notification (Failure)

[Doug Evans]

On Sun, Nov 23, 2014 at 11:18 AM, Doug Evans <xdje42@gmail.com> wrote:
> [+ 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.

Re: Plus I think we need to document in some linespec-related
place that "," is now required to be recognized as ending a linespec.

FAOD, I was thinking of, say, adding a comment to the
function-comment for decode_line_full in linespec.h.

E.g.,

--- linespec.h= 2014-11-23 10:30:23.566784217 -0800
+++ linespec.h  2014-11-23 11:55:35.531659067 -0800
@@ -133,7 +133,11 @@ extern struct symtabs_and_lines
    entry describing all the matching locations.  If FILTER is
    non-NULL, then only locations whose canonical name is equal (in the
    strcmp sense) to FILTER will be returned; all others will be
-   filtered out.  */
+   filtered out.
+
+   Note for new callers: If you need a way to mark the end of the linespec
+   in *ARGPTR, fyi the "info macro -at LOCATION, MACRO" command uses a comma.
+   A comma within the linespec is still ok (e.g., "foo (int, int)").  */

 extern void decode_line_full (char **argptr, int flags,
                              struct symtab *default_symtab, int default_line,

or some such.
The point being to make the comma terminator
being part of the spec of what's a valid linespec.


---
IWBN to "fix" the linespec parsing machinery to recognize
the end of the line spec so that the comma wasn't required.
I don't want to make that a prerequisite for this patch,
but down the road if that happens then the comma
could just become optional.