[PATCH 0/2] Make use of gdb::options for info variabels|functions|args|locals

[PATCH 0/2] Make use of gdb::options for info variabels|functions|args|locals

[Andrew Burgess]

Additional use of the gdb::options framework.

--

Andrew Burgess (2):
  gdb: Allow quoting around string options in the gdb::option framework
  gdb: Make use of gdb::option framework for some info commands

 gdb/ChangeLog                      |  23 +++++++
 gdb/cli/cli-option.c               |   5 +-
 gdb/cli/cli-utils.c                | 134 ++++++++++---------------------------
 gdb/cli/cli-utils.h                |  38 ++++++-----
 gdb/gdbsupport/common-utils.c      |  59 ++++++++++++++++
 gdb/gdbsupport/common-utils.h      |  10 +++
 gdb/stack.c                        |  38 +++--------
 gdb/symtab.c                       |  44 ++++--------
 gdb/testsuite/ChangeLog            |   6 ++
 gdb/testsuite/gdb.base/options.exp |  54 ++++++++++-----
 10 files changed, 220 insertions(+), 191 deletions(-)

-- 
2.14.5

[PATCH 1/2] gdb: Allow quoting around string options in the gdb::option framework

[Andrew Burgess]

Currently string options must be a single string with no whitespace,
this limitation prevents the gdb::option framework being used in some
places.

After this commit, string options can be quoted in single or double
quotes, and quote characters can be escaped with a backslash if needed
to either place them within quotes, or to avoid starting a quoted
argument.

This test adds a new function extract_string_maybe_quoted which is
basically a copy of extract_arg_maybe_quoted from cli/cli-utils.c,
however, the cli-utils.c function will be deleted in the next commit.

There are tests to exercise the new quoting mechanism.

gdb/ChangeLog:

	* cli/cli-option.c (parse_option): Use extract_string_maybe_quoted
	to extract string arguments.
	* common/common-utils.c (extract_string_maybe_quoted): New function.
	* common/common-utils.h (extract_string_maybe_quoted): Declare.

gdb/testsuite/ChangeLog:

	* gdb.base/options.exp (expect_string): Dequote strings in
	results.
	(test-string): Test strings with different quoting and reindent.
---
 gdb/ChangeLog                      |  7 +++++
 gdb/cli/cli-option.c               |  5 ++--
 gdb/gdbsupport/common-utils.c      | 59 ++++++++++++++++++++++++++++++++++++++
 gdb/gdbsupport/common-utils.h      | 10 +++++++
 gdb/testsuite/ChangeLog            |  6 ++++
 gdb/testsuite/gdb.base/options.exp | 54 +++++++++++++++++++++++-----------
 6 files changed, 121 insertions(+), 20 deletions(-)

diff --git a/gdb/cli/cli-option.c b/gdb/cli/cli-option.c
index 07d552b7f5b..eb8ef79d4f3 100644
--- a/gdb/cli/cli-option.c
+++ b/gdb/cli/cli-option.c
@@ -434,13 +434,12 @@ parse_option (gdb::array_view<const option_def_group> options_group,
 	  }
 
 	const char *arg_start = *args;
-	*args = skip_to_space (*args);
-
+	std::string str = extract_string_maybe_quoted (args);
 	if (*args == arg_start)
 	  error (_("-%s requires an argument"), match->name);
 
 	option_value val;
-	val.string = savestring (arg_start, *args - arg_start);
+	val.string = xstrdup (str.c_str ());
 	return option_def_and_value {*match, match_ctx, val};
       }
 
diff --git a/gdb/gdbsupport/common-utils.c b/gdb/gdbsupport/common-utils.c
index 384029db70a..d1059de0b33 100644
--- a/gdb/gdbsupport/common-utils.c
+++ b/gdb/gdbsupport/common-utils.c
@@ -160,6 +160,65 @@ savestring (const char *ptr, size_t len)
   return p;
 }
 
+/* See documentation in common-utils.h.  */
+
+std::string
+extract_string_maybe_quoted (const char **arg)
+{
+  bool squote = false;
+  bool dquote = false;
+  bool bsquote = false;
+  std::string result;
+  const char *p = *arg;
+
+  /* Find the start of the argument.  */
+  p = skip_spaces (p);
+
+  /* Parse p similarly to gdb_argv buildargv function.  */
+  while (*p != '\0')
+    {
+      if (isspace (*p) && !squote && !dquote && !bsquote)
+	break;
+      else
+	{
+	  if (bsquote)
+	    {
+	      bsquote = false;
+	      result += *p;
+	    }
+	  else if (*p == '\\')
+	    bsquote = true;
+	  else if (squote)
+	    {
+	      if (*p == '\'')
+		squote = false;
+	      else
+		result += *p;
+	    }
+	  else if (dquote)
+	    {
+	      if (*p == '"')
+		dquote = false;
+	      else
+		result += *p;
+	    }
+	  else
+	    {
+	      if (*p == '\'')
+		squote = true;
+	      else if (*p == '"')
+		dquote = true;
+	      else
+		result += *p;
+	    }
+	  p++;
+	}
+    }
+
+  *arg = p;
+  return result;
+}
+
 /* The bit offset of the highest byte in a ULONGEST, for overflow
    checking.  */
 
diff --git a/gdb/gdbsupport/common-utils.h b/gdb/gdbsupport/common-utils.h
index 52bf3437b1c..a5312cb0c49 100644
--- a/gdb/gdbsupport/common-utils.h
+++ b/gdb/gdbsupport/common-utils.h
@@ -94,6 +94,16 @@ void string_vappendf (std::string &dest, const char* fmt, va_list args)
 
 char *savestring (const char *ptr, size_t len);
 
+/* Extract the next word from ARG.  The next word is defined as either,
+   everything up to the next space, or, if the next word starts with either
+   a single or double quote, then everything up to the closing quote.  The
+   enclosing quotes are not returned in the result string.  The pointer in
+   ARG is updated to point to the first character after the end of the
+   word, or, for quoted words, the first character after the closing
+   quote.  */
+
+std::string extract_string_maybe_quoted (const char **arg);
+
 /* The strerror() function can return NULL for errno values that are
    out of range.  Provide a "safe" version that always returns a
    printable string.  */
diff --git a/gdb/testsuite/gdb.base/options.exp b/gdb/testsuite/gdb.base/options.exp
index e8f571d9ba9..3495a0142fb 100644
--- a/gdb/testsuite/gdb.base/options.exp
+++ b/gdb/testsuite/gdb.base/options.exp
@@ -128,6 +128,11 @@ proc expect_integer {option val operand} {
 # test-options xxx", with -string set to $STR.  OPERAND is the
 # expected operand.
 proc expect_string {str operand} {
+    # Dequote the string in the expected output.
+    if { ( [string range $str 0 0] == "\"" && [string range $str end end] == "\"") \
+	     || ([string range $str 0 0] == "'" && [string range $str end end] == "'")} {
+	set str [string range $str 1 end-1]
+    }
     return "-flag 0 -xx1 0 -xx2 0 -bool 0 -enum xxx -uint 0 -zuint-unl 0 -string '$str' -- $operand"
 }
 
@@ -967,26 +972,41 @@ proc_with_prefix test-string {variant} {
 	    "-string requires an argument"
     }
 
-    res_test_gdb_complete_none \
-	"1 [expect_none ""]" \
-	"$cmd -string STR"
-    gdb_test "$cmd -string STR --" [expect_string "STR" ""]
+    foreach_with_prefix str {
+	"STR"
+	"\"STR\""
+	"\\\"STR"
+	"'STR'"
+	"\\'STR"
+	"\"STR AAA\""
+	"'STR BBB'"
+	"\"STR 'CCC' DDD\""
+	"'STR \"EEE\" FFF'"
+	"\"STR \\\"GGG\\\" HHH\""
+	"'STR \\\'III\\\' JJJ'"
+    } {
+	res_test_gdb_complete_none \
+	    "1 [expect_none ""]" \
+	    "$cmd -string ${str}"
+	gdb_test "$cmd -string ${str} --" [expect_string "${str}" ""]
 
-    # Completing at "-" after parsing STR should list all options.
-    res_test_gdb_complete_multiple \
-	"1 [expect_string "STR" "-"]" \
-	"$cmd -string STR " "-" "" $all_options
+	# Completing at "-" after parsing STR should list all options.
+	res_test_gdb_complete_multiple \
+	    "1 [expect_string "${str}" "-"]" \
+	    "$cmd -string ${str} " "-" "" $all_options
 
-    # Check that only FOO is considered part of the string's value.
-    # I.e., that we stop parsing the string at the first whitespace.
-    if {$variant == "require-delimiter"} {
-	res_test_gdb_complete_none \
-	    "1 [expect_string "FOO" "BAR"]" \
-	    "$cmd -string FOO BAR"
-    } else {
-	res_test_gdb_complete_none "0 BAR" "$cmd -string FOO BAR"
+	# Check that only $STR is considered part of the string's value.
+	# I.e., that we stop parsing the string at the first
+	# whitespace or after the closing quote of $STR.
+	if {$variant == "require-delimiter"} {
+	    res_test_gdb_complete_none \
+		"1 [expect_string "${str}" "BAR"]" \
+		"$cmd -string ${str} BAR"
+	} else {
+	    res_test_gdb_complete_none "0 BAR" "$cmd -string ${str} BAR"
+	}
+	gdb_test "$cmd -string ${str} BAR --" "Unrecognized option at: BAR --"
     }
-    gdb_test "$cmd -string FOO BAR --" "Unrecognized option at: BAR --"
 }
 
 # Run the options framework tests first.
-- 
2.14.5

[PATCH 2/2] gdb: Make use of gdb::option framework for some info commands

[Andrew Burgess]

Update the 'info variables', 'info functions', 'info locals', and
'info args' commands to make use of the gdb::options framework.

There should be no user visible changes after this commit as I have
left the help text generation using the existing mechanism, which
already tries to customise the text for each of the commands.

gdb/ChangeLog:

	* cli/cli-utils.c (extract_info_print_args): Delete.
	(extract_arg_maybe_quoted): Delete.
	(info_print_options_defs): New variable.
	(make_info_print_options_def_group): New function.
	(extract_info_print_options): Define new function.
	* cli/cli-utils.h (extract_info_print_args): Delete.
	(struct info_print_options): New structure.
	(extract_info_print_options): Declare new function.
	* stack.c (info_locals_command): Update to use new
	extract_info_print_options, also add a header comment.
	(info_args_command): Likewise.
	* symtab.c (info_variables_command): Likewise.
	(info_functions_command): Likewise.
---
 gdb/ChangeLog       |  16 +++++++
 gdb/cli/cli-utils.c | 134 +++++++++++++++-------------------------------------
 gdb/cli/cli-utils.h |  38 ++++++++-------
 gdb/stack.c         |  38 ++++-----------
 gdb/symtab.c        |  44 ++++++-----------
 5 files changed, 99 insertions(+), 171 deletions(-)

diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index 333a86a81b9..cd3dfe65a2b 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -23,8 +23,6 @@
 
 #include <ctype.h>
 
-static std::string extract_arg_maybe_quoted (const char **arg);
-
 /* See documentation in cli-utils.h.  */
 
 ULONGEST
@@ -182,38 +180,6 @@ get_number (char **pp)
 
 /* See documentation in cli-utils.h.  */
 
-bool
-extract_info_print_args (const char **args,
-			 bool *quiet,
-			 std::string *regexp,
-			 std::string *t_regexp)
-{
-  /* Check for NAMEREGEXP or -- NAMEREGEXP.  */
-  if (**args != '-' || check_for_argument (args, "--", 2))
-    {
-      *regexp = *args;
-      *args = NULL;
-      return true;
-    }
-
-  if (check_for_argument (args, "-t", 2))
-    {
-      *t_regexp = extract_arg_maybe_quoted (args);
-      *args = skip_spaces (*args);
-      return true;
-    }
-
-  if (check_for_argument (args, "-q", 2))
-    {
-      *quiet = true;
-      return true;
-    }
-
-  return false;
-}
-
-/* See documentation in cli-utils.h.  */
-
 void
 report_unrecognized_option_error (const char *command, const char *args)
 {
@@ -407,69 +373,6 @@ remove_trailing_whitespace (const char *start, const char *s)
   return s;
 }
 
-/* A helper function to extract an argument from *ARG.  An argument is
-   delimited by whitespace, but it can also be optionally quoted.
-   The quoting and special characters are handled similarly to
-   the parsing done by gdb_argv.
-   The return value is empty if no argument was found.  */
-
-static std::string
-extract_arg_maybe_quoted (const char **arg)
-{
-  bool squote = false;
-  bool dquote = false;
-  bool bsquote = false;
-  std::string result;
-  const char *p = *arg;
-
-  /* Find the start of the argument.  */
-  p = skip_spaces (p);
-
-  /* Parse p similarly to gdb_argv buildargv function.  */
-  while (*p != '\0')
-    {
-      if (isspace (*p) && !squote && !dquote && !bsquote)
-	  break;
-      else
-	{
-	  if (bsquote)
-	    {
-	      bsquote = false;
-	      result += *p;
-	    }
-	  else if (*p == '\\')
-	      bsquote = true;
-	  else if (squote)
-	    {
-	      if (*p == '\'')
-		  squote = false;
-	      else
-		  result += *p;
-	    }
-	  else if (dquote)
-	    {
-	      if (*p == '"')
-		  dquote = false;
-	      else
-		  result += *p;
-	    }
-	  else
-	    {
-	      if (*p == '\'')
-		  squote = true;
-	      else if (*p == '"')
-		  dquote = true;
-	      else
-		  result += *p;
-	    }
-	  p++;
-	}
-    }
-
-  *arg = p;
-  return result;
-}
-
 /* See documentation in cli-utils.h.  */
 
 std::string
@@ -532,4 +435,41 @@ validate_flags_qcs (const char *which_command, qcs_flags *flags)
     error (_("%s: -c and -s are mutually exclusive"), which_command);
 }
 
+/* The options used by the 'info variables' commands and similar.  */
+
+static const gdb::option::option_def info_print_options_defs[] = {
+  gdb::option::boolean_option_def<info_print_options> {
+    "q",
+    [] (info_print_options *opt) { return &opt->quiet; },
+    nullptr, /* show_cmd_cb */
+    nullptr /* set_doc */
+  },
+
+  gdb::option::string_option_def<info_print_options> {
+    "t",
+    [] (info_print_options *opt) { return &opt->type_regexp; },
+    nullptr, /* show_cmd_cb */
+    nullptr /* set_doc */
+  }
+};
+
+/* Returns the option group used by 'info variables' and similar.  */
+
+static gdb::option::option_def_group
+make_info_print_options_def_group (info_print_options *opts)
+{
+  return {{info_print_options_defs}, opts};
+}
+
 /* See documentation in cli-utils.h.  */
+
+void
+extract_info_print_options (info_print_options *opts,
+			    const char **args)
+{
+  auto grp = make_info_print_options_def_group (opts);
+  gdb::option::process_options
+    (args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, grp);
+  if (*args != nullptr && **args == '\0')
+    *args = nullptr;
+}
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index c2a0f374a6e..a3826be6824 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -43,22 +43,28 @@ extern int get_number (char **);
    error instead of returning 0.  */
 extern ULONGEST get_ulongest (const char **pp, int trailer = '\0');
 
-/* Extract from ARGS the arguments [-q] [-t TYPEREGEXP] [--] NAMEREGEXP.
-
-   The caller is responsible to initialize *QUIET to false, *REGEXP
-   and *T_REGEXP to "".
-   extract_info_print_args can then be called iteratively to search
-   for valid arguments, as part of a 'main parsing loop' searching for
-   -q/-t/-- arguments together with other flags and options.
-
-   Returns true and updates *ARGS + one of *QUIET, *REGEXP, *T_REGEXP if
-   it finds a valid argument.
-   Returns false if no valid argument is found at the beginning of ARGS.  */
-
-extern bool extract_info_print_args (const char **args,
-				     bool *quiet,
-				     std::string *regexp,
-				     std::string *t_regexp);
+/* Structure to hold the values of the options used by the 'info
+   variables' command and other similar commands.  These correspond to the
+   -q and -t options.  */
+
+struct info_print_options
+{
+  int quiet = false;
+  char *type_regexp = nullptr;
+
+  ~info_print_options ()
+  {
+    xfree (type_regexp);
+  }
+};
+
+/* Extract options from ARGS for commands like 'info variables', placing
+   the options into OPTS.  ARGS is updated to point to the first character
+   after the options, or, if there is nothing after the options, then ARGS
+   is set to nullptr.  */
+
+extern void extract_info_print_options (info_print_options *opts,
+					const char **args);
 
 /* Throws an error telling the user that ARGS starts with an option
    unrecognized by COMMAND.  */
diff --git a/gdb/stack.c b/gdb/stack.c
index f7df7a40d10..175f2116a5b 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2359,24 +2359,16 @@ print_frame_local_vars (struct frame_info *frame,
     }
 }
 
+/* Implement the 'info locals' command.  */
+
 void
 info_locals_command (const char *args, int from_tty)
 {
-  std::string regexp;
-  std::string t_regexp;
-  bool quiet = false;
-
-  while (args != NULL
-	 && extract_info_print_args (&args, &quiet, &regexp, &t_regexp))
-    ;
-
-  if (args != NULL)
-    report_unrecognized_option_error ("info locals", args);
+  info_print_options opts;
+  extract_info_print_options (&opts, &args);
 
   print_frame_local_vars (get_selected_frame (_("No frame selected.")),
-			  quiet,
-			  regexp.empty () ? NULL : regexp.c_str (),
-			  t_regexp.empty () ? NULL : t_regexp.c_str (),
+			  opts.quiet, args, opts.type_regexp,
 			  0, gdb_stdout);
 }
 
@@ -2474,26 +2466,16 @@ print_frame_arg_vars (struct frame_info *frame,
     }
 }
 
+/* Implement the 'info args' command.  */
+
 void
 info_args_command (const char *args, int from_tty)
 {
-  std::string regexp;
-  std::string t_regexp;
-  bool quiet = false;
-
-  while (args != NULL
-	 && extract_info_print_args (&args, &quiet, &regexp, &t_regexp))
-    ;
-
-  if (args != NULL)
-    report_unrecognized_option_error ("info args", args);
-
+  info_print_options opts;
+  extract_info_print_options (&opts, &args);
 
   print_frame_arg_vars (get_selected_frame (_("No frame selected.")),
-			quiet,
-			regexp.empty () ? NULL : regexp.c_str (),
-			t_regexp.empty () ? NULL : t_regexp.c_str (),
-			gdb_stdout);
+			opts.quiet, args, opts.type_regexp, gdb_stdout);
 }
 \f
 /* Return the symbol-block in which the selected frame is executing.
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 01118c62555..46691122187 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4687,6 +4687,9 @@ symtab_symbol_info (bool quiet,
 
   gdb_assert (kind <= TYPES_DOMAIN);
 
+  if (regexp != nullptr && *regexp == '\0')
+    regexp = nullptr;
+
   /* Must make sure that if we're interrupted, symbols gets freed.  */
   std::vector<symbol_search> symbols = search_symbols (regexp, kind,
 						       t_regexp, 0, NULL);
@@ -4742,47 +4745,28 @@ symtab_symbol_info (bool quiet,
     }
 }
 
+/* Implement the 'info variables' command.  */
+
 static void
 info_variables_command (const char *args, int from_tty)
 {
-  std::string regexp;
-  std::string t_regexp;
-  bool quiet = false;
-
-  while (args != NULL
-	 && extract_info_print_args (&args, &quiet, &regexp, &t_regexp))
-    ;
+  info_print_options opts;
+  extract_info_print_options (&opts, &args);
 
-  if (args != NULL)
-    report_unrecognized_option_error ("info variables", args);
-
-  symtab_symbol_info (quiet,
-		      regexp.empty () ? NULL : regexp.c_str (),
-		      VARIABLES_DOMAIN,
-		      t_regexp.empty () ? NULL : t_regexp.c_str (),
-		      from_tty);
+  symtab_symbol_info (opts.quiet, args, VARIABLES_DOMAIN,
+		      opts.type_regexp, from_tty);
 }
 
+/* Implement the 'info functions' command.  */
 
 static void
 info_functions_command (const char *args, int from_tty)
 {
-  std::string regexp;
-  std::string t_regexp;
-  bool quiet = false;
-
-  while (args != NULL
-	 && extract_info_print_args (&args, &quiet, &regexp, &t_regexp))
-    ;
-
-  if (args != NULL)
-    report_unrecognized_option_error ("info functions", args);
+  info_print_options opts;
+  extract_info_print_options (&opts, &args);
 
-  symtab_symbol_info (quiet,
-		      regexp.empty () ? NULL : regexp.c_str (),
-		      FUNCTIONS_DOMAIN,
-		      t_regexp.empty () ? NULL : t_regexp.c_str (),
-		      from_tty);
+  symtab_symbol_info (opts.quiet, args, FUNCTIONS_DOMAIN,
+		      opts.type_regexp, from_tty);
 }
 
 
-- 
2.14.5

Re: [PATCH 0/2] Make use of gdb::options for info variabels|functions|args|locals

[Pedro Alves]

On 7/11/19 2:20 PM, Andrew Burgess wrote:
> Additional use of the gdb::options framework.
> 
> --
> 
> Andrew Burgess (2):
>   gdb: Allow quoting around string options in the gdb::option framework

Ahaha, that didn't take long.  Thanks for doing this.  LGTM.

>   gdb: Make use of gdb::option framework for some info commands

This LGTM to me too, but, I was surprised to find this doesn't add
completers at the same time?

Thanks,
Pedro Alves

Re: [PATCH 0/2] Make use of gdb::options for info variabels|functions|args|locals

[Andrew Burgess]

* Pedro Alves <palves@redhat.com> [2019-07-11 14:36:23 +0100]:

> On 7/11/19 2:20 PM, Andrew Burgess wrote:
> > Additional use of the gdb::options framework.
> > 
> > --
> > 
> > Andrew Burgess (2):
> >   gdb: Allow quoting around string options in the gdb::option framework
> 
> Ahaha, that didn't take long.  Thanks for doing this.  LGTM.
> 
> >   gdb: Make use of gdb::option framework for some info commands
> 
> This LGTM to me too, but, I was surprised to find this doesn't add
> completers at the same time?

Something like this, maybe?

Thanks,
Andrew

---

[PATCH] gdb: Add command completers for some info commands

Add command completion for info variables, functions, args, and
locals.  This completer only completes the command line options as
these commands all take a regexp which GDB can't really offer
completions for.

gdb/ChangeLog:

	* cli/cli-utils.c (info_print_command_completer): New function.
	* cli/cli-utils.h: Add 'completer.h' include, and forward
	declaration for 'struct cmd_list_element'.
	(info_print_command_completer): Declare.
	* stack.c (_initialize_stack): Add completer for 'info locals' and
	'info args'.
	* symtab.c (_initialize_symtab): Add completer for 'info
	variables' and 'info functions'.
---
 gdb/ChangeLog       | 11 +++++++++++
 gdb/cli/cli-utils.c | 19 +++++++++++++++++++
 gdb/cli/cli-utils.h | 13 +++++++++++++
 gdb/stack.c         | 10 ++++++----
 gdb/symtab.c        | 19 +++++++++++++------
 5 files changed, 62 insertions(+), 10 deletions(-)

diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index cd3dfe65a2b..be830ee9f9d 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -473,3 +473,22 @@ extract_info_print_options (info_print_options *opts,
   if (*args != nullptr && **args == '\0')
     *args = nullptr;
 }
+
+/* See documentation in cli-utils.h.  */
+
+void
+info_print_command_completer (struct cmd_list_element *ignore,
+			      completion_tracker &tracker,
+			      const char *text, const char * /* word */)
+{
+  const auto group
+    = make_info_print_options_def_group (nullptr);
+  if (gdb::option::complete_options
+      (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group))
+    return;
+
+  const char *word = advance_to_expression_complete_word_point (tracker, text);
+  symbol_completer (ignore, tracker, text, word);
+  return;
+}
+
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index a3826be6824..17cdd842b2f 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -20,6 +20,10 @@
 #ifndef CLI_CLI_UTILS_H
 #define CLI_CLI_UTILS_H
 
+#include "completer.h"
+
+struct cmd_list_element;
+
 /* *PP is a string denoting a number.  Get the number.  Advance *PP
    after the string and any trailing whitespace.
 
@@ -66,6 +70,15 @@ struct info_print_options
 extern void extract_info_print_options (info_print_options *opts,
 					const char **args);
 
+/* Function that can be used as a command completer for 'info variable'
+   and friends.  This offers command option completion as well as symbol
+   completion.  At the moment all symbols are offered for all commands.  */
+
+extern void info_print_command_completer (struct cmd_list_element *ignore,
+					  completion_tracker &tracker,
+					  const char *text,
+					  const char * /* word */);
+
 /* Throws an error telling the user that ARGS starts with an option
    unrecognized by COMMAND.  */
 
diff --git a/gdb/stack.c b/gdb/stack.c
index 175f2116a5b..9b1d1a68568 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -3423,18 +3423,20 @@ Print information about a stack frame selected by level.\n\
 Usage: info frame level LEVEL"),
 	   &info_frame_cmd_list);
 
-  add_info ("locals", info_locals_command,
-	    info_print_args_help (_("\
+  cmd = add_info ("locals", info_locals_command,
+		  info_print_args_help (_("\
 All local variables of current stack frame or those matching REGEXPs.\n\
 Usage: info locals [-q] [-t TYPEREGEXP] [NAMEREGEXP]\n\
 Prints the local variables of the current stack frame.\n"),
 				  _("local variables")));
-  add_info ("args", info_args_command,
-	    info_print_args_help (_("\
+  set_cmd_completer_handle_brkchars (cmd, info_print_command_completer);
+  cmd = add_info ("args", info_args_command,
+		  info_print_args_help (_("\
 All argument variables of current stack frame or those matching REGEXPs.\n\
 Usage: info args [-q] [-t TYPEREGEXP] [NAMEREGEXP]\n\
 Prints the argument variables of the current stack frame.\n"),
 				  _("argument variables")));
+  set_cmd_completer_handle_brkchars (cmd, info_print_command_completer);
 
   if (dbx_commands)
     add_com ("func", class_stack, func_command, _("\
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 46691122187..41898992c19 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5997,28 +5997,35 @@ symbol_set_symtab (struct symbol *symbol, struct symtab *symtab)
 void
 _initialize_symtab (void)
 {
+  cmd_list_element *c;
+
   initialize_ordinary_address_classes ();
 
-  add_info ("variables", info_variables_command,
-	    info_print_args_help (_("\
+  c = add_info ("variables", info_variables_command,
+		info_print_args_help (_("\
 All global and static variable names or those matching REGEXPs.\n\
 Usage: info variables [-q] [-t TYPEREGEXP] [NAMEREGEXP]\n\
 Prints the global and static variables.\n"),
 				  _("global and static variables")));
+  set_cmd_completer_handle_brkchars (c, info_print_command_completer);
   if (dbx_commands)
-    add_com ("whereis", class_info, info_variables_command,
-	     info_print_args_help (_("\
+    {
+      c = add_com ("whereis", class_info, info_variables_command,
+		   info_print_args_help (_("\
 All global and static variable names, or those matching REGEXPs.\n\
 Usage: whereis [-q] [-t TYPEREGEXP] [NAMEREGEXP]\n\
 Prints the global and static variables.\n"),
 				   _("global and static variables")));
+      set_cmd_completer_handle_brkchars (c, info_print_command_completer);
+    }
 
-  add_info ("functions", info_functions_command,
-	    info_print_args_help (_("\
+  c = add_info ("functions", info_functions_command,
+		info_print_args_help (_("\
 All function names or those matching REGEXPs.\n\
 Usage: info functions [-q] [-t TYPEREGEXP] [NAMEREGEXP]\n\
 Prints the functions.\n"),
 				  _("functions")));
+  set_cmd_completer_handle_brkchars (c, info_print_command_completer);
 
   /* FIXME:  This command has at least the following problems:
      1.  It prints builtin types (in a very strange and confusing fashion).
-- 
2.14.5

Re: [PATCH 0/2] Make use of gdb::options for info variabels|functions|args|locals

[Pedro Alves]

On 7/11/19 4:53 PM, Andrew Burgess wrote:
> * Pedro Alves <palves@redhat.com> [2019-07-11 14:36:23 +0100]:
> 
>> On 7/11/19 2:20 PM, Andrew Burgess wrote:
>>> Additional use of the gdb::options framework.
>>>
>>> --
>>>
>>> Andrew Burgess (2):
>>>   gdb: Allow quoting around string options in the gdb::option framework
>>
>> Ahaha, that didn't take long.  Thanks for doing this.  LGTM.
>>
>>>   gdb: Make use of gdb::option framework for some info commands
>>
>> This LGTM to me too, but, I was surprised to find this doesn't add
>> completers at the same time?
> 
> Something like this, maybe?

_Exactly_ like that.  Perfect.

So far, I've listed this sort of improvement under
"Completion improvements" in NEWS.  I think you lists these commands
there too, and it'd be an obvious change.

> +
> +  const char *word = advance_to_expression_complete_word_point (tracker, text);
> +  symbol_completer (ignore, tracker, text, word);
> +  return;

Redundant "return".

Thanks,
Pedro Alves

Re: [PATCH 0/2] Make use of gdb::options for info variabels|functions|args|locals

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> +void
Andrew> +info_print_command_completer (struct cmd_list_element *ignore,
Andrew> +			      completion_tracker &tracker,
Andrew> +			      const char *text, const char * /* word */)
Andrew> +{
Andrew> +  const auto group
Andrew> +    = make_info_print_options_def_group (nullptr);
Andrew> +  if (gdb::option::complete_options
Andrew> +      (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group))
Andrew> +    return;
Andrew> +
Andrew> +  const char *word = advance_to_expression_complete_word_point (tracker, text);
Andrew> +  symbol_completer (ignore, tracker, text, word);
Andrew> +  return;
Andrew> +}

I didn't really read the patch but this unnecessary return popped out at me.
I think gdb style is not to do this.

Tom

Re: [PATCH 1/2] gdb: Allow quoting around string options in the gdb::option framework

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew>  proc expect_string {str operand} {
Andrew> +    # Dequote the string in the expected output.
Andrew> +    if { ( [string range $str 0 0] == "\"" && [string range $str end end] == "\"") \
Andrew> +	     || ([string range $str 0 0] == "'" && [string range $str end end] == "'")} {

These lines look over-long.
Also the trailing "\" isn't needed here.

Tom

Re: [PATCH 0/2] Make use of gdb::options for info variabels|functions|args|locals

[Andrew Burgess]

* Pedro Alves <palves@redhat.com> [2019-07-11 17:05:18 +0100]:

> On 7/11/19 4:53 PM, Andrew Burgess wrote:
> > * Pedro Alves <palves@redhat.com> [2019-07-11 14:36:23 +0100]:
> > 
> >> On 7/11/19 2:20 PM, Andrew Burgess wrote:
> >>> Additional use of the gdb::options framework.
> >>>
> >>> --
> >>>
> >>> Andrew Burgess (2):
> >>>   gdb: Allow quoting around string options in the gdb::option framework
> >>
> >> Ahaha, that didn't take long.  Thanks for doing this.  LGTM.
> >>
> >>>   gdb: Make use of gdb::option framework for some info commands
> >>
> >> This LGTM to me too, but, I was surprised to find this doesn't add
> >> completers at the same time?
> > 
> > Something like this, maybe?
> 
> _Exactly_ like that.  Perfect.
> 
> So far, I've listed this sort of improvement under
> "Completion improvements" in NEWS.  I think you lists these commands
> there too, and it'd be an obvious change.
> 
> > +
> > +  const char *word = advance_to_expression_complete_word_point (tracker, text);
> > +  symbol_completer (ignore, tracker, text, word);
> > +  return;
> 
> Redundant "return".

Pushed with "return" fixed, and a NEWS entry.

Thanks,
Andrew



> 
> Thanks,
> Pedro Alves

Re: [PATCH 1/2] gdb: Allow quoting around string options in the gdb::option framework

[Andrew Burgess]

* Tom Tromey <tom@tromey.com> [2019-07-11 10:17:43 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew>  proc expect_string {str operand} {
> Andrew> +    # Dequote the string in the expected output.
> Andrew> +    if { ( [string range $str 0 0] == "\"" && [string range $str end end] == "\"") \
> Andrew> +	     || ([string range $str 0 0] == "'" && [string range $str end end] == "'")} {
> 
> These lines look over-long.
> Also the trailing "\" isn't needed here.

Pushed with this fix.

Thanks,
Andrew