[RFA,09/10] Ensure class_alias is only used for user-defined aliases.

Message ID 20200510205530.21923-10-philippe.waroquiers@skynet.be
State Superseded
Headers show
Series
  • fix/improve cmd structure, class_alias, help, apropos
Related show

Commit Message

H.J. Lu via Gdb-patches May 10, 2020, 8:55 p.m.
This commit finally does the (small) change that started this patch
series.

It ensures that the class_alias is only used for user-defined aliases.
So, the few GDB pre-defined aliases that were using the 'class_alias'
class are now using a real help class, typically the class of
the aliased command.

gdb/ChangeLog

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* command.h (enum command_class): Improve comments, document
	that class_alias is for user-defined aliases, give the class
	name for each class, remove unused class_xdb.
	* cli/cli-decode.c (add_com_alias): Document THECLASS intended usage.
	* breakpoint.c (_initialize_breakpoint): Replace class_alias
	by a precise class.
	* infcmd.c (_initialize_infcmd): Likewise.
	* reverse.c (_initialize_reverse): Likewise.
	* stack.c (_initialize_stack): Likewise.
	* symfile.c (_initialize_symfile): Likewise.
	* tracepoint.c (_initialize_tracepoint): Likewise.

gdb/testsuite/ChangeLog

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/alias.exp: Verify 'help aliases' shows user defined aliases.
---
 gdb/breakpoint.c                 | 12 ++++----
 gdb/cli/cli-decode.c             |  5 +++-
 gdb/command.h                    | 47 ++++++++++++++++++++++++--------
 gdb/infcmd.c                     |  6 ++--
 gdb/reverse.c                    | 10 +++----
 gdb/stack.c                      |  2 +-
 gdb/symfile.c                    |  4 +--
 gdb/testsuite/gdb.base/alias.exp |  3 ++
 gdb/tracepoint.c                 |  4 +--
 9 files changed, 62 insertions(+), 31 deletions(-)

-- 
2.20.1

Comments

Tom Tromey May 14, 2020, 5:33 p.m. | #1
>>>>> "Philippe" == Philippe Waroquiers via Gdb-patches <gdb-patches@sourceware.org> writes:


Philippe> This commit finally does the (small) change that started this patch
Philippe> series.

Isn't that always the way.

Philippe> YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

Philippe> 	* command.h (enum command_class): Improve comments, document
Philippe> 	that class_alias is for user-defined aliases, give the class
Philippe> 	name for each class, remove unused class_xdb.
Philippe> 	* cli/cli-decode.c (add_com_alias): Document THECLASS intended usage.
Philippe> 	* breakpoint.c (_initialize_breakpoint): Replace class_alias
Philippe> 	by a precise class.
Philippe> 	* infcmd.c (_initialize_infcmd): Likewise.
Philippe> 	* reverse.c (_initialize_reverse): Likewise.
Philippe> 	* stack.c (_initialize_stack): Likewise.
Philippe> 	* symfile.c (_initialize_symfile): Likewise.
Philippe> 	* tracepoint.c (_initialize_tracepoint): Likewise.

Looks good.

Philippe> +  add_com_alias ("tp", "trace", class_breakpoint, 0);
Philippe> +  add_com_alias ("tr", "trace", class_breakpoint, 1);
Philippe> +  add_com_alias ("tra", "trace", class_breakpoint, 1);
Philippe> +  add_com_alias ("trac", "trace", class_breakpoint, 1);

I wonder if there's ever a case where we want an alias to have a
different class from the thing it aliases.  If not, maybe add_com_alias
could just do this and we could remove the parameter.

Philippe>  enum command_class
Philippe>  {

Thanks for this.

This is ok.

Tom
H.J. Lu via Gdb-patches May 14, 2020, 9:12 p.m. | #2
On Thu, 2020-05-14 at 11:33 -0600, Tom Tromey wrote:
> Philippe> +  add_com_alias ("tp", "trace", class_breakpoint, 0);

> Philippe> +  add_com_alias ("tr", "trace", class_breakpoint, 1);

> Philippe> +  add_com_alias ("tra", "trace", class_breakpoint, 1);

> Philippe> +  add_com_alias ("trac", "trace", class_breakpoint, 1);

> 

> I wonder if there's ever a case where we want an alias to have a

> different class from the thing it aliases.  If not, maybe add_com_alias

> could just do this and we could remove the parameter.

I also wondered but decided to not do this, at least for the moment.

The advantage of allowing another class for an alias and for its aliased
command is that this can be used to 'market' the same GDB functionality
via different classes.
 
Now that the help shows the command and all its aliases, that might
help some users to find a certain command via different classes,
in case such a command could reasonably be related to more than
one class.

Thanks for the review, I have handled all your comments and the comments
of Eli about the documentation, and will send an RFAv2 soon.

Thanks

Philippe

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 22ddb3d5e8..9f237cee1f 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -15478,7 +15478,7 @@  A disabled breakpoint is not forgotten, but has no effect until re-enabled."),
   add_com_alias ("dis", "disable", class_breakpoint, 1);
   add_com_alias ("disa", "disable", class_breakpoint, 1);
 
-  add_cmd ("breakpoints", class_alias, disable_command, _("\
+  add_cmd ("breakpoints", class_breakpoint, disable_command, _("\
 Disable all or some breakpoints.\n\
 Usage: disable breakpoints [BREAKPOINTNUM]...\n\
 Arguments are breakpoint numbers with spaces in between.\n\
@@ -15498,7 +15498,7 @@  Also a prefix command for deletion of other GDB objects."),
   add_com_alias ("d", "delete", class_breakpoint, 1);
   add_com_alias ("del", "delete", class_breakpoint, 1);
 
-  add_cmd ("breakpoints", class_alias, delete_command, _("\
+  add_cmd ("breakpoints", class_breakpoint, delete_command, _("\
 Delete all or some breakpoints or auto-display expressions.\n\
 Usage: delete breakpoints [BREAKPOINTNUM]...\n\
 Arguments are breakpoint numbers with spaces in between.\n\
@@ -15686,10 +15686,10 @@  BREAK_ARGS_HELP ("trace") "\n\
 Do \"help tracepoints\" for info on other tracepoint commands."));
   set_cmd_completer (c, location_completer);
 
-  add_com_alias ("tp", "trace", class_alias, 0);
-  add_com_alias ("tr", "trace", class_alias, 1);
-  add_com_alias ("tra", "trace", class_alias, 1);
-  add_com_alias ("trac", "trace", class_alias, 1);
+  add_com_alias ("tp", "trace", class_breakpoint, 0);
+  add_com_alias ("tr", "trace", class_breakpoint, 1);
+  add_com_alias ("tra", "trace", class_breakpoint, 1);
+  add_com_alias ("trac", "trace", class_breakpoint, 1);
 
   c = add_com ("ftrace", class_breakpoint, ftrace_command, _("\
 Set a fast tracepoint at specified location.\n\
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 48187e8583..67758356cb 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1004,7 +1004,10 @@  add_com (const char *name, enum command_class theclass,
   return add_cmd (name, theclass, fun, doc, &cmdlist);
 }
 
-/* Add an alias or abbreviation command to the list of commands.  */
+/* Add an alias or abbreviation command to the list of commands.
+   For aliases predefined by GDB (such as bt), THECLASS must be
+   different of class_alias, as class_alias is used to identify
+   user defined aliases.  */
 
 struct cmd_list_element *
 add_com_alias (const char *name, const char *oldname, enum command_class theclass,
diff --git a/gdb/command.h b/gdb/command.h
index 0a1706c545..04a380cba4 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -29,21 +29,46 @@  struct completion_tracker;
 /* Command classes are top-level categories into which commands are
    broken down for "help" purposes.
 
-   Notes on classes: class_alias is for alias commands which are not
-   abbreviations of the original command.  class-pseudo is for
-   commands which are not really commands nor help topics ("stop").  */
+   The class_alias is used for the user-defined aliases, defined
+   using the "alias" command.
+
+   Aliases pre-defined by GDB (e.g. the alias "bt" of the "backtrace" command)
+   are not using the class_alias.
+   Different pre-defined aliases of the same command do not necessarily
+   have the same classes.  For example, class_stack is used for the
+   "backtrace" and its "bt" alias", while "info stack" (also an alias
+   of "backtrace" uses class_info.  */
 
 enum command_class
 {
-  /* Special args to help_list */
-  class_deprecated = -3, all_classes = -2, all_commands = -1,
+  /* Classes of commands followed by a comment giving the name
+     to use in "help <classname>".
+     Note that help accepts unambiguous abbreviated class names.  */
+
+  /* Special classes to help_list */
+  class_deprecated = -3,
+  all_classes = -2,  /* help without <classname> */
+  all_commands = -1, /* all */
+
   /* Classes of commands */
-  no_class = -1, class_run = 0, class_vars, class_stack, class_files,
-  class_support, class_info, class_breakpoint, class_trace,
-  class_alias, class_bookmark, class_obscure, class_maintenance,
-  class_tui, class_user, class_xdb,
-  no_set_class	/* Used for "show" commands that have no corresponding
-		   "set" command.  */
+  no_class = -1,
+  class_run = 0,     /* running */
+  class_vars,        /* data */
+  class_stack,       /* stack */
+  class_files,       /* files */
+  class_support,     /* support */
+  class_info,        /* status */
+  class_breakpoint,  /* breakpoints */
+  class_trace,       /* tracepoints */
+  class_alias,       /* aliases */
+  class_bookmark,
+  class_obscure,     /* obscure */
+  class_maintenance, /* internals */
+  class_tui,
+  class_user,        /* user-defined */
+
+  /* Used for "show" commands that have no corresponding "set" command.  */
+  no_set_class
 };
 
 /* FIXME: cagney/2002-03-17: Once cmd_type() has been removed, ``enum
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 9bbb413d4e..3e32e89d99 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -3180,7 +3180,7 @@  is restored."),
   cmd_name = "inferior-tty";
   c = lookup_cmd (&cmd_name, setlist, "", -1, 1);
   gdb_assert (c != NULL);
-  add_alias_cmd ("tty", c, class_alias, 0, &cmdlist);
+  add_alias_cmd ("tty", c, class_run, 0, &cmdlist);
 
   cmd_name = "args";
   add_setshow_string_noescape_cmd (cmd_name, class_run,
@@ -3318,14 +3318,14 @@  Step one instruction exactly.\n\
 Usage: stepi [N]\n\
 Argument N means step N times (or till program stops for another \
 reason)."));
-  add_com_alias ("si", "stepi", class_alias, 0);
+  add_com_alias ("si", "stepi", class_run, 0);
 
   add_com ("nexti", class_run, nexti_command, _("\
 Step one instruction, but proceed through subroutine calls.\n\
 Usage: nexti [N]\n\
 Argument N means step N times (or till program stops for another \
 reason)."));
-  add_com_alias ("ni", "nexti", class_alias, 0);
+  add_com_alias ("ni", "nexti", class_run, 0);
 
   add_com ("finish", class_run, finish_command, _("\
 Execute until selected stack frame returns.\n\
diff --git a/gdb/reverse.c b/gdb/reverse.c
index 1ccb9d2797..583e0d02da 100644
--- a/gdb/reverse.c
+++ b/gdb/reverse.c
@@ -330,7 +330,7 @@  _initialize_reverse ()
 Step program backward until it reaches the beginning of another source line.\n\
 Argument N means do this N times (or till program stops for another reason).")
 	   );
-  add_com_alias ("rs", "reverse-step", class_alias, 1);
+  add_com_alias ("rs", "reverse-step", class_run, 1);
 
   add_com ("reverse-next", class_run, reverse_next, _("\
 Step program backward, proceeding through subroutine calls.\n\
@@ -338,26 +338,26 @@  Like the \"reverse-step\" command as long as subroutine calls do not happen;\n\
 when they do, the call is treated as one instruction.\n\
 Argument N means do this N times (or till program stops for another reason).")
 	   );
-  add_com_alias ("rn", "reverse-next", class_alias, 1);
+  add_com_alias ("rn", "reverse-next", class_run, 1);
 
   add_com ("reverse-stepi", class_run, reverse_stepi, _("\
 Step backward exactly one instruction.\n\
 Argument N means do this N times (or till program stops for another reason).")
 	   );
-  add_com_alias ("rsi", "reverse-stepi", class_alias, 0);
+  add_com_alias ("rsi", "reverse-stepi", class_run, 0);
 
   add_com ("reverse-nexti", class_run, reverse_nexti, _("\
 Step backward one instruction, but proceed through called subroutines.\n\
 Argument N means do this N times (or till program stops for another reason).")
 	   );
-  add_com_alias ("rni", "reverse-nexti", class_alias, 0);
+  add_com_alias ("rni", "reverse-nexti", class_run, 0);
 
   add_com ("reverse-continue", class_run, reverse_continue, _("\
 Continue program being debugged but run it in reverse.\n\
 If proceeding from breakpoint, a number N may be used as an argument,\n\
 which means to set the ignore count of that breakpoint to N - 1 (so that\n\
 the breakpoint won't break until the Nth time it is reached)."));
-  add_com_alias ("rc", "reverse-continue", class_alias, 0);
+  add_com_alias ("rc", "reverse-continue", class_run, 0);
 
   add_com ("reverse-finish", class_run, reverse_finish, _("\
 Execute backward until just before selected stack frame is called."));
diff --git a/gdb/stack.c b/gdb/stack.c
index e8a9a924d4..11f6412b9d 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -3485,7 +3485,7 @@  With a negative COUNT, print outermost -COUNT frames."),
 
   add_com_alias ("bt", "backtrace", class_stack, 0);
 
-  add_com_alias ("where", "backtrace", class_alias, 0);
+  add_com_alias ("where", "backtrace", class_stack, 0);
   add_info ("stack", backtrace_command,
 	    _("Backtrace of the stack, or innermost COUNT frames."));
   add_info_alias ("s", "stack", 1);
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 7c862d5513..946443f07a 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3906,8 +3906,8 @@  on its own."), &cmdlist);
 			_("Commands for debugging overlays."), &overlaylist,
 			"overlay ", 0, &cmdlist);
 
-  add_com_alias ("ovly", "overlay", class_alias, 1);
-  add_com_alias ("ov", "overlay", class_alias, 1);
+  add_com_alias ("ovly", "overlay", class_support, 1);
+  add_com_alias ("ov", "overlay", class_support, 1);
 
   add_cmd ("map-overlay", class_support, map_overlay_command,
 	   _("Assert that an overlay section is mapped."), &overlaylist);
diff --git a/gdb/testsuite/gdb.base/alias.exp b/gdb/testsuite/gdb.base/alias.exp
index 69cfde67c1..6993d42648 100644
--- a/gdb/testsuite/gdb.base/alias.exp
+++ b/gdb/testsuite/gdb.base/alias.exp
@@ -122,3 +122,6 @@  gdb_test_no_output "alias abcd  = backtrace"
 gdb_test_no_output "alias abcde = backtrace"
 gdb_test_no_output "alias fghij = backtrace"
 gdb_test_no_output "alias fghi  = backtrace"
+
+# Verify help aliases shows the user defined aliases
+gdb_test "help aliases" ".*abcd --.*.*abcde --.*"
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index aa6bea4a8f..e7c7e50c0e 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -4098,8 +4098,8 @@  one or more \"collect\" commands, to specify what to collect\n\
 while single-stepping.\n\n\
 Note: this command can only be used in a tracepoint \"actions\" list."));
 
-  add_com_alias ("ws", "while-stepping", class_alias, 0);
-  add_com_alias ("stepping", "while-stepping", class_alias, 0);
+  add_com_alias ("ws", "while-stepping", class_trace, 0);
+  add_com_alias ("stepping", "while-stepping", class_trace, 0);
 
   add_com ("collect", class_trace, collect_pseudocommand, _("\
 Specify one or more data items to be collected at a tracepoint.\n\