Rewrite/enhance explicit locations completer, parse left->right

One of the most annoying (to me) things about GDB's completion is when
you have overloads in your program, and you want to set a breakpoint
in one of them:

 void function(int);  // set breakpoint here.
 void function(long);

 (gdb) b -f func[TAB]
 (gdb) b -f function(       # ok, gdb completed as much as possible.
 (gdb) b -f function([TAB]  # show me the overloads, please.
 <_all_ symbols in the program are shown...>

E.g., when debugging GDB, that'd be:

 (gdb) b -f function([TAB]
 (anonymous namespace)::get_global()::global  pt_insn_get_offset@plt                       scm_new_port_table_entry
 asprintf                                     pt_pkt_alloc_decoder                         scm_new_port_table_entry@plt
 asprintf@plt                                 pt_pkt_alloc_decoder@plt                     scm_out_of_range
 bt_ctf_get_char_array                        pt_pkt_sync_forward                          scm_out_of_range@plt
 bt_ctf_get_char_array@plt                    pt_pkt_sync_forward@plt                      scm_putc
 bt_ctf_get_uint64                            pwrite                                       scm_putc@plt
 bt_ctf_get_uint64@plt                        pwrite@plt                                   scm_reverse_x
 bt_ctf_iter_read_event                       PyErr_Restore                                scm_reverse_x@plt
 bt_ctf_iter_read_event@plt                   PyErr_Restore@plt                            scm_set_port_filename_x
 <snip...>

Now that's a load of completely useless completions.

The reason GDB offers those is that the completer relies on readline
figuring out the completion word point in the input line based on the
language's word break characters, which include "(".  So readline
tells the completer to complete on "", the string that is after '('.
Likewise, if you type "function(i[TAB]" to try to complete to "int",
you're out of luck.  GDB shows you all the symbols in the program that
start with "i"...  This makes sense for the expression completer, as
what you'd want to type is e.g., a global variable, say:

(gdb) print function(i[TAB]

but, it makes no sense when specifying a function name for a
breakpoint location.

To get around that limitation, users need to quote the function name,
like:

 (gdb) b -f 'function([TAB]
 function(int)      function(long)
 (gdb) b 'function(i[TAB]
 (gdb) b 'function(int)' # now completes correctly!

Note that the quoting is only necessary for completion.  Creating the
breakpoint does not require the quoting:

 (gdb) b -f function(int) [RET]
 Breakpoint 1 at ....

This patch removes this limitation.

(
Actually, it's a necessary patch, though not sufficient.  That'll
start working correctly by the end of the series.  With this patch, if try it,
you'll see:

 (gdb) b -f function(i[TAB]
 (gdb) b -f function

i.e., gdb strips everything after the "(".  That's caused by some code
in symtab.c that'll be eliminated further down the series.  These
patches are all unfortunately interrelated, which is also the reason
new tests only appear much later in the series.
But let's ignore that reality for the remainder of the description.
)

So... this patch gets rid of the need for quoting.

It does that by adding a way for a completer to control the exact
completion word point that readline should start the completion
request for, instead of letting readline try to figure it out using
the current language's word break chars array, and often failing.

In the case above, we want the completer to figure out that it's
completing a function name that starts with "function(i".  It now
does.

It took me a while to figure out a way to ask readline to "use this
exact word point", and for a while I feared that it'd be impossible
with current readline (and having to rely on master readline for core
functionality is something I'd like to avoid very much).  Eventually,
after several different attempts, I came up with what is described in
the comment above gdb_custom_word_point_brkchars in the patch.

With this patch, the handle_brkchars phase of the explicit location
completer advances the expected word point as it parses the input line
left to right, until it figures out exactly what we're completing,
instead of expecting readline to break the string using the word break
characters, and then having the completer heuristically fix up a bad
decision by parsing the input string backwards.  This allows correctly
knowning that we're completing a symbol name after -function, complete
functions without quoting, etc.

Later, we'll make use of this same mechanims to implement a proper
linespec completer that avoids need for quoting too.

gdb/ChangeLog:
2017-07-17  Pedro Alves  <palves@redhat.com>

	* ada-lang.c (ada_collect_symbol_completion_matches): Add
	complete_symbol_mode parameter.
	* cli/cli-cmds.c (complete_command): Get the completion result out
	of the handle_brkchars tracker if used a custom word point.
	* completer.c: Include "linespec.h".
	(enum explicit_location_match_type) <MATCH_LINE>: New enumerator.
	(advance_to_expression_complete_word_point): New.
	(completion_tracker::completes_to_completion_word): New.
	(complete_files_symbols): Pass down
	complete_symbol_mode::EXPRESSION.
	(explicit_options, probe_options): New.
	(collect_explicit_location_matches): Complete on the
	explictit_loc->foo instead of word.  Use
	linespec_complete_function.  Handle MATCH_LINE.  Handle offering
	keyword and options completions.
	(backup_text_ptr): Delete.
	(skip_keyword): New.
	(complete_explicit_location): Remove 'word' parameter.  Add
	language, quoted_arg_start and quoted_arg_end parameters.
	Rewrite, parsing left to right.
	(location_completer): Rewrite.
	(location_completer_handle_brkchars): New function.
	(symbol_completer): Pass down complete_symbol_mode::EXPRESSION.
	(enum complete_line_internal_reason): Adjust comments.
	(completion_tracker::discard_completions): New.
	(completer_handle_brkchars_func_for_completer): Handle
	location_completer.
	(gdb_custom_word_point_brkchars)
	(gdb_org_rl_basic_quote_characters): New.
	(gdb_completion_word_break_characters_throw)
	(completion_find_completion_word): Handle trackers that use a
	custom word point.
	(completion_tracker::advance_custom_word_point_by): New.
	(completion_tracker::build_completion_result): Don't rely on
	readline appending the quote char.
	(gdb_rl_attempted_completion_function_throw): Handle trackers that
	use a custom word point.
	(gdb_rl_attempted_completion_function): Restore
	rl_basic_quote_characters.
	* completer.h (class completion_tracker): Extend intro comment.
	(completion_tracker::set_quote_char)
	(completion_tracker::quote_char)
	(completion_tracker::set_use_custom_word_point)
	(completion_tracker::use_custom_word_point)
	(completion_tracker::custom_word_point)
	(completion_tracker::set_custom_word_point)
	(completion_tracker::advance_custom_word_point_by)
	(completion_tracker::completes_to_completion_word)
	(completion_tracker::discard_completions): New methods.
	(completion_tracker::m_quote_char)
	(completion_tracker::m_use_custom_word_point)
	(completion_tracker::m_custom_word_point): New fields.
	(advance_to_expression_complete_word_point): Declare.
	* f-lang.c (f_collect_symbol_completion_matches): Add
	complete_symbol_mode parameter.
	* language.h (struct language_defn)
	<la_collect_symbol_completion_matches>: Add complete_symbol_mode
	parameter.
	* linespec.c (linespec_keywords): Add NULL terminator.  Make extern.
	(linespec_complete_function): New function.
	(linespec_lexer_lex_keyword): Adjust.
	* linespec.h (linespec_keywords, linespec_complete_function): New
	declarations.
	* location.c (find_end_quote): New function.
	(explicit_location_lex_one): Add explicit_completion_info
	parameter.  Save quoting info.  Don't throw if being called for
	completion.  Don't handle Ada operators here.
	(is_cp_operator, skip_op_false_positives, first_of)
	(explicit_location_lex_one_function): New function.
	(string_to_explicit_location): Replace 'dont_throw' parameter with
	an explicit_completion_info pointer parameter.  Handle it.  Don't
	use explicit_location_lex_one to lex function names.  Use
	explicit_location_lex_one_function instead.
	* location.h (struct explicit_completion_info): New.
	(string_to_explicit_location): Replace 'dont_throw' parameter with
	an explicit_completion_info pointer parameter.
	* symtab.c (default_collect_symbol_completion_matches_break_on):
	Add complete_symbol_mode parameter.  Handle LINESPEC mode.
	(default_collect_symbol_completion_matches)
	(collect_symbol_completion_matches): Add complete_symbol_mode
	parameter.
	(collect_symbol_completion_matches_type): Pass down
	complete_symbol_mode::EXPRESSION.
	(collect_file_symbol_completion_matches): Add complete_symbol_mode
	parameter.  Handle LINESPEC mode.
	* symtab.h (complete_symbol_mode): New.
	(default_collect_symbol_completion_matches_break_on)
	(default_collect_symbol_completion_matches)
	(collect_symbol_completion_matches)
	(collect_file_symbol_completion_matches): Add complete_symbol_mode
	parameter.

gdb/testsuite/ChangeLog:
2017-07-17  Pedro Alves  <palves@redhat.com>

	* gdb.linespec/ls-errs.exp (do_test): Adjust expected output.

1 files changed, 273 insertions, 30 deletions

diff --git a/gdb/location.c b/gdb/location.c
index d711d7b25b5..3238c9a84bf 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -415,13 +415,44 @@ event_location_to_string (struct event_location *location)
   return EL_STRING (location);
 }
 
+/* Find an instance of the quote character C in the string S that is
+   outside of all single- and double-quoted strings (i.e., any quoting
+   other than C).  */
+
+static const char *
+find_end_quote (const char *s, char end_quote_char)
+{
+  /* zero if we're not in quotes;
+     '"' if we're in a double-quoted string;
+     '\'' if we're in a single-quoted string.  */
+  char nested_quote_char = '\0';
+
+  for (const char *scan = s; *scan != '\0'; scan++)
+    {
+      if (nested_quote_char != '\0')
+	{
+	  if (*scan == nested_quote_char)
+	    nested_quote_char = '\0';
+	  else if (scan[0] == '\\' && *(scan + 1) != '\0')
+	    scan++;
+	}
+      else if (*scan == end_quote_char && nested_quote_char == '\0')
+	return scan;
+      else if (*scan == '"' || *scan == '\'')
+	nested_quote_char = *scan;
+    }
+
+  return 0;
+}
+
 /* A lexer for explicit locations.  This function will advance INP
    past any strings that it lexes.  Returns a malloc'd copy of the
    lexed string or NULL if no lexing was done.  */
 
 static gdb::unique_xmalloc_ptr<char>
 explicit_location_lex_one (const char **inp,
-			   const struct language_defn *language)
+			   const struct language_defn *language,
+			   explicit_completion_info *completion_info)
 {
   const char *start = *inp;
 
@@ -431,21 +462,27 @@ explicit_location_lex_one (const char **inp,
   /* If quoted, skip to the ending quote.  */
   if (strchr (get_gdb_linespec_parser_quote_characters (), *start))
     {
-      char quote_char = *start;
+      if (completion_info != NULL)
+	completion_info->quoted_arg_start = start;
 
-      /* If the input is not an Ada operator, skip to the matching
-	 closing quote and return the string.  */
-      if (!(language->la_language == language_ada
-	    && quote_char == '\"' && is_ada_operator (start)))
-	{
-	  const char *end = find_toplevel_char (start + 1, quote_char);
+      const char *end = find_end_quote (start + 1, *start);
 
-	  if (end == NULL)
+      if (end == NULL)
+	{
+	  if (completion_info == NULL)
 	    error (_("Unmatched quote, %s."), start);
-	  *inp = end + 1;
+
+	  end = start + strlen (start);
+	  *inp = end;
 	  return gdb::unique_xmalloc_ptr<char> (savestring (start + 1,
-							    *inp - start - 2));
+							    *inp - start - 1));
 	}
+
+      if (completion_info != NULL)
+	completion_info->quoted_arg_end = end;
+      *inp = end + 1;
+      return gdb::unique_xmalloc_ptr<char> (savestring (start + 1,
+							*inp - start - 2));
     }
 
   /* If the input starts with '-' or '+', the string ends with the next
@@ -486,12 +523,180 @@ explicit_location_lex_one (const char **inp,
   return NULL;
 }
 
+/* Return true if COMMA points past "operator".  START is the start of
+   the line that COMMAND points to, hence when reading backwards, we
+   must not read any character before START.  */
+
+static bool
+is_cp_operator (const char *start, const char *comma)
+{
+  if (comma != NULL
+      && (comma - start) >= CP_OPERATOR_LEN)
+    {
+      const char *p = comma;
+
+      while (p > start && isspace (p[-1]))
+	p--;
+      if (p - start >= CP_OPERATOR_LEN)
+	{
+	  p -= CP_OPERATOR_LEN;
+	  if (strncmp (p, CP_OPERATOR_STR, CP_OPERATOR_LEN) == 0
+	      && (p == start
+		  || !(isalnum (p[-1]) || p[-1] == '_')))
+	    {
+	      return true;
+	    }
+	}
+    }
+  return false;
+}
+
+/* When scanning the input string looking for the next explicit
+   location option/delimiter, we jump to the next option by looking
+   for ",", and "-".  Such a character can also appear in C++ symbols
+   like "operator," and "operator-".  So when we find such a
+   character, we call this function to check if we found such a
+   symbol, meaning we had a false positive for an option string.  In
+   that case, we keep looking for the next delimiter, until we find
+   one that is not a false positive, or we reach end of string.  FOUND
+   is the character that scanning found (either '-' or ','), and START
+   is the start of the line that FOUND points to, hence when reading
+   backwards, we must not read any character before START.  Returns a
+   pointer to the next non-false-positive delimiter character, or NULL
+   if none was found.  */
+
+static const char *
+skip_op_false_positives (const char *start, const char *found)
+{
+  while (found != NULL && is_cp_operator (start, found))
+    {
+      if (found[0] == '-' && found[1] == '-')
+	start = found + 2;
+      else
+	start = found + 1;
+      found = find_toplevel_char (start, *found);
+    }
+
+  return found;
+}
+
+/* Assuming both FIRST and NEW_TOK point into the same string, return
+   the pointer that is closer to the start of the string.  If FIRST is
+   NULL, returns NEW_TOK.  If NEW_TOK is NULL, returns FIRST.  */
+
+static const char *
+first_of (const char *first, const char *new_tok)
+{
+  if (first == NULL)
+    return new_tok;
+  else if (new_tok != NULL && new_tok < first)
+    return new_tok;
+  else
+    return first;
+}
+
+/* A lexer for functions in explicit locations.  This function will
+   advance INP past a function until the next option, or until end of
+   string.  Returns a malloc'd copy of the lexed string or NULL if no
+   lexing was done.  */
+
+static gdb::unique_xmalloc_ptr<char>
+explicit_location_lex_one_function (const char **inp,
+				    const struct language_defn *language,
+				    explicit_completion_info *completion_info)
+{
+  const char *start = *inp;
+
+  if (*start == '\0')
+    return NULL;
+
+  /* If quoted, skip to the ending quote.  */
+  if (strchr (get_gdb_linespec_parser_quote_characters (), *start))
+    {
+      char quote_char = *start;
+
+      /* If the input is not an Ada operator, skip to the matching
+	 closing quote and return the string.  */
+      if (!(language->la_language == language_ada
+	    && quote_char == '\"' && is_ada_operator (start)))
+	{
+	  if (completion_info != NULL)
+	    completion_info->quoted_arg_start = start;
+
+	  const char *end = find_toplevel_char (start + 1, quote_char);
+
+	  if (end == NULL)
+	    {
+	      if (completion_info == NULL)
+		error (_("Unmatched quote, %s."), start);
+
+	      end = start + strlen (start);
+	      *inp = end;
+	      char *saved = savestring (start + 1, *inp - start - 1);
+	      return gdb::unique_xmalloc_ptr<char> (saved);
+	    }
+
+	  if (completion_info != NULL)
+	    completion_info->quoted_arg_end = end;
+	  *inp = end + 1;
+	  char *saved = savestring (start + 1, *inp - start - 2);
+	  return gdb::unique_xmalloc_ptr<char> (saved);
+	}
+    }
+
+  const char *comma = find_toplevel_char (start, ',');
+
+  /* If we have "-function -myfunction", or perhaps better example,
+     "-function -[BasicClass doIt]" (objc selector), treat
+     "-myfunction" as the function name.  I.e., skip the first char if
+     it is an hyphen.  Don't skip the first char always, because we
+     may have C++ "operator<", and find_toplevel_char needs to see the
+     'o' in that case.  */
+  const char *hyphen
+    = (*start == '-'
+       ? find_toplevel_char (start + 1, '-')
+       : find_toplevel_char (start, '-'));
+
+  /* Check for C++ "operator," and "operator-".  */
+  comma = skip_op_false_positives (start, comma);
+  hyphen = skip_op_false_positives (start, hyphen);
+
+  /* Pick the one that appears first.  */
+  const char *end = first_of (hyphen, comma);
+
+  /* See if a linespec keyword appears first.  */
+  const char *s = start;
+  const char *ws = find_toplevel_char (start, ' ');
+  while (ws != NULL && linespec_lexer_lex_keyword (ws + 1) == NULL)
+    {
+      s = ws + 1;
+      ws = find_toplevel_char (s, ' ');
+    }
+  if (ws != NULL)
+    end = first_of (end, ws + 1);
+
+  /* If we don't have any terminator, then take the whole string.  */
+  if (end == NULL)
+    end = start + strlen (start);
+
+  /* Trim whitespace at the end.  */
+  while (end > start && end[-1] == ' ')
+    end--;
+
+  *inp = end;
+
+  if (*inp - start > 0)
+    return gdb::unique_xmalloc_ptr<char> (savestring (start, *inp - start));
+
+  return NULL;
+}
+
 /* See description in location.h.  */
 
 event_location_up
 string_to_explicit_location (const char **argp,
 			     const struct language_defn *language,
-			     int dont_throw)
+			     explicit_completion_info *completion_info)
 {
   event_location_up location;
 
@@ -514,6 +719,14 @@ string_to_explicit_location (const char **argp,
       int len;
       const char *start;
 
+      /* Clear these on each iteration, since they should be filled
+	 with info about the last option.  */
+      if (completion_info != NULL)
+	{
+	  completion_info->quoted_arg_start = NULL;
+	  completion_info->quoted_arg_end = NULL;
+	}
+
       /* If *ARGP starts with a keyword, stop processing
 	 options.  */
       if (linespec_lexer_lex_keyword (*argp) != NULL)
@@ -522,40 +735,68 @@ string_to_explicit_location (const char **argp,
       /* Mark the start of the string in case we need to rewind.  */
       start = *argp;
 
+      if (completion_info != NULL)
+	completion_info->last_option = start;
+
       /* Get the option string.  */
       gdb::unique_xmalloc_ptr<char> opt
-	= explicit_location_lex_one (argp, language);
+	= explicit_location_lex_one (argp, language, NULL);
 
-      *argp = skip_spaces_const (*argp);
+      /* Use the length of the option to allow abbreviations.  */
+      len = strlen (opt.get ());
 
       /* Get the argument string.  */
-      gdb::unique_xmalloc_ptr<char> oarg
-	= explicit_location_lex_one (argp, language);
-      bool have_oarg = oarg != NULL;
       *argp = skip_spaces_const (*argp);
 
-      /* Use the length of the option to allow abbreviations.  */
-      len = strlen (opt.get ());
+      /* All options have a required argument.  Checking for this
+	 required argument is deferred until later.  */
+      gdb::unique_xmalloc_ptr<char> oarg;
+      /* True if we have an argument.  This is required because we'll
+	 move from OARG before checking whether we have an
+	 argument.  */
+      bool have_oarg = false;
+
+      /* Convenience to consistently set both OARG/HAVE_OARG from
+	 ARG.  */
+      auto set_oarg = [&] (gdb::unique_xmalloc_ptr<char> arg)
+	{
+	  oarg = std::move (arg);
+	  have_oarg = oarg != NULL;
+	};
 
-      /* All options have a required argument.  Checking for this required
-	 argument is deferred until later.  */
       if (strncmp (opt.get (), "-source", len) == 0)
-	EL_EXPLICIT (location)->source_filename = oarg.release ();
+	{
+	  set_oarg (explicit_location_lex_one (argp, language,
+					       completion_info));
+	  EL_EXPLICIT (location)->source_filename = oarg.release ();
+	}
       else if (strncmp (opt.get (), "-function", len) == 0)
-	EL_EXPLICIT (location)->function_name = oarg.release ();
+	{
+	  set_oarg (explicit_location_lex_one_function (argp, language,
+							completion_info));
+	  EL_EXPLICIT (location)->function_name = oarg.release ();
+	}
       else if (strncmp (opt.get (), "-line", len) == 0)
 	{
+	  set_oarg (explicit_location_lex_one (argp, language, NULL));
+	  *argp = skip_spaces_const (*argp);
 	  if (have_oarg)
-	    EL_EXPLICIT (location)->line_offset
-	      = linespec_parse_line_offset (oarg.get ());
+	    {
+	      EL_EXPLICIT (location)->line_offset
+		= linespec_parse_line_offset (oarg.get ());
+	      continue;
+	    }
 	}
       else if (strncmp (opt.get (), "-label", len) == 0)
-	EL_EXPLICIT (location)->label_name = oarg.release ();
+	{
+	  set_oarg (explicit_location_lex_one (argp, language, completion_info));
+	  EL_EXPLICIT (location)->label_name = oarg.release ();
+	}
       /* Only emit an "invalid argument" error for options
 	 that look like option strings.  */
       else if (opt.get ()[0] == '-' && !isdigit (opt.get ()[1]))
 	{
-	  if (!dont_throw)
+	  if (completion_info == NULL)
 	    error (_("invalid explicit location argument, \"%s\""), opt.get ());
 	}
       else
@@ -567,11 +808,13 @@ string_to_explicit_location (const char **argp,
 	  return location;
 	}
 
+      *argp = skip_spaces_const (*argp);
+
       /* It's a little lame to error after the fact, but in this
 	 case, it provides a much better user experience to issue
 	 the "invalid argument" error before any missing
 	 argument error.  */
-      if (!have_oarg && !dont_throw)
+      if (!have_oarg && completion_info == NULL)
 	error (_("missing argument for \"%s\""), opt.get ());
     }
 
@@ -581,7 +824,7 @@ string_to_explicit_location (const char **argp,
       && EL_EXPLICIT (location)->function_name == NULL
       && EL_EXPLICIT (location)->label_name == NULL
       && (EL_EXPLICIT (location)->line_offset.sign == LINE_OFFSET_UNKNOWN)
-      && !dont_throw)
+      && completion_info == NULL)
     {
       error (_("Source filename requires function, label, or "
 	       "line offset."));
@@ -639,7 +882,7 @@ string_to_event_location (char **stringp,
 
   /* Try an explicit location.  */
   orig = arg = *stringp;
-  event_location_up location = string_to_explicit_location (&arg, language, 0);
+  event_location_up location = string_to_explicit_location (&arg, language, NULL);
   if (location != NULL)
     {
       /* It was a valid explicit location.  Advance STRINGP to