gdb: update command completion for watch, awatch, and rwatch

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.

4 files changed, 123 insertions, 31 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a56208c..afff5d5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2020-11-19  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* 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.
+
+2020-11-19  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* breakpoint.c (update_watchpoint): Pass 'false' not '0'.
 	(watch_command_1): Update parameter types.  Convert locals to
 	bool.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index a4ae8b2..27324a0 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/ChangeLog b/gdb/testsuite/ChangeLog
index 1deb7a0..7b01010 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2020-11-19  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.base/completion.exp: Add new completion tests.
+
 2020-11-18  Simon Marchi  <simon.marchi@polymtl.ca>
 
 	* gdb.mi/mi-nonstop-exit.exp: Enable non-stop through GDBFLAGS.
diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
index 090f521..1c5d03b 100644
--- a/gdb/testsuite/gdb.base/completion.exp
+++ b/gdb/testsuite/gdb.base/completion.exp
@@ -951,3 +951,14 @@ test_gdb_complete_multiple "p -array on -- /d some_union_global." \
 	"f1"
 	"f2"
     }
+
+# 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"
+	    }
+    }
+}