* [PATCH 0/2] Improving tab completion for watch commands
@ 2020-11-16 22:19 Andrew Burgess
2020-11-16 22:19 ` [PATCH 1/2] gdb: convert some function arguments from int to bool Andrew Burgess
2020-11-16 22:19 ` [PATCH 2/2] gdb: update command completion for watch, awatch, and rwatch Andrew Burgess
0 siblings, 2 replies; 5+ messages in thread
From: Andrew Burgess @ 2020-11-16 22:19 UTC (permalink / raw)
To: gdb-patches
The good stuff is in patch #2.
Thanks,
Andrew
---
Andrew Burgess (2):
gdb: convert some function arguments from int to bool
gdb: update command completion for watch, awatch, and rwatch
gdb/ChangeLog | 27 +++++
gdb/breakpoint.c | 146 +++++++++++++++++++-------
gdb/breakpoint.h | 6 +-
gdb/eval.c | 2 +-
gdb/mi/mi-cmd-break.c | 6 +-
gdb/ppc-linux-nat.c | 4 +-
gdb/testsuite/ChangeLog | 4 +
gdb/testsuite/gdb.base/completion.exp | 16 +++
gdb/value.h | 2 +-
9 files changed, 164 insertions(+), 49 deletions(-)
--
2.25.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] gdb: convert some function arguments from int to bool
2020-11-16 22:19 [PATCH 0/2] Improving tab completion for watch commands Andrew Burgess
@ 2020-11-16 22:19 ` Andrew Burgess
2020-11-18 16:15 ` Tom Tromey
2020-11-16 22:19 ` [PATCH 2/2] gdb: update command completion for watch, awatch, and rwatch Andrew Burgess
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Burgess @ 2020-11-16 22:19 UTC (permalink / raw)
To: gdb-patches
A little int to bool conversion around the 'watch' type commands.
There should be no user visible changes after this commit.
gdb/ChangeLog:
* breakpoint.c (update_watchpoint): Pass 'false' not '0'.
(watch_command_1): Update parameter types. Convert locals to
bool.
(watch_command_wrapper): Change parameter type.
(watch_maybe_just_location): Change locals to bool.
(rwatch_command_wrapper): Update parameter type.
(awatch_command_wrapper): Update parameter type.
* breakpoint.h (watch_command_wrapper): Change parameter type.
(rwatch_command_wrapper): Update parameter type.
(awatch_command_wrapper): Update parameter type.
* eval.c (fetch_subexp_value): Change parameter type.
* ppc-linux-nat.c (ppc_linux_nat_target::check_condition): Pass
'false' not '0'.
* value.h (fetch_subexp_value): Change parameter type in
declaration.
---
gdb/ChangeLog | 18 ++++++++++++++++++
gdb/breakpoint.c | 22 +++++++++++-----------
gdb/breakpoint.h | 6 +++---
gdb/eval.c | 2 +-
gdb/mi/mi-cmd-break.c | 6 +++---
gdb/ppc-linux-nat.c | 4 ++--
gdb/value.h | 2 +-
7 files changed, 39 insertions(+), 21 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 23278320d9d..a4ae8b2d8c9 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1905,7 +1905,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
struct value *v, *result;
struct program_space *frame_pspace;
- fetch_subexp_value (b->exp.get (), &pc, &v, &result, &val_chain, 0);
+ fetch_subexp_value (b->exp.get (), &pc, &v, &result, &val_chain, false);
/* Avoid setting b->val if it's already set. The meaning of
b->val is 'the last value' user saw, and we should update
@@ -4969,7 +4969,7 @@ watchpoint_check (bpstat bs)
return WP_VALUE_CHANGED;
mark = value_mark ();
- fetch_subexp_value (b->exp.get (), &pc, &new_val, NULL, NULL, 0);
+ fetch_subexp_value (b->exp.get (), &pc, &new_val, NULL, NULL, false);
if (b->val_bitsize != 0)
new_val = extract_bitfield_from_watchpoint_value (b, new_val);
@@ -10676,7 +10676,7 @@ is_masked_watchpoint (const struct breakpoint *b)
hw_access: watch access (read or write) */
static void
watch_command_1 (const char *arg, int accessflag, int from_tty,
- int just_location, int internal)
+ bool just_location, bool internal)
{
struct breakpoint *scope_breakpoint = NULL;
const struct block *exp_valid_block = NULL, *cond_exp_valid_block = NULL;
@@ -10693,7 +10693,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
int pc = 0;
/* Flag to indicate whether we are going to use masks for
the hardware watchpoint. */
- int use_mask = 0;
+ bool use_mask = false;
CORE_ADDR mask = 0;
/* Make sure that we actually have parameters to parse. */
@@ -10760,7 +10760,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
if (use_mask)
error(_("You can specify only one mask."));
- use_mask = just_location = 1;
+ use_mask = just_location = true;
mark = value_mark ();
mask_value = parse_to_comma_and_eval (&value_start);
@@ -11077,7 +11077,7 @@ can_use_hardware_watchpoint (const std::vector<value_ref_ptr> &vals)
}
void
-watch_command_wrapper (const char *arg, int from_tty, int internal)
+watch_command_wrapper (const char *arg, int from_tty, bool internal)
{
watch_command_1 (arg, hw_write, from_tty, 0, internal);
}
@@ -11088,14 +11088,14 @@ watch_command_wrapper (const char *arg, int from_tty, int internal)
static void
watch_maybe_just_location (const char *arg, int accessflag, int from_tty)
{
- int just_location = 0;
+ bool just_location = false;
if (arg
&& (check_for_argument (&arg, "-location", sizeof ("-location") - 1)
|| check_for_argument (&arg, "-l", sizeof ("-l") - 1)))
- just_location = 1;
+ just_location = true;
- watch_command_1 (arg, accessflag, from_tty, just_location, 0);
+ watch_command_1 (arg, accessflag, from_tty, just_location, false);
}
static void
@@ -11105,7 +11105,7 @@ watch_command (const char *arg, int from_tty)
}
void
-rwatch_command_wrapper (const char *arg, int from_tty, int internal)
+rwatch_command_wrapper (const char *arg, int from_tty, bool internal)
{
watch_command_1 (arg, hw_read, from_tty, 0, internal);
}
@@ -11117,7 +11117,7 @@ rwatch_command (const char *arg, int from_tty)
}
void
-awatch_command_wrapper (const char *arg, int from_tty, int internal)
+awatch_command_wrapper (const char *arg, int from_tty, bool internal)
{
watch_command_1 (arg, hw_access, from_tty, 0, internal);
}
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index b13522e236b..4a65dd2dd43 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1300,9 +1300,9 @@ const char *bpdisp_text (enum bpdisp disp);
extern void break_command (const char *, int);
-extern void watch_command_wrapper (const char *, int, int);
-extern void awatch_command_wrapper (const char *, int, int);
-extern void rwatch_command_wrapper (const char *, int, int);
+extern void watch_command_wrapper (const char *, int, bool);
+extern void awatch_command_wrapper (const char *, int, bool);
+extern void rwatch_command_wrapper (const char *, int, bool);
extern void tbreak_command (const char *, int);
extern struct breakpoint_ops base_breakpoint_ops;
diff --git a/gdb/eval.c b/gdb/eval.c
index 308f4771482..2626ee6d876 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -183,7 +183,7 @@ void
fetch_subexp_value (struct expression *exp, int *pc, struct value **valp,
struct value **resultp,
std::vector<value_ref_ptr> *val_chain,
- int preserve_errors)
+ bool preserve_errors)
{
struct value *mark, *new_mark, *result;
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 3835c02650d..ed311a6ba33 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -463,13 +463,13 @@ mi_cmd_break_watch (const char *command, char **argv, int argc)
switch (type)
{
case REG_WP:
- watch_command_wrapper (expr, FROM_TTY, 0);
+ watch_command_wrapper (expr, FROM_TTY, false);
break;
case READ_WP:
- rwatch_command_wrapper (expr, FROM_TTY, 0);
+ rwatch_command_wrapper (expr, FROM_TTY, false);
break;
case ACCESS_WP:
- awatch_command_wrapper (expr, FROM_TTY, 0);
+ awatch_command_wrapper (expr, FROM_TTY, false);
break;
default:
error (_("-break-watch: Unknown watchpoint type."));
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 7131134c10a..095ed577a09 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -2484,13 +2484,13 @@ ppc_linux_nat_target::check_condition (CORE_ADDR watch_addr,
if (cond->elts[0].opcode != BINOP_EQUAL)
return 0;
- fetch_subexp_value (cond, &pc, &left_val, NULL, &left_chain, 0);
+ fetch_subexp_value (cond, &pc, &left_val, NULL, &left_chain, false);
num_accesses_left = num_memory_accesses (left_chain);
if (left_val == NULL || num_accesses_left < 0)
return 0;
- fetch_subexp_value (cond, &pc, &right_val, NULL, &right_chain, 0);
+ fetch_subexp_value (cond, &pc, &right_val, NULL, &right_chain, false);
num_accesses_right = num_memory_accesses (right_chain);
if (right_val == NULL || num_accesses_right < 0)
diff --git a/gdb/value.h b/gdb/value.h
index bc57f4f9012..2bd1999dc8f 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -928,7 +928,7 @@ extern value *eval_skip_value (expression *exp);
extern void fetch_subexp_value (struct expression *exp, int *pc,
struct value **valp, struct value **resultp,
std::vector<value_ref_ptr> *val_chain,
- int preserve_errors);
+ bool preserve_errors);
extern const char *extract_field_op (struct expression *exp, int *subexp);
--
2.25.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] gdb: update command completion for watch, awatch, and rwatch
2020-11-16 22:19 [PATCH 0/2] Improving tab completion for watch commands Andrew Burgess
2020-11-16 22:19 ` [PATCH 1/2] gdb: convert some function arguments from int to bool Andrew Burgess
@ 2020-11-16 22:19 ` Andrew Burgess
2020-11-18 16:55 ` Tom Tromey
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Burgess @ 2020-11-16 22:19 UTC (permalink / raw)
To: gdb-patches
Switch over to using new option processing mechanism for watch,
awatch, and rwatch commands. Add command completion function.
This means that expression completion now works correctly when the
-location flag is used. So previously:
(gdb) watch var.<TAB><TAB>
.... list fields of var ....
But,
(gdb) watch -location var.<TAB><TAB>
.... list all symbols ....
After this commit only the fields of 'var' are listed even when
'-location' is passed.
Another benefit of this change is that '-location' will now complete.
One thing to note is that previous these commands accepted both
'-location' or '-l' (these being synonyms). The new option scheme
doesn't really allow for official short form flags, however, it does
allow for non-ambiguous sub-strings to be used. What this means is
that currently (as these commands only have the '-location' flag) the
user can still use '-l', so there's no change there.
The interactive help text for these commands now emphasises
'-location' as the real option, but does mention that '-l' can also be
used.
gdb/ChangeLog:
* breakpoint.c (struct watch_options): New struct.
(watch_option_defs): New static global.
(make_watch_options_def_group): New function.
(watch_maybe_just_location): Convert option parsing.
(watch_command_completer): New function.
(_initialize_breakpoint): Build help text using options mechanism.
gdb/testsuite/ChangeLog:
* gdb.base/completion.exp: Add new completion tests.
---
gdb/ChangeLog | 9 ++
gdb/breakpoint.c | 130 ++++++++++++++++++++------
gdb/testsuite/ChangeLog | 4 +
gdb/testsuite/gdb.base/completion.exp | 16 ++++
4 files changed, 128 insertions(+), 31 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index a4ae8b2d8c9..27324a0c08d 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -11082,20 +11082,74 @@ watch_command_wrapper (const char *arg, int from_tty, bool internal)
watch_command_1 (arg, hw_write, from_tty, 0, internal);
}
+/* Options for the watch, awatch, and rwatch commands. */
+
+struct watch_options
+{
+ /* For -location. */
+ bool location = false;
+};
+
+/* Definitions of options for the "watch", "awatch", and "rwatch" commands.
+
+ Historically GDB always accepted both '-location' and '-l' flags for
+ these commands (both flags being synonyms). When converting to the
+ newer option scheme only '-location' is added here. That's fine (for
+ backward compatibility) as any non-ambiguous prefix of a flag will be
+ accepted, so '-l', '-loc', are now all accepted.
+
+ What this means is that, if in the future, we add any new flag here
+ that starts with '-l' then this will break backward compatibility, so
+ please, don't do that! */
+
+static const gdb::option::option_def watch_option_defs[] = {
+ gdb::option::flag_option_def<watch_options> {
+ "location",
+ [] (watch_options *opt) { return &opt->location; },
+ N_("\
+This evaluates EXPRESSION and watches the memory to which is refers.\n\
+-l can be used as a short form of -location."),
+ },
+};
+
+/* Returns the option group used by 'watch', 'awatch', and 'rwatch'
+ commands. */
+
+static gdb::option::option_def_group
+make_watch_options_def_group (watch_options *opts)
+{
+ return {{watch_option_defs}, opts};
+}
+
/* A helper function that looks for the "-location" argument and then
calls watch_command_1. */
static void
watch_maybe_just_location (const char *arg, int accessflag, int from_tty)
{
- bool just_location = false;
+ watch_options opts;
+ auto grp = make_watch_options_def_group (&opts);
+ gdb::option::process_options
+ (&arg, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, grp);
+ if (arg != nullptr && *arg == '\0')
+ arg = nullptr;
+
+ watch_command_1 (arg, accessflag, from_tty, opts.location, false);
+}
- if (arg
- && (check_for_argument (&arg, "-location", sizeof ("-location") - 1)
- || check_for_argument (&arg, "-l", sizeof ("-l") - 1)))
- just_location = true;
+/* Command completion for 'watch', 'awatch', and 'rwatch' commands. */
+static void
+watch_command_completer (struct cmd_list_element *ignore,
+ completion_tracker &tracker,
+ const char *text, const char * /*word*/)
+{
+ const auto group = make_watch_options_def_group (nullptr);
+ if (gdb::option::complete_options
+ (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group))
+ return;
- watch_command_1 (arg, accessflag, from_tty, just_location, false);
+ const char *word = advance_to_expression_complete_word_point (tracker, text);
+ expression_completer (ignore, tracker, text, word);
}
static void
@@ -15914,32 +15968,46 @@ If REGEX is given, only stop for libraries matching the regular expression."),
CATCH_PERMANENT,
CATCH_TEMPORARY);
- c = add_com ("watch", class_breakpoint, watch_command, _("\
-Set a watchpoint for an expression.\n\
-Usage: watch [-l|-location] EXPRESSION\n\
-A watchpoint stops execution of your program whenever the value of\n\
-an expression changes.\n\
-If -l or -location is given, this evaluates EXPRESSION and watches\n\
-the memory to which it refers."));
- set_cmd_completer (c, expression_completer);
-
- c = add_com ("rwatch", class_breakpoint, rwatch_command, _("\
-Set a read watchpoint for an expression.\n\
-Usage: rwatch [-l|-location] EXPRESSION\n\
-A watchpoint stops execution of your program whenever the value of\n\
-an expression is read.\n\
-If -l or -location is given, this evaluates EXPRESSION and watches\n\
-the memory to which it refers."));
- set_cmd_completer (c, expression_completer);
-
- c = add_com ("awatch", class_breakpoint, awatch_command, _("\
-Set a watchpoint for an expression.\n\
-Usage: awatch [-l|-location] EXPRESSION\n\
+ const auto opts = make_watch_options_def_group (nullptr);
+
+ static const std::string watch_help = gdb::option::build_help (_("\
+Set a watchpoint for EXPRESSION.\n\
+Usage: watch [-location] EXPRESSION\n\
+\n\
+Options:\n\
+%OPTIONS%\n\
+\n\
A watchpoint stops execution of your program whenever the value of\n\
-an expression is either read or written.\n\
-If -l or -location is given, this evaluates EXPRESSION and watches\n\
-the memory to which it refers."));
- set_cmd_completer (c, expression_completer);
+an expression changes."), opts);
+ c = add_com ("watch", class_breakpoint, watch_command,
+ watch_help.c_str ());
+ set_cmd_completer_handle_brkchars (c, watch_command_completer);
+
+ static const std::string rwatch_help = gdb::option::build_help (_("\
+Set a read watchpoint for EXPRESSION.\n\
+Usage: rwatch [-location] EXPRESSION\n\
+\n\
+Options:\n\
+%OPTIONS%\n\
+\n\
+A read watchpoint stops execution of your program whenever the value of\n\
+an expression is read."), opts);
+ c = add_com ("rwatch", class_breakpoint, rwatch_command,
+ rwatch_help.c_str ());
+ set_cmd_completer_handle_brkchars (c, watch_command_completer);
+
+ static const std::string awatch_help = gdb::option::build_help (_("\
+Set an access watchpoint for EXPRESSION.\n\
+Usage: awatch [-location] EXPRESSION\n\
+\n\
+Options:\n\
+%OPTIONS%\n\
+\n\
+An access watchpoint stops execution of your program whenever the value\n\
+of an expression is either read or written."), opts);
+ c = add_com ("awatch", class_breakpoint, awatch_command,
+ awatch_help.c_str ());
+ set_cmd_completer_handle_brkchars (c, watch_command_completer);
add_info ("watchpoints", info_watchpoints_command, _("\
Status of specified watchpoints (all watchpoints if no argument)."));
diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
index ac7f61ddfbc..5bbfda21b25 100644
--- a/gdb/testsuite/gdb.base/completion.exp
+++ b/gdb/testsuite/gdb.base/completion.exp
@@ -172,6 +172,11 @@ if { ![readline_is_used] } {
return -1
}
+# The bulk of this test script pre-dates the completion-support
+# library, and should probably (where possible) be converted.
+# However, for now, new tests are being added using this library.
+load_lib completion-support.exp
+
set test "complete 'hfgfh'"
send_gdb "hfgfh\t"
gdb_test_multiple "" "$test" {
@@ -922,3 +927,14 @@ gdb_test_multiple "" "$test" {
pass "$test"
}
}
+
+# Check the watch commands can all complete, with and without flags.
+foreach_with_prefix cmd { "watch" "awatch" "rwatch" } {
+ foreach_with_prefix opt { "" "-l" "-location" } {
+ test_gdb_complete_multiple "${cmd} ${opt} some_union_global." \
+ "" "f" {
+ "f1"
+ "f2"
+ }
+ }
+}
--
2.25.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] gdb: convert some function arguments from int to bool
2020-11-16 22:19 ` [PATCH 1/2] gdb: convert some function arguments from int to bool Andrew Burgess
@ 2020-11-18 16:15 ` Tom Tromey
0 siblings, 0 replies; 5+ messages in thread
From: Tom Tromey @ 2020-11-18 16:15 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
Andrew> A little int to bool conversion around the 'watch' type commands.
Andrew> There should be no user visible changes after this commit.
Looks good, thanks for doing this.
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] gdb: update command completion for watch, awatch, and rwatch
2020-11-16 22:19 ` [PATCH 2/2] gdb: update command completion for watch, awatch, and rwatch Andrew Burgess
@ 2020-11-18 16:55 ` Tom Tromey
0 siblings, 0 replies; 5+ messages in thread
From: Tom Tromey @ 2020-11-18 16:55 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
Andrew> The interactive help text for these commands now emphasises
Andrew> '-location' as the real option, but does mention that '-l' can also be
Andrew> used.
Andrew> gdb/ChangeLog:
Andrew> * breakpoint.c (struct watch_options): New struct.
Andrew> (watch_option_defs): New static global.
Andrew> (make_watch_options_def_group): New function.
Andrew> (watch_maybe_just_location): Convert option parsing.
Andrew> (watch_command_completer): New function.
Andrew> (_initialize_breakpoint): Build help text using options mechanism.
Thank you for doing this. This looks good to me.
Andrew> +Usage: watch [-location] EXPRESSION\n\
Not relevant for this patch, but I've wondered if these Usage lines
should explain the full syntax, like '[if EXPRESSION]' and '[thread ID]'
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-18 16:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-16 22:19 [PATCH 0/2] Improving tab completion for watch commands Andrew Burgess
2020-11-16 22:19 ` [PATCH 1/2] gdb: convert some function arguments from int to bool Andrew Burgess
2020-11-18 16:15 ` Tom Tromey
2020-11-16 22:19 ` [PATCH 2/2] gdb: update command completion for watch, awatch, and rwatch Andrew Burgess
2020-11-18 16:55 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).