Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command"

Joel Brobecker
2013-11-19T04:10:35Z

[-- Attachment #1: Type: text/plain, Size: 2278 bytes --]

> > I now think that it was indeed the correct choice. Not only does it
> > facilitate implementation (but only marginally), it also is consistent
> > with the current output. For instance, notice how GDB names the command
> > in the following error message:
> >
> > -unsupported
> > ^error,msg="Undefined MI command: unsupported"
> > ^^^^^^^^^^^
> > (no leading dash)
>
> Your example shows _output_ from MI. By contrast, we are talking
> about _input_. When I send commands to MI, I cannot omit the leading
> dash, so it can be very natural to consider it part of the command.
>
> We don't have to advertise that we support the dash,
>
> > Also, looking at the grammar, the leading dash isn't listed
> > as part of what they call the "operation"
>
> IMO, this line of reasoning makes little sense to users. Grammars are
> for programs, not for people.

To me, documentation is not an issue. I confess that I remain
unconvinced in this case, especially since this is a command meant
for programs rather than humans, so the risk of using it improperly
is low, given the clear documentation. However, I don't have a strong
opinion, and supporting both forms is pretty easy, so unless someone
strongly objects to allowing the second form, I've just gone ahead and
added it.

Updated patch attached. And for review convenience, I am also attaching
a diff of the changes I made on top of path #1 (to get to the updated
patch).

gdb/ChangeLog:

* mi/mi-cmds.h (mi_cmd_info_gdb_mi_command): Declare.
* mi/mi-cmd-info.c (mi_cmd_info_gdb_mi_command): New function.
* mi/mi-cmds.c (mi_cmds): Add -info-gdb-mi-command command.
* mi/mi-main.c (mi_cmd_list_features): Add "info-gdb-mi-command"
field to output of "-list-features".

* NEWS: Add entry for new -info-gdb-mi-command.

gdb/doc/ChangeLog:

* gdb.texinfo (GDB/MI Miscellaneous Commands): Document
the new -info-gdb-mi-command GDB/MI command. Document
the meaning of "-info-gdb-mi-command" in the output of
-list-features.

gdb/testsuite/ChangeLog:

* gdb.mi/mi-i-cmd.exp: New file.

Re-tested on x86_64-linux. OK to commit?

Thank you,
--
Joel

[-- Attachment #2: 0001-New-GDB-MI-command-info-gdb-mi-command.patch --]
[-- Type: text/x-diff, Size: 10540 bytes --]

From 5a8ff8bf85f7d455c3f75312e30ec5c1e077ae01 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 12 Nov 2013 14:51:30 +0400
Subject: [PATCH] New GDB/MI command "-info-gdb-mi-command"

This patch adds a new GDB/MI command meant for graphical frontends
trying to determine whether a given GDB/MI command exists or not.

Examples:

-info-gdb-mi-command unsupported-command
^done,command={exists="false"}
(gdb)
-info-gdb-mi-command symbol-list-lines
^done,command={exists="true"}
(gdb)

At the moment, this is the only piece of information that this
command returns.

Eventually, and if needed, we can extend it to provide
command-specific pieces of information, such as updates to
the command's syntax since inception. This could become,
for instance:

-info-gdb-mi-command symbol-list-lines
^done,command={exists="true",features=[]}
(gdb)
-info-gdb-mi-command catch-assert
^done,command={exists="true",features=["conditions"]}

In the first case, it would mean that no extra features,
while in the second, it announces that the -catch-assert
command in this version of the debugger supports a feature
called "condition" - exact semantics to be documented with
combined with the rest of the queried command's documentation.

But for now, we start small, and only worry about existance.
And to bootstrap the process, I have added an entry in the
output of the -list-features command as well ("info-gdb-mi-command"),
allowing the graphical frontends to go through the following process:

1. Send -list-features, collect info from there as before;
2. Check if the output contains "info-gdb-mi-command".
If it does, then support for various commands can be
queried though -info-gdb-mi-command. Newer commands
will be expected to always be checked via this new
-info-gdb-mi-command.

gdb/ChangeLog:

* mi/mi-cmds.h (mi_cmd_info_gdb_mi_command): Declare.
* mi/mi-cmd-info.c (mi_cmd_info_gdb_mi_command): New function.
* mi/mi-cmds.c (mi_cmds): Add -info-gdb-mi-command command.
* mi/mi-main.c (mi_cmd_list_features): Add "info-gdb-mi-command"
field to output of "-list-features".

* NEWS: Add entry for new -info-gdb-mi-command.

gdb/doc/ChangeLog:

* gdb.texinfo (GDB/MI Miscellaneous Commands): Document
the new -info-gdb-mi-command GDB/MI command. Document
the meaning of "-info-gdb-mi-command" in the output of
-list-features.

gdb/testsuite/ChangeLog:

* gdb.mi/mi-i-cmd.exp: New file.
---
gdb/NEWS | 3 +++
gdb/doc/gdb.texinfo | 53 +++++++++++++++++++++++++++++++++++++++
gdb/mi/mi-cmd-info.c | 29 +++++++++++++++++++++
gdb/mi/mi-cmds.c | 1 +
gdb/mi/mi-cmds.h | 1 +
gdb/mi/mi-main.c | 1 +
gdb/testsuite/gdb.mi/mi-i-cmd.exp | 46 +++++++++++++++++++++++++++++++++
7 files changed, 134 insertions(+)
create mode 100644 gdb/testsuite/gdb.mi/mi-i-cmd.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 9fc3638..e61c79f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -153,6 +153,9 @@ show startup-with-shell

** All MI commands now accept an optional "--language" option.

+ ** The new command -info-gdb-mi-command allows the user to determine
+ whether a GDB/MI command is supported or not.
+
** The -trace-save MI command can optionally save trace buffer in Common
Trace Format now.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 19e9aa5..41856b4 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35069,6 +35069,57 @@ default shows this information when you start an interactive session.
(gdb)
@end smallexample

+@subheading The @code{-info-gdb-mi-command} Command
+@cindex @code{-info-gdb-mi-command}
+@findex -info-gdb-mi-command
+
+@subsubheading Synopsis
+
+@smallexample
+ -info-gdb-mi-command @var{cmd_name}
+@end smallexample
+
+Query support for the @sc{gdb/mi} command named @var{cmd_name}.
+
+Note that the dash (@code{-}) starting all @sc{gdb/mi} commands
+is technically not part of the command name (@pxref{GDB/MI Input
+Syntax}), and thus should be omitted in @var{cmd_name}. However,
+for ease of use, this command also accepts the form with the leading
+dash.
+
+@subsubheading @value{GDBN} Command
+
+There is no corresponding @value{GDBN} command.
+
+@subsubheading Result
+
+The result is a tuple. There is currently only one field:
+
+@table @samp
+@item exists
+This field is equal to @code{"true"} if the @sc{gdb/mi} command exists,
+@code{"false"} otherwise.
+
+@end table
+
+@subsubheading Example
+
+Here is an example where the @sc{gdb/mi} command does not exist:
+
+@smallexample
+-info-gdb-mi-command unsupported-command
+^done,command=@{exists="false"@}
+@end smallexample
+
+@noindent
+And here is an example where the @sc{gdb/mi} command is known
+to the debugger:
+
+@smallexample
+-info-gdb-mi-command symbol-list-lines
+^done,command=@{exists="true"@}
+@end smallexample
+
@subheading The @code{-list-features} Command
@findex -list-features

@@ -35122,6 +35173,8 @@ exceptions: @code{-info-ada-exceptions}, @code{-catch-assert} and
@item language-option
Indicates that all @sc{gdb/mi} commands accept the @option{--language}
option (@pxref{Context management}).
+@item info-gdb-mi-command
+Indicates support for the @code{-info-gdb-mi-command} command.
@end table

@subheading The @code{-list-target-features} Command
diff --git a/gdb/mi/mi-cmd-info.c b/gdb/mi/mi-cmd-info.c
index aa4d210..0fce25a 100644
--- a/gdb/mi/mi-cmd-info.c
+++ b/gdb/mi/mi-cmd-info.c
@@ -71,6 +71,35 @@ mi_cmd_info_ada_exceptions (char *command, char **argv, int argc)
do_cleanups (old_chain);
}

+/* Implement the "-info-gdb-mi-command" GDB/MI command. */
+
+void
+mi_cmd_info_gdb_mi_command (char *command, char **argv, int argc)
+{
+ const char *cmd_name;
+ struct mi_cmd *cmd;
+ struct ui_out *uiout = current_uiout;
+ struct cleanup *old_chain;
+
+ /* This command takes exactly one argument. */
+ if (argc != 1)
+ error (_("Usage: -info-gdb-mi-command MI_COMMAND_NAME"));
+ cmd_name = argv[0];
+
+ /* Normally, the command name (aka the "operation" in the GDB/MI
+ grammar), does not include the leading '-' (dash). But for
+ the user's convenience, allow the user to specify the command
+ name to be with or without that leading dash. */
+ if (cmd_name[0] == '-')
+ cmd_name++;
+
+ cmd = mi_lookup (cmd_name);
+
+ old_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "command");
+ ui_out_field_string (uiout, "exists", cmd != NULL ? "true" : "false");
+ do_cleanups (old_chain);
+}
+
void
mi_cmd_info_os (char *command, char **argv, int argc)
{
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index c536d8a..58a8b89 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -125,6 +125,7 @@ static struct mi_cmd mi_cmds[] =
DEF_MI_CMD_MI ("inferior-tty-set", mi_cmd_inferior_tty_set),
DEF_MI_CMD_MI ("inferior-tty-show", mi_cmd_inferior_tty_show),
DEF_MI_CMD_MI ("info-ada-exceptions", mi_cmd_info_ada_exceptions),
+ DEF_MI_CMD_MI ("info-gdb-mi-command", mi_cmd_info_gdb_mi_command),
DEF_MI_CMD_MI ("info-os", mi_cmd_info_os),
DEF_MI_CMD_MI ("interpreter-exec", mi_cmd_interpreter_exec),
DEF_MI_CMD_MI ("list-features", mi_cmd_list_features),
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index cb8aac1..4ea95fa 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -74,6 +74,7 @@ extern mi_cmd_argv_ftype mi_cmd_gdb_exit;
extern mi_cmd_argv_ftype mi_cmd_inferior_tty_set;
extern mi_cmd_argv_ftype mi_cmd_inferior_tty_show;
extern mi_cmd_argv_ftype mi_cmd_info_ada_exceptions;
+extern mi_cmd_argv_ftype mi_cmd_info_gdb_mi_command;
extern mi_cmd_argv_ftype mi_cmd_info_os;
extern mi_cmd_argv_ftype mi_cmd_interpreter_exec;
extern mi_cmd_argv_ftype mi_cmd_list_features;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 793204d..48c8d09 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1817,6 +1817,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
ui_out_field_string (uiout, NULL, "ada-task-info");
ui_out_field_string (uiout, NULL, "ada-exceptions");
ui_out_field_string (uiout, NULL, "language-option");
+ ui_out_field_string (uiout, NULL, "info-gdb-mi-command");

#if HAVE_PYTHON
if (gdb_python_initialized)
diff --git a/gdb/testsuite/gdb.mi/mi-i-cmd.exp b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
new file mode 100644
index 0000000..d460559
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
@@ -0,0 +1,46 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+ continue
+}
+
+# First, verify that the debugger correctly advertises support
+# for the -info-gdb-mi-command command.
+mi_gdb_test "-list-features" \
+ "\\^done,features=\\\[.*\"info-gdb-mi-command\".*\\\]" \
+ "-list-features should include \"info-gdb-mi-command\""
+
+mi_gdb_test "-info-gdb-mi-command unsupported-command" \
+ "\\^done,command=\\\{exists=\"false\"\\\}" \
+ "-info-gdb-mi-command unsupported-command"
+
+# Same test as above, but including the leading '-' in the command name.
+mi_gdb_test "-info-gdb-mi-command -unsupported-command" \
+ "\\^done,command=\\\{exists=\"false\"\\\}" \
+ "-info-gdb-mi-command -unsupported-command"
+
+mi_gdb_test "-info-gdb-mi-command symbol-list-lines" \
+ "\\^done,command=\\\{exists=\"true\"\\\}" \
+ "-info-gdb-mi-command symbol-list-lines"
+
+# Same test as above, but including the leading '-' in the command name.
+mi_gdb_test "-info-gdb-mi-command -symbol-list-lines" \
+ "\\^done,command=\\\{exists=\"true\"\\\}" \
+ "-info-gdb-mi-command -symbol-list-lines"
--
1.8.1.2

[-- Attachment #3: info-gdb-mi-command-updates.diff --]
[-- Type: text/x-diff, Size: 2833 bytes --]

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f0662ff..4227f31 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35088,8 +35088,13 @@ default shows this information when you start an interactive session.
-info-gdb-mi-command @var{cmd_name}
@end smallexample

-Query support for the @sc{gdb/mi} command named @var{cmd_name}
-(the leading dash (@code{-}) in the command name should be omitted).
+Query support for the @sc{gdb/mi} command named @var{cmd_name}.
+
+Note that the dash (@code{-}) starting all @sc{gdb/mi} commands
+is technically not part of the command name (@pxref{GDB/MI Input
+Syntax}), and thus should be omitted in @var{cmd_name}. However,
+for ease of use, this command also accepts the form with the leading
+dash.

@subsubheading @value{GDBN} Command

@@ -35120,6 +35120,7 @@ Here is an example where the @sc{gdb/mi} command does not exist:
^done,command=@{exists="false"@}
@end smallexample

+@noindent
And here is an example where the @sc{gdb/mi} command is known
to the debugger:

diff --git a/gdb/mi/mi-cmd-info.c b/gdb/mi/mi-cmd-info.c
index 18f4927..0fce25a 100644
--- a/gdb/mi/mi-cmd-info.c
+++ b/gdb/mi/mi-cmd-info.c
@@ -85,6 +85,14 @@ mi_cmd_info_gdb_mi_command (char *command, char **argv, int argc)
if (argc != 1)
error (_("Usage: -info-gdb-mi-command MI_COMMAND_NAME"));
cmd_name = argv[0];
+
+ /* Normally, the command name (aka the "operation" in the GDB/MI
+ grammar), does not include the leading '-' (dash). But for
+ the user's convenience, allow the user to specify the command
+ name to be with or without that leading dash. */
+ if (cmd_name[0] == '-')
+ cmd_name++;
+
cmd = mi_lookup (cmd_name);

old_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "command");
diff --git a/gdb/testsuite/gdb.mi/mi-i-cmd.exp b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
index 5285d31..d460559 100644
--- a/gdb/testsuite/gdb.mi/mi-i-cmd.exp
+++ b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
@@ -31,7 +31,16 @@ mi_gdb_test "-info-gdb-mi-command unsupported-command" \
"\\^done,command=\\\{exists=\"false\"\\\}" \
"-info-gdb-mi-command unsupported-command"

+# Same test as above, but including the leading '-' in the command name.
+mi_gdb_test "-info-gdb-mi-command -unsupported-command" \
+ "\\^done,command=\\\{exists=\"false\"\\\}" \
+ "-info-gdb-mi-command -unsupported-command"
+
mi_gdb_test "-info-gdb-mi-command symbol-list-lines" \
"\\^done,command=\\\{exists=\"true\"\\\}" \
"-info-gdb-mi-command symbol-list-lines"

+# Same test as above, but including the leading '-' in the command name.
+mi_gdb_test "-info-gdb-mi-command -symbol-list-lines" \
+ "\\^done,command=\\\{exists=\"true\"\\\}" \
+ "-info-gdb-mi-command -symbol-list-lines"