Make "thread apply" use the gdb::option framework

Similarly to the "frame apply" patch, this makes the "thread apply"
family of commands -- "thread apply TID", "thread apply all" and
"taas" use the gdb::option framework for '-'-style options.

No new options are added, but there are some user-visible changes:

- Can now abbreviate and complete "-ascending"

- We now have a completer for "thread apply" commands

  Can now complete options ("thread apply all -[TAB]"), and also,
  'thread apply all COMMAND[TAB]' now does what you'd expect, by
  making use of the new complete_command routine.

- "help" output tweaked with auto-generated option descriptions:

   ~~~
   Usage: thread apply all [OPTION]... COMMAND
   Prints per-inferior thread number and target system's thread id
   followed by COMMAND output.

   By default, an error raised during the execution of COMMAND
   aborts "thread apply".

   Options:
     -ascending
       Call COMMAND for all threads in ascending order.
       The default is descending order.

     -q
       Disables printing the thread information.

     -c
       Print any error raised by COMMAND and continue.

     -s
       Silently ignore any errors or empty output produced by COMMAND.
   ~~~

  The "By default ..." sentence is new as well.

gdb/ChangeLog:
2019-06-13  Pedro Alves  <palves@redhat.com>

	* thread.c: Include "cli/cli-option.h".
	(tp_array_compar_ascending): Global.
	(tp_array_compar): Delete function.
	(tp_array_compar_ascending, tp_array_compar_descending): New
	functions.
	(ascending_option_def, qcs_flag_option_def)
	(thr_qcs_flags_option_defs)
	(make_thread_apply_all_options_def_group)
	(make_thread_apply_options_def_group): New.
	(thread_apply_all_command): Use gdb::option::process_options.
	(thread_apply_command_completer)
	(thread_apply_all_command_completer): New.
	(thread_apply_command): Use gdb::option::process_options.
	(_initialize_thread): Delete THREAD_APPLY_FLAGS_HELP, replace it
	with a new THREAD_APPLY_OPTION_HELP.  Use gdb::option::build_help
	to generate help text of "thread apply".  Adjust "taas"'s help.
	* tid-parse.c (tid_range_parser::in_thread_range): New method.
	* tid-parse.h (tid_range_parser::in_thread_range): New method.

gdb/testsuite/ChangeLog:
2019-06-13  Pedro Alves  <palves@redhat.com>

	* gdb.base/options.exp (test-thread-apply): New.
	(top level): Call it.

gdb/ChangeLog

@@ -1,3 +1,24 @@
+2019-06-13  Pedro Alves  <palves@redhat.com>
+
+	* thread.c: Include "cli/cli-option.h".
+	(tp_array_compar_ascending): Global.
+	(tp_array_compar): Delete function.
+	(tp_array_compar_ascending, tp_array_compar_descending): New
+	functions.
+	(ascending_option_def, qcs_flag_option_def)
+	(thr_qcs_flags_option_defs)
+	(make_thread_apply_all_options_def_group)
+	(make_thread_apply_options_def_group): New.
+	(thread_apply_all_command): Use gdb::option::process_options.
+	(thread_apply_command_completer)
+	(thread_apply_all_command_completer): New.
+	(thread_apply_command): Use gdb::option::process_options.
+	(_initialize_thread): Delete THREAD_APPLY_FLAGS_HELP, replace it
+	with a new THREAD_APPLY_OPTION_HELP.  Use gdb::option::build_help
+	to generate help text of "thread apply".  Adjust "taas"'s help.
+	* tid-parse.c (tid_range_parser::in_thread_range): New method.
+	* tid-parse.h (tid_range_parser::in_thread_range): New method.
+
 2019-06-13  Pedro Alves  <palves@redhat.com>
 
 	* thread.c (thread_apply_command): Check for invalid TID with

gdb/testsuite/ChangeLog

@@ -1,3 +1,8 @@
+2019-06-13  Pedro Alves  <palves@redhat.com>
+
+	* gdb.base/options.exp (test-thread-apply): New.
+	(top level): Call it.
+
 2019-06-13  Pedro Alves  <palves@redhat.com>
 
 	* gdb.base/options.exp (test-frame-apply): New.

gdb/testsuite/gdb.base/options.exp

@@ -28,6 +28,8 @@
 #  - frame apply
 #  - faas
 #  - tfaas
+#  - thread apply
+#  - taas
 
 load_lib completion-support.exp
 
@@ -371,6 +373,87 @@ proc_with_prefix test-frame-apply {} {
     }
 }
 
+# Basic option-machinery + "thread apply" command integration tests.
+proc_with_prefix test-thread-apply {} {
+
+    test_gdb_complete_unique "thread apply all" "thread apply all"
+    test_gdb_complete_unique "taas" "taas"
+
+    gdb_test "thread apply 1-" \
+	"inverted range"
+    test_gdb_complete_none "frame apply level 1-"
+
+    foreach cmd {
+	"thread apply all"
+	"thread apply 1"
+	"taas"
+    } {
+	test_gdb_completion_offers_commands "$cmd "
+
+	# taas is silent on command error by design.  This procedure
+	# hides the difference.  EXPECTED_RE is only considered when
+	# not testing with "taas".
+	proc test_invalid_cmd {cmd arg expected_re} {
+	    if {$cmd != "taas"} {
+		gdb_test "$cmd$arg" $expected_re
+	    } else {
+		gdb_test_no_output "$cmd$arg"
+	    }
+	}
+
+	gdb_test "$cmd -" "Ambiguous option at: -"
+
+	if {$cmd != "thread apply 1"} {
+	    test_gdb_complete_multiple "$cmd " "-" "" {
+		"-ascending"
+		"-c"
+		"-q"
+		"-s"
+	    }
+	} else {
+	    # "-ascending" only works with "all".
+	    test_gdb_complete_multiple "$cmd " "-" "" {
+		"-c"
+		"-q"
+		"-s"
+	    }
+	}
+
+	if {$cmd == "thread apply all" || $cmd == "taas"} {
+	    set errmsg \
+		"Please specify a command at the end of 'thread apply all'"
+	} elseif {$cmd == "thread apply 1"} {
+	    set errmsg \
+		"Please specify a command following the thread ID list"
+	} else {
+	    error "unexpected cmd: $cmd"
+	}
+
+	with_test_prefix "no-trailing-space" {
+	    gdb_test "$cmd --" $errmsg
+	    test_gdb_complete_unique "$cmd --" "$cmd --"
+	}
+
+	with_test_prefix "trailing-space" {
+	    gdb_test "$cmd -- " $errmsg
+	    test_gdb_completion_offers_commands "$cmd -- "
+	}
+
+	# '-' is a valid TUI command.
+	test_invalid_cmd "$cmd" " -- -" \
+	    "Cannot enable the TUI when output is not a terminal"
+	test_gdb_complete_unique \
+	    "$cmd -- -" \
+	    "$cmd -- -"
+
+	test_invalid_cmd $cmd " -foo" \
+	    "Undefined command: \"-foo\".  Try \"help\"\\."
+	test_gdb_complete_none "$cmd -foo"
+
+	test_gdb_completion_offers_commands "$cmd -c "
+    }
+}
+
 # Miscellaneous tests.
 proc_with_prefix test-misc {variant} {
     global all_options
@@ -808,14 +891,17 @@ foreach_with_prefix cmd {
 }
 
 # Run the print integration tests, both as "standalone", and under
-# "frame apply".  The latter checks that the "frame apply ... COMMAND"
+# "frame/thread apply".  The latter checks that the "frame/thread
-# commands recurse the completion machinery for COMMAND completion
+# apply ... COMMAND" commands recurse the completion machinery for
-# correctly.
+# COMMAND completion correctly.
 foreach prefix {
     ""
     "frame apply all "
     "frame apply 1 "
     "frame apply level 0 "
+    "thread apply all "
+    "thread apply 1 "
+    "thread apply 1 frame apply 1 "
 } {
     test-print $prefix
 }
@@ -832,3 +918,6 @@ test-backtrace
 
 # Basic "frame apply" integration tests.
 test-frame-apply
+
+# Basic "thread apply" integration tests.
+test-thread-apply

gdb/thread.c

@@ -39,6 +39,7 @@
 #include "observable.h"
 #include "annotate.h"
 #include "cli/cli-decode.h"
+#include "cli/cli-option.h"
 #include "gdb_regex.h"
 #include "cli/cli-utils.h"
 #include "thread-fsm.h"
@@ -1426,30 +1427,30 @@ print_thread_id (struct thread_info *thr)
   return s;
 }
 
-/* If true, tp_array_compar should sort in ascending order, otherwise
+/* Sort an array of struct thread_info pointers by thread ID (first by
-   in descending order.  */
+   inferior number, and then by per-inferior thread number).  Sorts in
-
+   ascending order.  */
-static bool tp_array_compar_ascending;
-
-/* Sort an array for struct thread_info pointers by thread ID (first
-   by inferior number, and then by per-inferior thread number).  The
-   order is determined by TP_ARRAY_COMPAR_ASCENDING.  */
 
 static bool
-tp_array_compar (const thread_info *a, const thread_info *b)
+tp_array_compar_ascending (const thread_info *a, const thread_info *b)
 {
   if (a->inf->num != b->inf->num)
-    {
+    return a->inf->num < b->inf->num;
-      if (tp_array_compar_ascending)
-	return a->inf->num < b->inf->num;
-      else
-	return a->inf->num > b->inf->num;
-    }
 
-  if (tp_array_compar_ascending)
+  return (a->per_inf_num < b->per_inf_num);
-    return (a->per_inf_num < b->per_inf_num);
+}
-  else
+
-    return (a->per_inf_num > b->per_inf_num);
+/* Sort an array of struct thread_info pointers by thread ID (first by
+   inferior number, and then by per-inferior thread number).  Sorts in
+   descending order.  */
+
+static bool
+tp_array_compar_descending (const thread_info *a, const thread_info *b)
+{
+  if (a->inf->num != b->inf->num)
+    return a->inf->num > b->inf->num;
+
+  return (a->per_inf_num > b->per_inf_num);
 }
 
 /* Switch to thread THR and execute CMD.
@@ -1490,6 +1491,60 @@ thr_try_catch_cmd (thread_info *thr, const char *cmd, int from_tty,
     }
 }
 
+/* Option definition of "thread apply"'s "-ascending" option.  */
+
+static const gdb::option::flag_option_def<> ascending_option_def = {
+  "ascending",
+  N_("\
+Call COMMAND for all threads in ascending order.\n\
+The default is descending order."),
+};
+
+/* The qcs command line flags for the "thread apply" commands.  Keep
+   this in sync with the "frame apply" commands.  */
+
+using qcs_flag_option_def
+  = gdb::option::flag_option_def<qcs_flags>;
+
+static const gdb::option::option_def thr_qcs_flags_option_defs[] = {
+  qcs_flag_option_def {
+    "q", [] (qcs_flags *opt) { return &opt->quiet; },
+    N_("Disables printing the thread information."),
+  },
+
+  qcs_flag_option_def {
+    "c", [] (qcs_flags *opt) { return &opt->cont; },
+    N_("Print any error raised by COMMAND and continue."),
+  },
+
+  qcs_flag_option_def {
+    "s", [] (qcs_flags *opt) { return &opt->silent; },
+    N_("Silently ignore any errors or empty output produced by COMMAND."),
+  },
+};
+
+/* Create an option_def_group for the "thread apply all" options, with
+   ASCENDING and FLAGS as context.  */
+
+static inline std::array<gdb::option::option_def_group, 2>
+make_thread_apply_all_options_def_group (int *ascending,
+					 qcs_flags *flags)
+{
+  return {{
+    { ascending_option_def.def (), ascending},
+    { thr_qcs_flags_option_defs, flags },
+  }};
+}
+
+/* Create an option_def_group for the "thread apply" options, with
+   FLAGS as context.  */
+
+static inline gdb::option::option_def_group
+make_thread_apply_options_def_group (qcs_flags *flags)
+{
+  return {thr_qcs_flags_option_defs, flags};
+}
+
 /* Apply a GDB command to a list of threads.  List syntax is a whitespace
    separated list of numbers, or ranges, or the keyword `all'.  Ranges consist
    of two numbers separated by a hyphen.  Examples:
@@ -1501,24 +1556,15 @@ thr_try_catch_cmd (thread_info *thr, const char *cmd, int from_tty,
 static void
 thread_apply_all_command (const char *cmd, int from_tty)
 {
+  int ascending = false;
   qcs_flags flags;
 
-  tp_array_compar_ascending = false;
+  auto group = make_thread_apply_all_options_def_group (&ascending,
+							&flags);
+  gdb::option::process_options
+    (&cmd, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group);
 
-  while (cmd != NULL)
+  validate_flags_qcs ("thread apply all", &flags);
-    {
-      if (check_for_argument (&cmd, "-ascending", strlen ("-ascending")))
-	{
-	  cmd = skip_spaces (cmd);
-	  tp_array_compar_ascending = true;
-	  continue;
-	}
-
-      if (parse_flags_qcs ("thread apply all", &cmd, &flags))
-	continue;
-
-      break;
-    }
 
   if (cmd == NULL || *cmd == '\000')
     error (_("Please specify a command at the end of 'thread apply all'"));
@@ -1544,7 +1590,10 @@ thread_apply_all_command (const char *cmd, int from_tty)
 	 exit.  */
       scoped_inc_dec_ref inc_dec_ref (thr_list_cpy);
 
-      std::sort (thr_list_cpy.begin (), thr_list_cpy.end (), tp_array_compar);
+      auto *sorter = (ascending
+		      ? tp_array_compar_ascending
+		      : tp_array_compar_descending);
+      std::sort (thr_list_cpy.begin (), thr_list_cpy.end (), sorter);
 
       scoped_restore_current_thread restore_thread;
 
@@ -1554,6 +1603,81 @@ thread_apply_all_command (const char *cmd, int from_tty)
     }
 }
 
+/* Completer for "thread apply [ID list]".  */
+
+static void
+thread_apply_command_completer (cmd_list_element *ignore,
+				completion_tracker &tracker,
+				const char *text, const char * /*word*/)
+{
+  /* Don't leave this to complete_options because there's an early
+     return below.  */
+  tracker.set_use_custom_word_point (true);
+
+  tid_range_parser parser;
+  parser.init (text, current_inferior ()->num);
+
+  try
+    {
+      while (!parser.finished ())
+	{
+	  int inf_num, thr_start, thr_end;
+
+	  if (!parser.get_tid_range (&inf_num, &thr_start, &thr_end))
+	    break;
+
+	  if (parser.in_star_range () || parser.in_thread_range ())
+	    parser.skip_range ();
+	}
+    }
+  catch (const gdb_exception_error &ex)
+    {
+      /* get_tid_range throws if it parses a negative number, for
+	 example.  But a seemingly negative number may be the start of
+	 an option instead.  */
+    }
+
+  const char *cmd = parser.cur_tok ();
+
+  if (cmd == text)
+    {
+      /* No thread ID list yet.  */
+      return;
+    }
+
+  /* Check if we're past a valid thread ID list already.  */
+  if (parser.finished ()
+      && cmd > text && !isspace (cmd[-1]))
+    return;
+
+  /* We're past the thread ID list, advance word point.  */
+  tracker.advance_custom_word_point_by (cmd - text);
+  text = cmd;
+
+  const auto group = make_thread_apply_options_def_group (nullptr);
+  if (gdb::option::complete_options
+      (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group))
+    return;
+
+  complete_nested_command_line (tracker, text);
+}
+
+/* Completer for "thread apply all".  */
+
+static void
+thread_apply_all_command_completer (cmd_list_element *ignore,
+				    completion_tracker &tracker,
+				    const char *text, const char *word)
+{
+  const auto group = make_thread_apply_all_options_def_group (nullptr,
+							      nullptr);
+  if (gdb::option::complete_options
+      (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group))
+    return;
+
+  complete_nested_command_line (tracker, text);
+}
+
 /* Implementation of the "thread apply" command.  */
 
 static void
@@ -1577,8 +1701,11 @@ thread_apply_command (const char *tidlist, int from_tty)
 
   cmd = parser.cur_tok ();
 
-  while (parse_flags_qcs ("thread apply", &cmd, &flags))
+  auto group = make_thread_apply_options_def_group (&flags);
-    ;
+  gdb::option::process_options
+    (&cmd, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group);
+
+  validate_flags_qcs ("thread apply", &flags);
 
   if (*cmd == '\0')
     error (_("Please specify a command following the thread ID list"));
@@ -1953,37 +2080,52 @@ Use this command to switch between threads.\n\
 The new thread ID must be currently known."),
 		  &thread_cmd_list, "thread ", 1, &cmdlist);
 
-#define THREAD_APPLY_FLAGS_HELP "\
+#define THREAD_APPLY_OPTION_HELP "\
 Prints per-inferior thread number and target system's thread id\n\
 followed by COMMAND output.\n\
-FLAG arguments are -q (quiet), -c (continue), -s (silent).\n\
+\n\
-Flag -q disables printing the thread information.\n\
+By default, an error raised during the execution of COMMAND\n\
-By default, if a COMMAND raises an error, thread apply is aborted.\n\
+aborts \"thread apply\".\n\
-Flag -c indicates to print the error and continue.\n\
+\n\
-Flag -s indicates to silently ignore a COMMAND that raises an error\n\
+Options:\n\
-or produces no output."
+%OPTIONS%"
 
-  add_prefix_cmd ("apply", class_run, thread_apply_command,
+  const auto thread_apply_opts = make_thread_apply_options_def_group (nullptr);
-		  _("Apply a command to a list of threads.\n\
+
-Usage: thread apply ID... [FLAG]... COMMAND\n\
+  static std::string thread_apply_help = gdb::option::build_help (N_("\
+Apply a command to a list of threads.\n\
+Usage: thread apply ID... [OPTION]... COMMAND\n\
 ID is a space-separated list of IDs of threads to apply COMMAND on.\n"
-THREAD_APPLY_FLAGS_HELP),
+THREAD_APPLY_OPTION_HELP),
-		  &thread_apply_list, "thread apply ", 1, &thread_cmd_list);
+			       thread_apply_opts);
 
-  add_cmd ("all", class_run, thread_apply_all_command,
+  c = add_prefix_cmd ("apply", class_run, thread_apply_command,
-	   _("\
+		      thread_apply_help.c_str (),
+		      &thread_apply_list, "thread apply ", 1,
+		      &thread_cmd_list);
+  set_cmd_completer_handle_brkchars (c, thread_apply_command_completer);
+
+  const auto thread_apply_all_opts
+    = make_thread_apply_all_options_def_group (nullptr, nullptr);
+
+  static std::string thread_apply_all_help = gdb::option::build_help (N_("\
 Apply a command to all threads.\n\
 \n\
-Usage: thread apply all [-ascending] [FLAG]... COMMAND\n\
+Usage: thread apply all [OPTION]... COMMAND\n"
--ascending: Call COMMAND for all threads in ascending order.\n\
+THREAD_APPLY_OPTION_HELP),
-            The default is descending order.\n"
+			       thread_apply_all_opts);
-THREAD_APPLY_FLAGS_HELP),
-	   &thread_apply_list);
 
-  add_com ("taas", class_run, taas_command, _("\
+  c = add_cmd ("all", class_run, thread_apply_all_command,
+	       thread_apply_all_help.c_str (),
+	       &thread_apply_list);
+  set_cmd_completer_handle_brkchars (c, thread_apply_all_command_completer);
+
+  c = add_com ("taas", class_run, taas_command, _("\
 Apply a command to all threads (ignoring errors and empty output).\n\
-Usage: taas COMMAND\n\
+Usage: taas [OPTION]... COMMAND\n\
-shortcut for 'thread apply all -s COMMAND'"));
+shortcut for 'thread apply all -s [OPTION]... COMMAND'\n\
+See \"help thread apply all\" for available options."));
+  set_cmd_completer_handle_brkchars (c, thread_apply_all_command_completer);
 
   c = add_com ("tfaas", class_run, tfaas_command, _("\
 Apply a command to all frames of all threads (ignoring errors and empty output).\n\

gdb/tid-parse.c

@@ -307,6 +307,12 @@ tid_range_parser::in_star_range () const
   return m_state == STATE_STAR_RANGE;
 }
 
+bool
+tid_range_parser::in_thread_range () const
+{
+  return m_state == STATE_THREAD_RANGE;
+}
+
 /* See tid-parse.h.  */
 
 int

gdb/tid-parse.h

@@ -114,6 +114,9 @@ public:
      range.  */
   bool in_star_range () const;
 
+  /* Returns true if processing a thread range (e.g., 1.2-3).  */
+  bool in_thread_range () const;
+
   /* Returns true if parsing has completed.  */
   bool finished () const;