[RFA] Improve completion of locations

[RFA] Improve completion of locations

[Eli Zaretskii]

I'm seeking approval for the patches below, which improve GDB's
completion of locations in commands like "break LOCATION".  The two
main improvements are:

  - GDB now considers file names as well as symbol names when you
    type, e.g., "break foo".  If there's a symbol foo_bar and a file
    foo-bar.c, both will be shown in the list of possible completions.
    Only files recorded in the debug info are used for completing file
    names for these commands.

  - When the user types "break foo.c:bar TAB", only symbols defined in
    the source file foo.c whose names begin with "bar" will be
    considered for completion.  This reduces the number of possible
    completions by a large factor in many typical situations (I'd even
    dare to say that it makes symbol completion a useful feature ;-).

This is the last installment of my recent completion-related frenzy.
I'm now quite happy with GDB's completion ;-)

AFAICS, the responsible maintainers who need to approve these patches
are Fernando, Elena, Michael Snyder, and someone for printcmd.c and
infcmd.c.

2001-05-05  Eli Zaretskii  <eliz@is.elta.co.il>

	* completer.c (gdb_completer_loc_break_characters): New variable.
	(line_completion_function): If we are completing on locations,
	back up the start of word pointer past all characters which can
	appear in a location spec.
	(location_completer): New function.

	* completer.h: Add prototype for location_completer.

	* symtab.c (make_source_files_completion_list)
	(add_filename_to_list, not_interesting_fname): New functions.
	(filename_seen): New function, body extracted from
	output_source_filename.
	(output_source_filename): Call filename_seen to check if the file
	was already printed.
	(make_symbol_completion_list): If TEXT includes a
	double-quoted string, return an empty list, not NULL.
	(make_file_symbol_completion_list): New function, similar to
	make_symbol_completion_list but with an additional argument
	SRCFILE.

	* symtab.h (make_file_symbol_completion_list)
	(make_source_files_completion_list): Add prototypes.

	* breakpoint.c (_initialize_breakpoint): Make location_completer
	be the completion function for all commands which set breakpoints
	and watchpoints.
	(top-level): #include "completer.h".

	* tracepoint.c (_initialize_tracepoint): Make location_completer
	be the completion function for the "trace" command.

	* printcmd.c (_initialize_printcmd): Make location_completer be
	the completion function for the "print", "inspect", "call", and
	"disassemble" commands.
	(top-level): #include "completer.h".

	* infcmd.c (_initialize_infcmd): Make location_completer be the
	completion function for the "go", "jump", and "until" commands.


--- gdb/completer.c~1	Sat Feb 17 12:22:10 2001
+++ gdb/completer.c	Sat May  5 20:20:24 2001
@@ -22,12 +22,14 @@
 #include "symtab.h"
 #include "gdbtypes.h"
 #include "expression.h"
+#include "filenames.h"		/* for DOSish file names */
 
 /* FIXME: This is needed because of lookup_cmd_1().
    We should be calling a hook instead so we eliminate the CLI dependency. */
 #include "gdbcmd.h"
 
-/* Needed for rl_completer_word_break_characters() */
+/* Needed for rl_completer_word_break_characters() and for
+   filename_completion_function.  */
 #include <readline/readline.h>
 
 /* readline defines this.  */
@@ -72,6 +74,10 @@ static char *gdb_completer_file_name_bre
 static char *gdb_completer_file_name_break_characters = " \t\n*|\"';:?><";
 #endif
 
+/* These are used when completing on locations, which can mix file
+   names and symbol names separated by a colon.  */
+static char *gdb_completer_loc_break_characters = " \t\n*|\"';:?><,";
+
 /* Characters that can be used to quote completion strings.  Note that we
    can't include '"' because the gdb C parser treats such quoted sequences
    as strings. */
@@ -95,8 +101,6 @@ get_gdb_completer_quote_characters (void
 char **
 filename_completer (char *text, char *word)
 {
-  /* From readline.  */
-extern char *filename_completion_function (char *, int);
   int subsequent_name;
   char **return_val;
   int return_val_used;
@@ -170,6 +174,151 @@ extern char *filename_completion_functio
   return return_val;
 }
 
+/* Complete on locations, which might be of two possible forms:
+
+       file:line
+   or
+       symbol+offset
+
+   This is intended to be used in commands that set breakpoints etc.  */
+char **
+location_completer (char *text, char *word)
+{
+  int n_syms = 0, n_files = 0;
+  char ** fn_list = NULL;
+  char ** list = NULL;
+  char *p;
+  int quote_found = 0;
+  int quoted = *text == '\'' || *text == '"';
+  int quote_char = '\0';
+  char *colon = NULL;
+  char *file_to_match = NULL;
+  char *symbol_start = text;
+  char *orig_text = text;
+  size_t text_len;
+
+  /* Do we have an unquoted colon, as in "break foo.c::bar"?  */
+  for (p = text; *p != '\0'; ++p)
+    {
+      if (*p == '\\' && p[1] == '\'')
+	p++;
+      else if (*p == '\'' || *p == '"')
+	{
+	  quote_found = *p;
+	  quote_char = *p++;
+	  while (*p != '\0' && *p != quote_found)
+	    {
+	      if (*p == '\\' && p[1] == quote_found)
+		p++;
+	      p++;
+	    }
+
+	  if (*p == quote_found)
+	    quote_found = 0;
+	}
+#if HAVE_DOS_BASED_FILE_SYSTEM
+      /* If we have a DOS-style absolute file name at the beginning of
+	 TEXT, and the colon after the drive letter is the only colon
+	 we found, pretend the colon is not there.  */
+      else if (p < text + 3 && *p == ':' && p == text + 1 + quoted)
+	;
+#endif
+      else if (*p == ':' && !colon)
+	{
+	  colon = p;
+	  symbol_start = p + 1;
+	}
+      else if (strchr (gdb_completer_word_break_characters, *p))
+	symbol_start = p + 1;
+    }
+
+  if (quoted)
+    text++;
+  text_len = strlen (text);
+
+  /* Where is the file name?  */
+  if (colon)
+    {
+      char *s;
+
+      file_to_match = (char *) xmalloc (colon - text + 1);
+      strncpy (file_to_match, text, colon - text + 1);
+      /* Remove trailing colons and quotes from the file name.  */
+      for (s = file_to_match + (colon - text);
+	   s > file_to_match;
+	   s--)
+	if (*s == ':' || *s == quote_char)
+	  *s = '\0';
+    }
+  /* If the text includes a colon, they want completion only on a
+     symbol name after the colon.  Otherwise, we need to complete on
+     symbols as well as on files.  */
+  if (colon)
+    {
+      list = make_file_symbol_completion_list (symbol_start, word,
+					       file_to_match);
+      xfree (file_to_match);
+    }
+  else
+    {
+      list = make_symbol_completion_list (symbol_start, word);
+      /* If text includes characters which cannot appear in a file
+	 name, they cannot be asking for completion on files.  */
+      if (strcspn (text, gdb_completer_file_name_break_characters) == text_len)
+	fn_list = make_source_files_completion_list (text, text);
+    }
+
+  /* How many completions do we have in both lists?  */
+  if (fn_list)
+    for ( ; fn_list[n_files]; n_files++)
+      ;
+  if (list)
+    for ( ; list[n_syms]; n_syms++)
+      ;
+
+  /* Make list[] large enough to hold both lists, then catenate
+     fn_list[] onto the end of list[].  */
+  if (n_syms && n_files)
+    {
+      list = xrealloc (list, (n_syms + n_files + 1) * sizeof (char *));
+      memcpy (list + n_syms, fn_list, (n_files + 1) * sizeof (char *));
+      xfree (fn_list);
+    }
+  else if (n_files)
+    {
+      /* If we only have file names as possible completion, we should
+	 bring them in sync with what rl_complete expects.  The
+	 problem is that if the user types "break /foo/b TAB", and the
+	 possible completions are "/foo/bar" and "/foo/baz"
+	 rl_complete expects us to return "bar" and "baz", without the
+	 leading directories, as possible completions, because `word'
+	 starts at the "b".  But we ignore the value of `word' when we
+	 call make_source_files_completion_list above (because that
+	 would not DTRT when the completion results in both symbols
+	 and file names), so make_source_files_completion_list returns
+	 the full "/foo/bar" and "/foo/baz" strings.  This produces
+	 wrong results when, e.g., there's only one possible
+	 completion, because rl_complete will prepend "/foo/" to each
+	 candidate completion.  The loop below removes that leading
+	 part.  */
+      for (n_files = 0; fn_list[n_files]; n_files++)
+	{
+	  memmove (fn_list[n_files], fn_list[n_files] + (word - text),
+		   strlen (fn_list[n_files]) + 1 - (word - text));
+	}
+      /* Return just the file-name list as the result.  */
+      list = fn_list;
+    }
+  else if (!n_syms)
+    {
+      /* No completions at all.  As the final resort, try completing
+	 on the entire text as a symbol.  */
+      list = make_symbol_completion_list (orig_text, word);
+    }
+
+  return list;
+}
+
 /* Here are some useful test cases for completion.  FIXME: These should
    be put in the test suite.  They should be tested with both M-? and TAB.
 
@@ -362,7 +511,7 @@ line_completion_function (char *text, in
 			     to complete the entire text after the
 			     command, just the last word.  To this
 			     end, we need to find the beginning of the
-			     file name starting at `word' and going
+			     file name by starting at `word' and going
 			     backwards.  */
 			  for (p = word;
 			       p > tmp_command
@@ -372,6 +521,16 @@ line_completion_function (char *text, in
 			  rl_completer_word_break_characters =
 			    gdb_completer_file_name_break_characters;
 			}
+		      else if (c->completer == location_completer)
+			{
+			  /* Commands which complete on locations want to
+			     see the entire argument.  */
+			  for (p = word;
+			       p > tmp_command
+				 && p[-1] != ' ' && p[-1] != '\t';
+			       p--)
+			    ;
+			}
 		      list = (*c->completer) (p, word);
 		    }
 		}
@@ -430,6 +589,14 @@ line_completion_function (char *text, in
 		      rl_completer_word_break_characters =
 			gdb_completer_file_name_break_characters;
 		    }
+		  else if (c->completer == location_completer)
+		    {
+		      for (p = word;
+			   p > tmp_command
+			     && p[-1] != ' ' && p[-1] != '\t';
+			   p--)
+			;
+		    }
 		  list = (*c->completer) (p, word);
 		}
 	    }

--- gdb/completer.h~	Fri Dec  1 02:41:26 2000
+++ gdb/completer.h	Sat Apr  7 16:44:22 2001
@@ -23,6 +23,8 @@ extern char *line_completion_function (c
 
 extern char **filename_completer (char *, char *);
 
+extern char **location_completer (char *, char *);
+
 extern char *get_gdb_completer_word_break_characters (void); 
 
 extern char *get_gdb_completer_quote_characters (void);

--- gdb/symtab.c~2	Fri Mar 30 14:57:20 2001
+++ gdb/symtab.c	Sat May  5 18:56:00 2001
@@ -2161,50 +2147,72 @@ operator_chars (char *p, char **end)
 }
 \f
 
-/* Slave routine for sources_info.  Force line breaks at ,'s.
-   NAME is the name to print and *FIRST is nonzero if this is the first
-   name printed.  Set *FIRST to zero.  */
-static void
-output_source_filename (char *name, int *first)
+/* If FILE is not already in the table of files, return zero;
+   otherwise return non-zero.  Optionally add FILE to the table if ADD
+   is non-zero.  If *FIRST is non-zero, forget the old table
+   contents.  */
+static int
+filename_seen (const char *file, int add, int *first)
 {
-  /* Table of files printed so far.  Since a single source file can
-     result in several partial symbol tables, we need to avoid printing
-     it more than once.  Note: if some of the psymtabs are read in and
-     some are not, it gets printed both under "Source files for which
-     symbols have been read" and "Source files for which symbols will
-     be read in on demand".  I consider this a reasonable way to deal
-     with the situation.  I'm not sure whether this can also happen for
-     symtabs; it doesn't hurt to check.  */
-  static char **tab = NULL;
+  /* Table of files seen so far.  */
+  static const char **tab = NULL;
   /* Allocated size of tab in elements.
      Start with one 256-byte block (when using GNU malloc.c).
      24 is the malloc overhead when range checking is in effect.  */
   static int tab_alloc_size = (256 - 24) / sizeof (char *);
   /* Current size of tab in elements.  */
   static int tab_cur_size;
-
-  char **p;
+  const char **p;
 
   if (*first)
     {
       if (tab == NULL)
-	tab = (char **) xmalloc (tab_alloc_size * sizeof (*tab));
+	tab = (const char **) xmalloc (tab_alloc_size * sizeof (*tab));
       tab_cur_size = 0;
     }
 
-  /* Is NAME in tab?  */
+  /* Is FILE in tab?  */
   for (p = tab; p < tab + tab_cur_size; p++)
-    if (STREQ (*p, name))
-      /* Yes; don't print it again.  */
-      return;
-  /* No; add it to tab.  */
-  if (tab_cur_size == tab_alloc_size)
+    if (strcmp (*p, file) == 0)
+      return 1;
+
+  /* No; maybe add it to tab.  */
+  if (add)
     {
-      tab_alloc_size *= 2;
-      tab = (char **) xrealloc ((char *) tab, tab_alloc_size * sizeof (*tab));
+      if (tab_cur_size == tab_alloc_size)
+	{
+	  tab_alloc_size *= 2;
+	  tab = (const char **) xrealloc ((char *) tab,
+					  tab_alloc_size * sizeof (*tab));
+	}
+      tab[tab_cur_size++] = file;
     }
-  tab[tab_cur_size++] = name;
 
+  return 0;
+}
+
+/* Slave routine for sources_info.  Force line breaks at ,'s.
+   NAME is the name to print and *FIRST is nonzero if this is the first
+   name printed.  Set *FIRST to zero.  */
+static void
+output_source_filename (char *name, int *first)
+{
+  /* Since a single source file can result in several partial symbol
+     tables, we need to avoid printing it more than once.  Note: if
+     some of the psymtabs are read in and some are not, it gets
+     printed both under "Source files for which symbols have been
+     read" and "Source files for which symbols will be read in on
+     demand".  I consider this a reasonable way to deal with the
+     situation.  I'm not sure whether this can also happen for
+     symtabs; it doesn't hurt to check.  */
+
+  /* Was NAME already seen?  */
+  if (filename_seen (name, 1, first))
+    {
+      /* Yes; don't print it again.  */
+      return;
+    }
+  /* No; print it and reset *FIRST.  */
   if (*first)
     {
       *first = 0;
@@ -2871,9 +2879,9 @@ completion_list_add_name (char *symname,
   }
 }
 
-/* Return a NULL terminated array of all symbols (regardless of class) which
-   begin by matching TEXT.  If the answer is no symbols, then the return value
-   is an array which contains only a NULL pointer.
+/* Return a NULL terminated array of all symbols (regardless of class)
+   which begin by matching TEXT.  If the answer is no symbols, then
+   the return value is an array which contains only a NULL pointer.
 
    Problem: All of the symbols have to be copied because readline frees them.
    I'm not going to worry about this; hopefully there won't be that many.  */
@@ -2927,7 +2935,11 @@ make_symbol_completion_list (char *text,
     else if (quote_found == '"')
       /* A double-quoted string is never a symbol, nor does it make sense
          to complete it any other way.  */
-      return NULL;
+      {
+	return_val = (char **) xmalloc (sizeof (char *));
+	return_val[0] = NULL;
+	return return_val;
+      }
     else
       {
 	/* It is not a quoted string.  Break it based on the characters
@@ -3059,6 +3071,277 @@ make_symbol_completion_list (char *text,
   return (return_val);
 }
 
+/* Like make_symbol_completion_list, but returns a list of symbols
+   defined in a source file FILE.  */
+
+char **
+make_file_symbol_completion_list (char *text, char *word, char *srcfile)
+{
+  register struct symbol *sym;
+  register struct symtab *s;
+  register struct block *b;
+  register int i;
+  /* The symbol we are completing on.  Points in same buffer as text.  */
+  char *sym_text;
+  /* Length of sym_text.  */
+  int sym_text_len;
+
+  /* Now look for the symbol we are supposed to complete on.
+     FIXME: This should be language-specific.  */
+  {
+    char *p;
+    char quote_found;
+    char *quote_pos = NULL;
+
+    /* First see if this is a quoted string.  */
+    quote_found = '\0';
+    for (p = text; *p != '\0'; ++p)
+      {
+	if (quote_found != '\0')
+	  {
+	    if (*p == quote_found)
+	      /* Found close quote.  */
+	      quote_found = '\0';
+	    else if (*p == '\\' && p[1] == quote_found)
+	      /* A backslash followed by the quote character
+	         doesn't end the string.  */
+	      ++p;
+	  }
+	else if (*p == '\'' || *p == '"')
+	  {
+	    quote_found = *p;
+	    quote_pos = p;
+	  }
+      }
+    if (quote_found == '\'')
+      /* A string within single quotes can be a symbol, so complete on it.  */
+      sym_text = quote_pos + 1;
+    else if (quote_found == '"')
+      /* A double-quoted string is never a symbol, nor does it make sense
+         to complete it any other way.  */
+      {
+	return_val = (char **) xmalloc (sizeof (char *));
+	return_val[0] = NULL;
+	return return_val;
+      }
+    else
+      {
+	/* It is not a quoted string.  Break it based on the characters
+	   which are in symbols.  */
+	while (p > text)
+	  {
+	    if (isalnum (p[-1]) || p[-1] == '_' || p[-1] == '\0')
+	      --p;
+	    else
+	      break;
+	  }
+	sym_text = p;
+      }
+  }
+
+  sym_text_len = strlen (sym_text);
+
+  return_val_size = 10;
+  return_val_index = 0;
+  return_val = (char **) xmalloc ((return_val_size + 1) * sizeof (char *));
+  return_val[0] = NULL;
+
+  /* Find the symtab for SRCFILE (this loads it if it was not yet read
+     in).  */
+  s = lookup_symtab (srcfile);
+  if (s == NULL)
+    {
+      /* Maybe they typed the file with leading directories, while the
+	 symbol tables record only its basename.  */
+      char *tail = basename (srcfile);
+
+      if (tail > srcfile)
+	s = lookup_symtab (tail);
+    }
+
+  /* If we have no symtab for that file, return an empty list.  */
+  if (s == NULL)
+    return (return_val);
+
+  /* Go through this symtab and check the externs and statics for
+     symbols which match.  */
+
+  b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
+  for (i = 0; i < BLOCK_NSYMS (b); i++)
+    {
+      sym = BLOCK_SYM (b, i);
+      COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
+    }
+
+  b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), STATIC_BLOCK);
+  for (i = 0; i < BLOCK_NSYMS (b); i++)
+    {
+      sym = BLOCK_SYM (b, i);
+      COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
+    }
+
+  return (return_val);
+}
+
+/* A helper function for make_source_files_completion_list.  It adds
+   another file name to a list of possible completions, growing the
+   list as necessary.  */
+
+static void
+add_filename_to_list (const char *fname, char *text, char *word,
+		      char ***list, int *list_used, int *list_alloced)
+{
+  char *new;
+  size_t fnlen = strlen (fname);
+
+  if (*list_used + 1 >= *list_alloced)
+    {
+      *list_alloced *= 2;
+      *list = (char **) xrealloc ((char *) *list,
+				  *list_alloced * sizeof (char *));
+    }
+
+  if (word == text)
+    {
+      /* Return exactly fname.  */
+      new = xmalloc (fnlen + 5);
+      strcpy (new, fname);
+    }
+  else if (word > text)
+    {
+      /* Return some portion of fname.  */
+      new = xmalloc (fnlen + 5);
+      strcpy (new, fname + (word - text));
+    }
+  else
+    {
+      /* Return some of TEXT plus fname.  */
+      new = xmalloc (fnlen + (text - word) + 5);
+      strncpy (new, word, text - word);
+      new[text - word] = '\0';
+      strcat (new, fname);
+    }
+  (*list)[*list_used] = new;
+  (*list)[*list_used += 1] = NULL;
+}
+
+static int
+not_interesting_fname (const char *fname)
+{
+  static const char *illegal_aliens[] = {
+    "_globals_",	/* inserted by coff_symtab_read */
+    NULL
+  };
+  int i;
+
+  for (i = 0; illegal_aliens[i]; i++)
+    {
+      if (strcmp (fname, illegal_aliens[i]) == 0)
+	return 1;
+    }
+  return 0;
+}
+
+/* Return a NULL terminated array of all source files whose names
+   begin with matching TEXT.  The file names are looked up in the
+   symbol tables of this program.  If the answer is no matchess, then
+   the return value is an array which contains only a NULL pointer.  */
+
+char **
+make_source_files_completion_list (char *text, char *word)
+{
+  register struct symtab *s;
+  register struct partial_symtab *ps;
+  register struct objfile *objfile;
+  int first = 1;
+  int list_alloced = 1;
+  int list_used = 0;
+  size_t text_len = strlen (text);
+  char **list = (char **) xmalloc (list_alloced * sizeof (char *));
+  char *base_name;
+
+  list[0] = NULL;
+
+  if (!have_full_symbols () && !have_partial_symbols ())
+    return list;
+
+  ALL_SYMTABS (objfile, s)
+    {
+      if (not_interesting_fname (s->filename))
+	continue;
+      if (!filename_seen (s->filename, 1, &first)
+#if HAVE_DOS_BASED_FILE_SYSTEM
+	  && strncasecmp (s->filename, text, text_len) == 0
+#else
+	  && strncmp (s->filename, text, text_len) == 0
+#endif
+	  )
+	{
+	  /* This file matches for a completion; add it to the current
+	     list of matches.  */
+	  add_filename_to_list (s->filename, text, word,
+				&list, &list_used, &list_alloced);
+	}
+      else
+	{
+	  /* NOTE: We allow the user to type a base name when the
+	     debug info records leading directories, but not the other
+	     way around.  This is what subroutines of breakpoint
+	     command do when they parse file names.  */
+	  base_name = basename (s->filename);
+	  if (base_name != s->filename
+	      && !filename_seen (base_name, 1, &first)
+#if HAVE_DOS_BASED_FILE_SYSTEM
+	      && strncasecmp (base_name, text, text_len) == 0
+#else
+	      && strncmp (base_name, text, text_len) == 0
+#endif
+	      )
+	    add_filename_to_list (base_name, text, word,
+				  &list, &list_used, &list_alloced);
+	}
+    }
+
+  ALL_PSYMTABS (objfile, ps)
+    {
+      if (not_interesting_fname (ps->filename))
+	continue;
+      if (!ps->readin)
+	{
+	  if (!filename_seen (ps->filename, 1, &first)
+#if HAVE_DOS_BASED_FILE_SYSTEM
+	      && strncasecmp (ps->filename, text, text_len) == 0
+#else
+	      && strncmp (ps->filename, text, text_len) == 0
+#endif
+	      )
+	    {
+	      /* This file matches for a completion; add it to the
+		 current list of matches.  */
+	      add_filename_to_list (ps->filename, text, word,
+				    &list, &list_used, &list_alloced);
+
+	    }
+	  else
+	    {
+	      base_name = basename (ps->filename);
+	      if (base_name != ps->filename
+		  && !filename_seen (base_name, 1, &first)
+#if HAVE_DOS_BASED_FILE_SYSTEM
+		  && strncasecmp (base_name, text, text_len) == 0
+#else
+		  && strncmp (base_name, text, text_len) == 0
+#endif
+		  )
+		add_filename_to_list (base_name, text, word,
+				      &list, &list_used, &list_alloced);
+	    }
+	}
+    }
+
+  return list;
+}
+
 /* Determine if PC is in the prologue of a function.  The prologue is the area
    between the first instruction of a function, and the first executable line.
    Returns 1 if PC *might* be in prologue, 0 if definately *not* in prologue.

--- gdb/symtab.h~2	Fri Feb  2 22:01:16 2001
+++ gdb/symtab.h	Sat Apr 28 14:00:24 2001
@@ -1386,8 +1386,12 @@ extern void select_source_symtab (struct
 
 extern char **make_symbol_completion_list (char *, char *);
 
+extern char **make_file_symbol_completion_list (char *, char *, char *);
+
 extern struct symbol **make_symbol_overload_list (struct symbol *);
 
+extern char **make_source_files_completion_list (char *, char *);
+
 /* symtab.c */
 
 extern struct partial_symtab *find_main_psymtab (void);

--- gdb/breakpoint.c~2	Sun Mar 18 20:34:06 2001
+++ gdb/breakpoint.c	Sat May  5 19:05:36 2001
@@ -40,6 +40,7 @@
 #include "symfile.h"
 #include "objfiles.h"
 #include "linespec.h"
+#include "completer.h"
 #ifdef UI_OUT
 #include "ui-out.h"
 #endif
@@ -7520,24 +7521,29 @@ then no output is printed when it is hit
 Usage is `condition N COND', where N is an integer and COND is an\n\
 expression to be evaluated whenever breakpoint N is reached.  ");
 
-  add_com ("tbreak", class_breakpoint, tbreak_command,
-	   "Set a temporary breakpoint.  Args like \"break\" command.\n\
+  c = add_com ("tbreak", class_breakpoint, tbreak_command,
+	       "Set a temporary breakpoint.  Args like \"break\" command.\n\
 Like \"break\" except the breakpoint is only temporary,\n\
 so it will be deleted when hit.  Equivalent to \"break\" followed\n\
 by using \"enable delete\" on the breakpoint number.");
-  add_com ("txbreak", class_breakpoint, tbreak_at_finish_command,
-	   "Set temporary breakpoint at procedure exit.  Either there should\n\
+  c->completer = location_completer;
+
+  c = add_com ("txbreak", class_breakpoint, tbreak_at_finish_command,
+	       "Set temporary breakpoint at procedure exit.  Either there should\n\
 be no argument or the argument must be a depth.\n");
+  c->completer = location_completer;
 
-  add_com ("hbreak", class_breakpoint, hbreak_command,
-	   "Set a hardware assisted  breakpoint. Args like \"break\" command.\n\
+  c = add_com ("hbreak", class_breakpoint, hbreak_command,
+	       "Set a hardware assisted  breakpoint. Args like \"break\" command.\n\
 Like \"break\" except the breakpoint requires hardware support,\n\
 some target hardware may not have this support.");
+  c->completer = location_completer;
 
-  add_com ("thbreak", class_breakpoint, thbreak_command,
-	   "Set a temporary hardware assisted breakpoint. Args like \"break\" command.\n\
+  c = add_com ("thbreak", class_breakpoint, thbreak_command,
+	       "Set a temporary hardware assisted breakpoint. Args like \"break\" command.\n\
 Like \"hbreak\" except the breakpoint is only temporary,\n\
 so it will be deleted when hit.");
+  c->completer = location_completer;
 
   add_prefix_cmd ("enable", class_breakpoint, enable_command,
 		  "Enable some breakpoints.\n\
@@ -7639,8 +7645,8 @@ is executing in.\n\
 \n\
 See also the \"delete\" command which clears breakpoints by number.", NULL));
 
-  add_com ("break", class_breakpoint, break_command,
-	   concat ("Set breakpoint at specified line or function.\n\
+  c = add_com ("break", class_breakpoint, break_command,
+	       concat ("Set breakpoint at specified line or function.\n\
 Argument may be line number, function name, or \"*\" and an address.\n\
 If line number is specified, break at start of code for that line.\n\
 If function is specified, break at start of code for that function.\n\
@@ -7651,6 +7657,8 @@ This is useful for breaking on return to
 Multiple breakpoints at one place are permitted, and useful if conditional.\n\
 \n\
 Do \"help breakpoints\" for info on other commands dealing with breakpoints.", NULL));
+  c->completer = location_completer;
+
   add_com_alias ("b", "break", class_run, 1);
   add_com_alias ("br", "break", class_run, 1);
   add_com_alias ("bre", "break", class_run, 1);
@@ -7796,20 +7804,23 @@ Like \"catch\" except the catchpoint is 
 so it will be deleted when hit.  Equivalent to \"catch\" followed\n\
 by using \"enable delete\" on the catchpoint number.");
 
-  add_com ("watch", class_breakpoint, watch_command,
-	   "Set a watchpoint for an expression.\n\
+  c = add_com ("watch", class_breakpoint, watch_command,
+	       "Set a watchpoint for an expression.\n\
 A watchpoint stops execution of your program whenever the value of\n\
 an expression changes.");
+  c->completer = location_completer;
 
-  add_com ("rwatch", class_breakpoint, rwatch_command,
-	   "Set a read watchpoint for an expression.\n\
+  c = add_com ("rwatch", class_breakpoint, rwatch_command,
+	       "Set a read watchpoint for an expression.\n\
 A watchpoint stops execution of your program whenever the value of\n\
 an expression is read.");
+  c->completer = location_completer;
 
-  add_com ("awatch", class_breakpoint, awatch_command,
-	   "Set a watchpoint for an expression.\n\
+  c = add_com ("awatch", class_breakpoint, awatch_command,
+	       "Set a watchpoint for an expression.\n\
 A watchpoint stops execution of your program whenever the value of\n\
 an expression is either read or written.");
+  c->completer = location_completer;
 
   add_info ("watchpoints", breakpoints_info,
 	    "Synonym for ``info breakpoints''.");

--- gdb/tracepoint.c~2	Sat Feb 17 22:42:54 2001
+++ gdb/tracepoint.c	Sat May  5 19:33:50 2001
@@ -2774,12 +2774,13 @@ Arguments are tracepoint numbers, separa
 No argument means enable all tracepoints.",
 	   &enablelist);
 
-  add_com ("trace", class_trace, trace_command,
-	   "Set a tracepoint at a specified line or function or address.\n\
+  c = add_com ("trace", class_trace, trace_command,
+	       "Set a tracepoint at a specified line or function or address.\n\
 Argument may be a line number, function name, or '*' plus an address.\n\
 For a line number or function, trace at the start of its code.\n\
 If an address is specified, trace at that exact address.\n\n\
 Do \"help tracepoints\" for info on other tracepoint commands.");
+  c->completer = location_completer;
 
   add_com_alias ("tp", "trace", class_alias, 0);
   add_com_alias ("tr", "trace", class_alias, 1);

--- gdb/printcmd.c~2	Fri Dec 15 03:01:48 2000
+++ gdb/printcmd.c	Sat May  5 19:38:42 2001
@@ -37,6 +37,7 @@
 #include "annotate.h"
 #include "symfile.h"		/* for overlay functions */
 #include "objfiles.h"		/* ditto */
+#include "completer.h"		/* for completion functions */
 #ifdef UI_OUT
 #include "ui-out.h"
 #endif
@@ -2452,6 +2453,8 @@ print_insn (CORE_ADDR memaddr, struct ui
 void
 _initialize_printcmd (void)
 {
+  struct cmd_list_element *c;
+
   current_display_number = -1;
 
   add_info ("address", address_info,
@@ -2474,11 +2477,12 @@ Defaults for format and size letters are
 Default count is 1.  Default address is following last thing printed\n\
 with this command or \"print\".", NULL));
 
-  add_com ("disassemble", class_vars, disassemble_command,
-	   "Disassemble a specified section of memory.\n\
+  c = add_com ("disassemble", class_vars, disassemble_command,
+	       "Disassemble a specified section of memory.\n\
 Default is the function surrounding the pc of the selected frame.\n\
 With a single argument, the function surrounding that address is dumped.\n\
 Two arguments are taken as a range of memory to dump.");
+  c->completer = location_completer;
   if (xdb_commands)
     add_com_alias ("va", "disassemble", class_xdb, 0);
 
@@ -2556,11 +2560,12 @@ variable in the program being debugged. 
 You can see these environment settings with the \"show\" command.", NULL));
 
   /* "call" is the same as "set", but handy for dbx users to call fns. */
-  add_com ("call", class_vars, call_command,
-	   "Call a function in the program.\n\
+  c = add_com ("call", class_vars, call_command,
+	       "Call a function in the program.\n\
 The argument is the function name and arguments, in the notation of the\n\
 current working language.  The result is printed and saved in the value\n\
 history, if it is not void.");
+  c->completer = location_completer;
 
   add_cmd ("variable", class_vars, set_command,
 	   "Evaluate expression EXP and assign result to variable VAR, using assignment\n\
@@ -2571,7 +2576,7 @@ variable in the program being debugged. 
 This may usually be abbreviated to simply \"set\".",
 	   &setlist);
 
-  add_com ("print", class_vars, print_command,
+  c = add_com ("print", class_vars, print_command,
 	   concat ("Print value of expression EXP.\n\
 Variables accessible are those of the lexical environment of the selected\n\
 stack frame, plus all those whose scope is global or an entire file.\n\
@@ -2593,11 +2598,13 @@ resides in memory.\n",
 		   "\n\
 EXP may be preceded with /FMT, where FMT is a format letter\n\
 but no count or size letter (see \"x\" command).", NULL));
+  c->completer = location_completer;
   add_com_alias ("p", "print", class_vars, 1);
 
-  add_com ("inspect", class_vars, inspect_command,
+  c = add_com ("inspect", class_vars, inspect_command,
 	   "Same as \"print\" command, except that if you are running in the epoch\n\
 environment, the value is printed in its own window.");
+  c->completer = location_completer;
 
   add_show_from_set (
 		 add_set_cmd ("max-symbolic-offset", no_class, var_uinteger,

--- gdb/infcmd.c~2	Mon Feb 12 23:03:02 2001
+++ gdb/infcmd.c	Sat May  5 19:23:36 2001
@@ -1793,8 +1793,8 @@ _initialize_infcmd (void)
 {
   struct cmd_list_element *c;
 
-  c= add_com ("tty", class_run, tty_command,
-	      "Set terminal for future runs of program being debugged.");
+  c = add_com ("tty", class_run, tty_command,
+	       "Set terminal for future runs of program being debugged.");
   c->completer = filename_completer;
 
   add_show_from_set
@@ -1899,25 +1899,30 @@ Argument N means do this N times (or til
 Argument N means do this N times (or till program stops for another reason).");
   add_com_alias ("s", "step", class_run, 1);
 
-  add_com ("until", class_run, until_command,
-	   "Execute until the program reaches a source line greater than the current\n\
+  c = add_com ("until", class_run, until_command,
+	       "Execute until the program reaches a source line greater than the current\n\
 or a specified line or address or function (same args as break command).\n\
 Execution will also stop upon exit from the current stack frame.");
+  c->completer = location_completer;
   add_com_alias ("u", "until", class_run, 1);
 
-  add_com ("jump", class_run, jump_command,
-	   "Continue program being debugged at specified line or address.\n\
+  c = add_com ("jump", class_run, jump_command,
+	       "Continue program being debugged at specified line or address.\n\
 Give as argument either LINENUM or *ADDR, where ADDR is an expression\n\
 for an address to start at.");
+  c->completer = location_completer;
 
   if (xdb_commands)
-    add_com ("go", class_run, go_command,
-	     "Usage: go <location>\n\
+    {
+      c = add_com ("go", class_run, go_command,
+		   "Usage: go <location>\n\
 Continue program being debugged, stopping at specified line or \n\
 address.\n\
 Give as argument either LINENUM or *ADDR, where ADDR is an \n\
 expression for an address to start at.\n\
 This command is a combination of tbreak and jump.");
+      c->completer = location_completer;
+    }
 
   if (xdb_commands)
     add_com_alias ("g", "go", class_run, 1);

Re: [RFA] Improve completion of locations

[Daniel Berlin]

"Eli Zaretskii" <eliz@is.elta.co.il> writes:

> I'm seeking approval for the patches below, which improve GDB's
> completion of locations in commands like "break LOCATION".  The two
> main improvements are:
> 
>   - GDB now considers file names as well as symbol names when you
>     type, e.g., "break foo".  If there's a symbol foo_bar and a file
>     foo-bar.c, both will be shown in the list of possible completions.
>     Only files recorded in the debug info are used for completing file
>     names for these commands.
> 
>   - When the user types "break foo.c:bar TAB", only symbols defined in
>     the source file foo.c whose names begin with "bar" will be
>     considered for completion.  This reduces the number of possible
>     completions by a large factor in many typical situations (I'd even
>     dare to say that it makes symbol completion a useful feature
>     ;-).

Whoops, you can't do this the way you have it implemented (checking if
they have a colon)
You'll interfere with C++ completion, because scopes are delimited by
double colons as well.
If you disambiguate first by checking if the stuff before the colon is
a valid source filename, i have no problem with it.

You can't just check for double colon, either, since I may have typed
"foo:", wanting to complete foo::bar


--Dan

Re: [RFA] Improve completion of locations

[Eli Zaretskii]

On 5 May 2001, Daniel Berlin wrote:

> >   - When the user types "break foo.c:bar TAB", only symbols defined in
> >     the source file foo.c whose names begin with "bar" will be
> >     considered for completion.  This reduces the number of possible
> >     completions by a large factor in many typical situations (I'd even
> >     dare to say that it makes symbol completion a useful feature
> >     ;-).
> 
> Whoops, you can't do this the way you have it implemented (checking if
> they have a colon)
> You'll interfere with C++ completion, because scopes are delimited by
> double colons as well.
> If you disambiguate first by checking if the stuff before the colon is
> a valid source filename, i have no problem with it.
> 
> You can't just check for double colon, either, since I may have typed
> "foo:", wanting to complete foo::bar

I actually tested this, and it seemed to work, but perhaps I didn't
test enough.  Could you please post a test program where this should
be an issue?  I'm afraid my C++ talents are virtually non-existent.

And thanks for reviewing the patch.

Re: [RFA] Improve completion of locations

[Daniel Berlin]

>
> I actually tested this, and it seemed to work, but perhaps I didn't
> test enough.  Could you please post a test program where this should
> be an issue?  I'm afraid my C++ talents are virtually non-existent.

I can make it fail miserably with a simple example (IE my first
try).

Try the following:
#include <stdio.h>
class fred
{
	public:
		int bob();
};
int fred::bob()
{
	return 5;
}
int main(void)
{
	fred test;
	test.bob();
}


Complete on 'fred
It'll list fred and fred::bob(void)

Hit enter (you need to clear the completion status to make it redo the
list)

Complete on 'fred:

It'll list every symbol around (Or at least, 3792 of them, i would
imagine this is every single one, i never checked)

Complete on 'fred::
It'll complete to fred::bob(void)

The first is fine
The third is fine
The second is what your patch breaks right now.

It'll also break completion without quotes when i get around to rewriting
those amazingly complex parsing routines.

Would anyone really object if i just started a flex based lexer to parse
the specs, to replace all this silly ad-hoc parsing.
It seems pretty easy to do using the state based stuff in flex.
(It makes more sense to do it that way for our purposes, than using
lex+yacc.)

>
> And thanks for reviewing the patch.
>

Re: [RFA] Improve completion of locations

[Eli Zaretskii]

On Sun, 6 May 2001, Daniel Berlin wrote:

> I can make it fail miserably with a simple example (IE my first
> try).
> 
> Try the following:
> #include <stdio.h>
> class fred
> {
> 	public:
> 		int bob();
> };
> int fred::bob()
> {
> 	return 5;
> }
> int main(void)
> {
> 	fred test;
> 	test.bob();
> }

Thanks for the example.

> Complete on 'fred
> It'll list fred and fred::bob(void)
> 
> Hit enter (you need to clear the completion status to make it redo the
> list)
> 
> Complete on 'fred:
> 
> It'll list every symbol around (Or at least, 3792 of them, i would
> imagine this is every single one, i never checked)
> 
> Complete on 'fred::
> It'll complete to fred::bob(void)
> 
> The first is fine
> The third is fine
> The second is what your patch breaks right now.

Well, I'd hardly call this ``fail miserably''.  It is also simple to fix; 
I'll post a modified patch soon.

> It'll also break completion without quotes when i get around to rewriting
> those amazingly complex parsing routines.

I'm sorry, I don't follow: which parsing routines did you refer to?  Does 
the code I wrote use them?

> Would anyone really object if i just started a flex based lexer to parse
> the specs, to replace all this silly ad-hoc parsing.

Note that Readline has its own ideas about breaking user input into 
``words'', and it does that even before our completion functions are 
called.  So, in contrast to our code which parses the full location spec, 
completion cannot be much smarter than it currently is, because Readline
doesn't give us a chance to be smarter.  Most of the time I debugged this 
code went into trying to get along with Readline's idiosyncrasies.

Thanks again for the feedback.

Re: [RFA] Improve completion of locations

[Eli Zaretskii]

On Sun, 6 May 2001, Daniel Berlin wrote:

> Complete on 'fred:
> 
> It'll list every symbol around (Or at least, 3792 of them, i would
> imagine this is every single one, i never checked)

This small patch (to be applied on top of the large one I sent) should 
solve this:

--- gdb/completer.c~4	Sun May  6 09:50:42 2001
+++ gdb/completer.c	Sun May  6 13:10:30 2001
@@ -215,6 +215,8 @@ location_completer (char *text, char *wo
 
 	  if (*p == quote_found)
 	    quote_found = 0;
+	  else
+	    break;		/* hit the end of text */
 	}
 #if HAVE_DOS_BASED_FILE_SYSTEM
       /* If we have a DOS-style absolute file name at the beginning of

Re: [RFA] Improve completion of locations

[Daniel Berlin]

On Sun, 6 May 2001, Eli Zaretskii wrote:

>
> On Sun, 6 May 2001, Daniel Berlin wrote:
>
> > I can make it fail miserably with a simple example (IE my first
> > try).
> >
> > Try the following:
> > #include <stdio.h>
> > class fred
> > {
> > 	public:
> > 		int bob();
> > };
> > int fred::bob()
> > {
> > 	return 5;
> > }
> > int main(void)
> > {
> > 	fred test;
> > 	test.bob();
> > }
>
> Thanks for the example.
>
> > Complete on 'fred
> > It'll list fred and fred::bob(void)
> >
> > Hit enter (you need to clear the completion status to make it redo the
> > list)
> >
> > Complete on 'fred:
> >
> > It'll list every symbol around (Or at least, 3792 of them, i would
> > imagine this is every single one, i never checked)
> >
> > Complete on 'fred::
> > It'll complete to fred::bob(void)
> >
> > The first is fine
> > The third is fine
> > The second is what your patch breaks right now.
>
> Well, I'd hardly call this ``fail miserably''.  It is also simple to fix;
> I'll post a modified patch soon.

At 4am in the mroning, everything seems miserable.
:)


>
> > It'll also break completion without quotes when i get around to rewriting
> > those amazingly complex parsing routines.
>
> I'm sorry, I don't follow: which parsing routines did you refer to?  Does
> the code I wrote use them?
decode_line_1, et all.

You are doing the same type of thing, trying to determine what the user
wrote.


>
> > Would anyone really object if i just started a flex based lexer to parse
> > the specs, to replace all this silly ad-hoc parsing.
>
> Note that Readline has its own ideas about breaking user input into
> ``words'', and it does that even before our completion functions are
> called.  So, in contrast to our code which parses the full location spec,
> completion cannot be much smarter than it currently is, because Readline
> doesn't give us a chance to be smarter.  Most of the time I debugged this
> code went into trying to get along with Readline's idiosyncrasies.

Yeah, I figured as much.

Still, wouldn't it be easier to just say something like:


if (filename_spec_p(userstring))
else if (symbol_location_spec_p(userstring))


etc

We have like the same type of parsing going on in quite a few places, and
it just all seems completley ad-hoc.


>
> Thanks again for the feedback.
>

Re: [RFA] Improve completion of locations

[Eli Zaretskii]

> Date: Sun, 6 May 2001 12:38:00 -0400 (EDT)
> From: Daniel Berlin <dan@www.cgsoftware.com>
> 
> At 4am in the morning, everything seems miserable.
> :)

I can imagine ;-)

> > Note that Readline has its own ideas about breaking user input into
> > ``words'', and it does that even before our completion functions are
> > called.  So, in contrast to our code which parses the full location spec,
> > completion cannot be much smarter than it currently is, because Readline
> > doesn't give us a chance to be smarter.  Most of the time I debugged this
> > code went into trying to get along with Readline's idiosyncrasies.
> 
> Yeah, I figured as much.
> 
> Still, wouldn't it be easier to just say something like:
> 
> if (filename_spec_p(userstring))
> else if (symbol_location_spec_p(userstring))

Indeed, it would be great.  But this kind of stuff will only be
possible if several functions in symtab.c and wherenot which do their
own ``parsing'' will all be rewritten to use those new functions you
suggest.  If this is the plan, I'm with you all the way, I'm far from
happy wading through all that code each time I want to understand what
the heck is it doing, and inventing my own code to work around any of
its misfeatures.

Also, AFAICS there's no formal definition of the location syntax
accepted by GDB, so you'd probably need to set the rules as you go.
Which might be a Good Thing in and of itself.

Finally, if we want to solve this in a reliable way in the context of
completion, I think there's a real need to ask Readline for more hooks
than what it currently provides.  One thing that is sorely needed is
the way for GDB to tell Readline to use the right word-break
characters _before_ it breaks the line into ``words'' and calls the
completer.

> We have like the same type of parsing going on in quite a few places, and
> it just all seems completley ad-hoc.

Yes.  What's more, almost any more-or-less reasonable change you do in
that ad-hoc code won't change the end result except in a few marginal
and hard-to-come-by cases, so it is very hard to debug.

Re: [RFA] Improve completion of locations

[Elena Zannoni]

Eli Zaretskii writes:
 > I'm seeking approval for the patches below, which improve GDB's
 > completion of locations in commands like "break LOCATION".  The two
 > main improvements are:
 > 
 >   - GDB now considers file names as well as symbol names when you
 >     type, e.g., "break foo".  If there's a symbol foo_bar and a file
 >     foo-bar.c, both will be shown in the list of possible completions.
 >     Only files recorded in the debug info are used for completing file
 >     names for these commands.
 > 
 >   - When the user types "break foo.c:bar TAB", only symbols defined in
 >     the source file foo.c whose names begin with "bar" will be
 >     considered for completion.  This reduces the number of possible
 >     completions by a large factor in many typical situations (I'd even
 >     dare to say that it makes symbol completion a useful feature ;-).
 > 
 > This is the last installment of my recent completion-related frenzy.
 > I'm now quite happy with GDB's completion ;-)
 > 
 > AFAICS, the responsible maintainers who need to approve these patches
 > are Fernando, Elena, Michael Snyder, and someone for printcmd.c and
 > infcmd.c.
 > 

Eli, one minor problem with your patch is that your sources are not up
to date. The #include of completer.h in tracepoint.c and infcmd.c was
removed by JT on March 27. So you will have to add it back in when you
commit the patch.

Using a gdb with your patch applied to debug another gdb I am getting
something I don't understand:

(top-gdb) b symtab.<TAB>
Display all 10987 possibilities? (y or n)

what is it trying to do (note the '.')?

Using the ':' I get a list of completions which includes:
(top-gdb) b symtab.c:
__builtin_va_list                  int
_initialize_symtab                 keep_going
add_filename_to_list               long double
block_found                        long int
block_function                     long long int
bound                              long long unsigned int
builtin_type_error                 long unsigned int
char                               lookup_block_symbol
completion_list_add_name           lookup_partial_symbol
complex double                     lookup_partial_symtab
complex float                      lookup_symbol
complex int                        lookup_symbol_aux
complex long double                lookup_symtab
contained_in                       lookup_symtab_1
cplusplus_hint                     lookup_transparent_type
[...many more...]

But why am I getting the types names, like 'char', 'long double', etc?
They must be coming from the minsymbols, maybe. Do we want those?
Probably not.

(top-gdb) b symtab
symtab              symtab.h            symtab_fns          symtab_to_filename
symtab.c            symtab_and_line     symtab_symbol_info  symtabs_and_lines

(top-gdb) b symt<TAB>
symtab              symtab_and_line     symtab_to_filename  
symtab.c            symtab_fns          symtabs_and_lines   
symtab.h            symtab_symbol_info  symtoken            

(top-gdb) b symtab.c:make_<TAB>
make_cleanup_free_search_symbols   make_symbol_completion_list
make_file_symbol_completion_list   make_symbol_overload_list
make_source_files_completion_list  

These all seem to work now and didn't before, which is really neat.

I think some more refinements are needed.

Syntactically, there is a line I don't like :

 > +  (*list)[*list_used += 1] = NULL;

Could you just use '++'?


Sorry, I don't have much more time right now to look at it in more
detail. I think I am starting to understand this whole patch. I may
have some more comments tomorrow.

Elena


 > 2001-05-05  Eli Zaretskii  <eliz@is.elta.co.il>
 > 
 > 	* completer.c (gdb_completer_loc_break_characters): New variable.
 > 	(line_completion_function): If we are completing on locations,
 > 	back up the start of word pointer past all characters which can
 > 	appear in a location spec.
 > 	(location_completer): New function.
 > 
 > 	* completer.h: Add prototype for location_completer.
 > 
 > 	* symtab.c (make_source_files_completion_list)
 > 	(add_filename_to_list, not_interesting_fname): New functions.
 > 	(filename_seen): New function, body extracted from
 > 	output_source_filename.
 > 	(output_source_filename): Call filename_seen to check if the file
 > 	was already printed.
 > 	(make_symbol_completion_list): If TEXT includes a
 > 	double-quoted string, return an empty list, not NULL.
 > 	(make_file_symbol_completion_list): New function, similar to
 > 	make_symbol_completion_list but with an additional argument
 > 	SRCFILE.
 > 
 > 	* symtab.h (make_file_symbol_completion_list)
 > 	(make_source_files_completion_list): Add prototypes.
 > 
 > 	* breakpoint.c (_initialize_breakpoint): Make location_completer
 > 	be the completion function for all commands which set breakpoints
 > 	and watchpoints.
 > 	(top-level): #include "completer.h".
 > 
 > 	* tracepoint.c (_initialize_tracepoint): Make location_completer
 > 	be the completion function for the "trace" command.
 > 
 > 	* printcmd.c (_initialize_printcmd): Make location_completer be
 > 	the completion function for the "print", "inspect", "call", and
 > 	"disassemble" commands.
 > 	(top-level): #include "completer.h".
 > 
 > 	* infcmd.c (_initialize_infcmd): Make location_completer be the
 > 	completion function for the "go", "jump", and "until" commands.
 > 
 > 
 > --- gdb/completer.c~1	Sat Feb 17 12:22:10 2001
 > +++ gdb/completer.c	Sat May  5 20:20:24 2001
 > @@ -22,12 +22,14 @@
 >  #include "symtab.h"
 >  #include "gdbtypes.h"
 >  #include "expression.h"
 > +#include "filenames.h"		/* for DOSish file names */
 >  
 >  /* FIXME: This is needed because of lookup_cmd_1().
 >     We should be calling a hook instead so we eliminate the CLI dependency. */
 >  #include "gdbcmd.h"
 >  
 > -/* Needed for rl_completer_word_break_characters() */
 > +/* Needed for rl_completer_word_break_characters() and for
 > +   filename_completion_function.  */
 >  #include <readline/readline.h>
 >  
 >  /* readline defines this.  */
 > @@ -72,6 +74,10 @@ static char *gdb_completer_file_name_bre
 >  static char *gdb_completer_file_name_break_characters = " \t\n*|\"';:?><";
 >  #endif
 >  
 > +/* These are used when completing on locations, which can mix file
 > +   names and symbol names separated by a colon.  */
 > +static char *gdb_completer_loc_break_characters = " \t\n*|\"';:?><,";
 > +
 >  /* Characters that can be used to quote completion strings.  Note that we
 >     can't include '"' because the gdb C parser treats such quoted sequences
 >     as strings. */
 > @@ -95,8 +101,6 @@ get_gdb_completer_quote_characters (void
 >  char **
 >  filename_completer (char *text, char *word)
 >  {
 > -  /* From readline.  */
 > -extern char *filename_completion_function (char *, int);
 >    int subsequent_name;
 >    char **return_val;
 >    int return_val_used;
 > @@ -170,6 +174,151 @@ extern char *filename_completion_functio
 >    return return_val;
 >  }
 >  
 > +/* Complete on locations, which might be of two possible forms:
 > +
 > +       file:line
 > +   or
 > +       symbol+offset
 > +
 > +   This is intended to be used in commands that set breakpoints etc.  */
 > +char **
 > +location_completer (char *text, char *word)
 > +{
 > +  int n_syms = 0, n_files = 0;
 > +  char ** fn_list = NULL;
 > +  char ** list = NULL;
 > +  char *p;
 > +  int quote_found = 0;
 > +  int quoted = *text == '\'' || *text == '"';
 > +  int quote_char = '\0';
 > +  char *colon = NULL;
 > +  char *file_to_match = NULL;
 > +  char *symbol_start = text;
 > +  char *orig_text = text;
 > +  size_t text_len;
 > +
 > +  /* Do we have an unquoted colon, as in "break foo.c::bar"?  */
 > +  for (p = text; *p != '\0'; ++p)
 > +    {
 > +      if (*p == '\\' && p[1] == '\'')
 > +	p++;
 > +      else if (*p == '\'' || *p == '"')
 > +	{
 > +	  quote_found = *p;
 > +	  quote_char = *p++;
 > +	  while (*p != '\0' && *p != quote_found)
 > +	    {
 > +	      if (*p == '\\' && p[1] == quote_found)
 > +		p++;
 > +	      p++;
 > +	    }
 > +
 > +	  if (*p == quote_found)
 > +	    quote_found = 0;
 > +	}
 > +#if HAVE_DOS_BASED_FILE_SYSTEM
 > +      /* If we have a DOS-style absolute file name at the beginning of
 > +	 TEXT, and the colon after the drive letter is the only colon
 > +	 we found, pretend the colon is not there.  */
 > +      else if (p < text + 3 && *p == ':' && p == text + 1 + quoted)
 > +	;
 > +#endif
 > +      else if (*p == ':' && !colon)
 > +	{
 > +	  colon = p;
 > +	  symbol_start = p + 1;
 > +	}
 > +      else if (strchr (gdb_completer_word_break_characters, *p))
 > +	symbol_start = p + 1;
 > +    }
 > +
 > +  if (quoted)
 > +    text++;
 > +  text_len = strlen (text);
 > +
 > +  /* Where is the file name?  */
 > +  if (colon)
 > +    {
 > +      char *s;
 > +
 > +      file_to_match = (char *) xmalloc (colon - text + 1);
 > +      strncpy (file_to_match, text, colon - text + 1);
 > +      /* Remove trailing colons and quotes from the file name.  */
 > +      for (s = file_to_match + (colon - text);
 > +	   s > file_to_match;
 > +	   s--)
 > +	if (*s == ':' || *s == quote_char)
 > +	  *s = '\0';
 > +    }
 > +  /* If the text includes a colon, they want completion only on a
 > +     symbol name after the colon.  Otherwise, we need to complete on
 > +     symbols as well as on files.  */
 > +  if (colon)
 > +    {
 > +      list = make_file_symbol_completion_list (symbol_start, word,
 > +					       file_to_match);
 > +      xfree (file_to_match);
 > +    }
 > +  else
 > +    {
 > +      list = make_symbol_completion_list (symbol_start, word);
 > +      /* If text includes characters which cannot appear in a file
 > +	 name, they cannot be asking for completion on files.  */
 > +      if (strcspn (text, gdb_completer_file_name_break_characters) == text_len)
 > +	fn_list = make_source_files_completion_list (text, text);
 > +    }
 > +
 > +  /* How many completions do we have in both lists?  */
 > +  if (fn_list)
 > +    for ( ; fn_list[n_files]; n_files++)
 > +      ;
 > +  if (list)
 > +    for ( ; list[n_syms]; n_syms++)
 > +      ;
 > +
 > +  /* Make list[] large enough to hold both lists, then catenate
 > +     fn_list[] onto the end of list[].  */
 > +  if (n_syms && n_files)
 > +    {
 > +      list = xrealloc (list, (n_syms + n_files + 1) * sizeof (char *));
 > +      memcpy (list + n_syms, fn_list, (n_files + 1) * sizeof (char *));
 > +      xfree (fn_list);
 > +    }
 > +  else if (n_files)
 > +    {
 > +      /* If we only have file names as possible completion, we should
 > +	 bring them in sync with what rl_complete expects.  The
 > +	 problem is that if the user types "break /foo/b TAB", and the
 > +	 possible completions are "/foo/bar" and "/foo/baz"
 > +	 rl_complete expects us to return "bar" and "baz", without the
 > +	 leading directories, as possible completions, because `word'
 > +	 starts at the "b".  But we ignore the value of `word' when we
 > +	 call make_source_files_completion_list above (because that
 > +	 would not DTRT when the completion results in both symbols
 > +	 and file names), so make_source_files_completion_list returns
 > +	 the full "/foo/bar" and "/foo/baz" strings.  This produces
 > +	 wrong results when, e.g., there's only one possible
 > +	 completion, because rl_complete will prepend "/foo/" to each
 > +	 candidate completion.  The loop below removes that leading
 > +	 part.  */
 > +      for (n_files = 0; fn_list[n_files]; n_files++)
 > +	{
 > +	  memmove (fn_list[n_files], fn_list[n_files] + (word - text),
 > +		   strlen (fn_list[n_files]) + 1 - (word - text));
 > +	}
 > +      /* Return just the file-name list as the result.  */
 > +      list = fn_list;
 > +    }
 > +  else if (!n_syms)
 > +    {
 > +      /* No completions at all.  As the final resort, try completing
 > +	 on the entire text as a symbol.  */
 > +      list = make_symbol_completion_list (orig_text, word);
 > +    }
 > +
 > +  return list;
 > +}
 > +
 >  /* Here are some useful test cases for completion.  FIXME: These should
 >     be put in the test suite.  They should be tested with both M-? and TAB.
 >  
 > @@ -362,7 +511,7 @@ line_completion_function (char *text, in
 >  			     to complete the entire text after the
 >  			     command, just the last word.  To this
 >  			     end, we need to find the beginning of the
 > -			     file name starting at `word' and going
 > +			     file name by starting at `word' and going
 >  			     backwards.  */
 >  			  for (p = word;
 >  			       p > tmp_command
 > @@ -372,6 +521,16 @@ line_completion_function (char *text, in
 >  			  rl_completer_word_break_characters =
 >  			    gdb_completer_file_name_break_characters;
 >  			}
 > +		      else if (c->completer == location_completer)
 > +			{
 > +			  /* Commands which complete on locations want to
 > +			     see the entire argument.  */
 > +			  for (p = word;
 > +			       p > tmp_command
 > +				 && p[-1] != ' ' && p[-1] != '\t';
 > +			       p--)
 > +			    ;
 > +			}
 >  		      list = (*c->completer) (p, word);
 >  		    }
 >  		}
 > @@ -430,6 +589,14 @@ line_completion_function (char *text, in
 >  		      rl_completer_word_break_characters =
 >  			gdb_completer_file_name_break_characters;
 >  		    }
 > +		  else if (c->completer == location_completer)
 > +		    {
 > +		      for (p = word;
 > +			   p > tmp_command
 > +			     && p[-1] != ' ' && p[-1] != '\t';
 > +			   p--)
 > +			;
 > +		    }
 >  		  list = (*c->completer) (p, word);
 >  		}
 >  	    }
 > 
 > --- gdb/completer.h~	Fri Dec  1 02:41:26 2000
 > +++ gdb/completer.h	Sat Apr  7 16:44:22 2001
 > @@ -23,6 +23,8 @@ extern char *line_completion_function (c
 >  
 >  extern char **filename_completer (char *, char *);
 >  
 > +extern char **location_completer (char *, char *);
 > +
 >  extern char *get_gdb_completer_word_break_characters (void); 
 >  
 >  extern char *get_gdb_completer_quote_characters (void);
 > 
 > --- gdb/symtab.c~2	Fri Mar 30 14:57:20 2001
 > +++ gdb/symtab.c	Sat May  5 18:56:00 2001
 > @@ -2161,50 +2147,72 @@ operator_chars (char *p, char **end)
 >  }
 >  \f
 >  
 > -/* Slave routine for sources_info.  Force line breaks at ,'s.
 > -   NAME is the name to print and *FIRST is nonzero if this is the first
 > -   name printed.  Set *FIRST to zero.  */
 > -static void
 > -output_source_filename (char *name, int *first)
 > +/* If FILE is not already in the table of files, return zero;
 > +   otherwise return non-zero.  Optionally add FILE to the table if ADD
 > +   is non-zero.  If *FIRST is non-zero, forget the old table
 > +   contents.  */
 > +static int
 > +filename_seen (const char *file, int add, int *first)
 >  {
 > -  /* Table of files printed so far.  Since a single source file can
 > -     result in several partial symbol tables, we need to avoid printing
 > -     it more than once.  Note: if some of the psymtabs are read in and
 > -     some are not, it gets printed both under "Source files for which
 > -     symbols have been read" and "Source files for which symbols will
 > -     be read in on demand".  I consider this a reasonable way to deal
 > -     with the situation.  I'm not sure whether this can also happen for
 > -     symtabs; it doesn't hurt to check.  */
 > -  static char **tab = NULL;
 > +  /* Table of files seen so far.  */
 > +  static const char **tab = NULL;
 >    /* Allocated size of tab in elements.
 >       Start with one 256-byte block (when using GNU malloc.c).
 >       24 is the malloc overhead when range checking is in effect.  */
 >    static int tab_alloc_size = (256 - 24) / sizeof (char *);
 >    /* Current size of tab in elements.  */
 >    static int tab_cur_size;
 > -
 > -  char **p;
 > +  const char **p;
 >  
 >    if (*first)
 >      {
 >        if (tab == NULL)
 > -	tab = (char **) xmalloc (tab_alloc_size * sizeof (*tab));
 > +	tab = (const char **) xmalloc (tab_alloc_size * sizeof (*tab));
 >        tab_cur_size = 0;
 >      }
 >  
 > -  /* Is NAME in tab?  */
 > +  /* Is FILE in tab?  */
 >    for (p = tab; p < tab + tab_cur_size; p++)
 > -    if (STREQ (*p, name))
 > -      /* Yes; don't print it again.  */
 > -      return;
 > -  /* No; add it to tab.  */
 > -  if (tab_cur_size == tab_alloc_size)
 > +    if (strcmp (*p, file) == 0)
 > +      return 1;
 > +
 > +  /* No; maybe add it to tab.  */
 > +  if (add)
 >      {
 > -      tab_alloc_size *= 2;
 > -      tab = (char **) xrealloc ((char *) tab, tab_alloc_size * sizeof (*tab));
 > +      if (tab_cur_size == tab_alloc_size)
 > +	{
 > +	  tab_alloc_size *= 2;
 > +	  tab = (const char **) xrealloc ((char *) tab,
 > +					  tab_alloc_size * sizeof (*tab));
 > +	}
 > +      tab[tab_cur_size++] = file;
 >      }
 > -  tab[tab_cur_size++] = name;
 >  
 > +  return 0;
 > +}
 > +
 > +/* Slave routine for sources_info.  Force line breaks at ,'s.
 > +   NAME is the name to print and *FIRST is nonzero if this is the first
 > +   name printed.  Set *FIRST to zero.  */
 > +static void
 > +output_source_filename (char *name, int *first)
 > +{
 > +  /* Since a single source file can result in several partial symbol
 > +     tables, we need to avoid printing it more than once.  Note: if
 > +     some of the psymtabs are read in and some are not, it gets
 > +     printed both under "Source files for which symbols have been
 > +     read" and "Source files for which symbols will be read in on
 > +     demand".  I consider this a reasonable way to deal with the
 > +     situation.  I'm not sure whether this can also happen for
 > +     symtabs; it doesn't hurt to check.  */
 > +
 > +  /* Was NAME already seen?  */
 > +  if (filename_seen (name, 1, first))
 > +    {
 > +      /* Yes; don't print it again.  */
 > +      return;
 > +    }
 > +  /* No; print it and reset *FIRST.  */
 >    if (*first)
 >      {
 >        *first = 0;
 > @@ -2871,9 +2879,9 @@ completion_list_add_name (char *symname,
 >    }
 >  }
 >  
 > -/* Return a NULL terminated array of all symbols (regardless of class) which
 > -   begin by matching TEXT.  If the answer is no symbols, then the return value
 > -   is an array which contains only a NULL pointer.
 > +/* Return a NULL terminated array of all symbols (regardless of class)
 > +   which begin by matching TEXT.  If the answer is no symbols, then
 > +   the return value is an array which contains only a NULL pointer.
 >  
 >     Problem: All of the symbols have to be copied because readline frees them.
 >     I'm not going to worry about this; hopefully there won't be that many.  */
 > @@ -2927,7 +2935,11 @@ make_symbol_completion_list (char *text,
 >      else if (quote_found == '"')
 >        /* A double-quoted string is never a symbol, nor does it make sense
 >           to complete it any other way.  */
 > -      return NULL;
 > +      {
 > +	return_val = (char **) xmalloc (sizeof (char *));
 > +	return_val[0] = NULL;
 > +	return return_val;
 > +      }
 >      else
 >        {
 >  	/* It is not a quoted string.  Break it based on the characters
 > @@ -3059,6 +3071,277 @@ make_symbol_completion_list (char *text,
 >    return (return_val);
 >  }
 >  
 > +/* Like make_symbol_completion_list, but returns a list of symbols
 > +   defined in a source file FILE.  */
 > +
 > +char **
 > +make_file_symbol_completion_list (char *text, char *word, char *srcfile)
 > +{
 > +  register struct symbol *sym;
 > +  register struct symtab *s;
 > +  register struct block *b;
 > +  register int i;
 > +  /* The symbol we are completing on.  Points in same buffer as text.  */
 > +  char *sym_text;
 > +  /* Length of sym_text.  */
 > +  int sym_text_len;
 > +
 > +  /* Now look for the symbol we are supposed to complete on.
 > +     FIXME: This should be language-specific.  */
 > +  {
 > +    char *p;
 > +    char quote_found;
 > +    char *quote_pos = NULL;
 > +
 > +    /* First see if this is a quoted string.  */
 > +    quote_found = '\0';
 > +    for (p = text; *p != '\0'; ++p)
 > +      {
 > +	if (quote_found != '\0')
 > +	  {
 > +	    if (*p == quote_found)
 > +	      /* Found close quote.  */
 > +	      quote_found = '\0';
 > +	    else if (*p == '\\' && p[1] == quote_found)
 > +	      /* A backslash followed by the quote character
 > +	         doesn't end the string.  */
 > +	      ++p;
 > +	  }
 > +	else if (*p == '\'' || *p == '"')
 > +	  {
 > +	    quote_found = *p;
 > +	    quote_pos = p;
 > +	  }
 > +      }
 > +    if (quote_found == '\'')
 > +      /* A string within single quotes can be a symbol, so complete on it.  */
 > +      sym_text = quote_pos + 1;
 > +    else if (quote_found == '"')
 > +      /* A double-quoted string is never a symbol, nor does it make sense
 > +         to complete it any other way.  */
 > +      {
 > +	return_val = (char **) xmalloc (sizeof (char *));
 > +	return_val[0] = NULL;
 > +	return return_val;
 > +      }
 > +    else
 > +      {
 > +	/* It is not a quoted string.  Break it based on the characters
 > +	   which are in symbols.  */
 > +	while (p > text)
 > +	  {
 > +	    if (isalnum (p[-1]) || p[-1] == '_' || p[-1] == '\0')
 > +	      --p;
 > +	    else
 > +	      break;
 > +	  }
 > +	sym_text = p;
 > +      }
 > +  }
 > +
 > +  sym_text_len = strlen (sym_text);
 > +
 > +  return_val_size = 10;
 > +  return_val_index = 0;
 > +  return_val = (char **) xmalloc ((return_val_size + 1) * sizeof (char *));
 > +  return_val[0] = NULL;
 > +
 > +  /* Find the symtab for SRCFILE (this loads it if it was not yet read
 > +     in).  */
 > +  s = lookup_symtab (srcfile);
 > +  if (s == NULL)
 > +    {
 > +      /* Maybe they typed the file with leading directories, while the
 > +	 symbol tables record only its basename.  */
 > +      char *tail = basename (srcfile);
 > +
 > +      if (tail > srcfile)
 > +	s = lookup_symtab (tail);
 > +    }
 > +
 > +  /* If we have no symtab for that file, return an empty list.  */
 > +  if (s == NULL)
 > +    return (return_val);
 > +
 > +  /* Go through this symtab and check the externs and statics for
 > +     symbols which match.  */
 > +
 > +  b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
 > +  for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +    {
 > +      sym = BLOCK_SYM (b, i);
 > +      COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
 > +    }
 > +
 > +  b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), STATIC_BLOCK);
 > +  for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +    {
 > +      sym = BLOCK_SYM (b, i);
 > +      COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
 > +    }
 > +
 > +  return (return_val);
 > +}
 > +
 > +/* A helper function for make_source_files_completion_list.  It adds
 > +   another file name to a list of possible completions, growing the
 > +   list as necessary.  */
 > +
 > +static void
 > +add_filename_to_list (const char *fname, char *text, char *word,
 > +		      char ***list, int *list_used, int *list_alloced)
 > +{
 > +  char *new;
 > +  size_t fnlen = strlen (fname);
 > +
 > +  if (*list_used + 1 >= *list_alloced)
 > +    {
 > +      *list_alloced *= 2;
 > +      *list = (char **) xrealloc ((char *) *list,
 > +				  *list_alloced * sizeof (char *));
 > +    }
 > +
 > +  if (word == text)
 > +    {
 > +      /* Return exactly fname.  */
 > +      new = xmalloc (fnlen + 5);
 > +      strcpy (new, fname);
 > +    }
 > +  else if (word > text)
 > +    {
 > +      /* Return some portion of fname.  */
 > +      new = xmalloc (fnlen + 5);
 > +      strcpy (new, fname + (word - text));
 > +    }
 > +  else
 > +    {
 > +      /* Return some of TEXT plus fname.  */
 > +      new = xmalloc (fnlen + (text - word) + 5);
 > +      strncpy (new, word, text - word);
 > +      new[text - word] = '\0';
 > +      strcat (new, fname);
 > +    }
 > +  (*list)[*list_used] = new;
 > +  (*list)[*list_used += 1] = NULL;
 > +}
 > +
 > +static int
 > +not_interesting_fname (const char *fname)
 > +{
 > +  static const char *illegal_aliens[] = {
 > +    "_globals_",	/* inserted by coff_symtab_read */
 > +    NULL
 > +  };
 > +  int i;
 > +
 > +  for (i = 0; illegal_aliens[i]; i++)
 > +    {
 > +      if (strcmp (fname, illegal_aliens[i]) == 0)
 > +	return 1;
 > +    }
 > +  return 0;
 > +}
 > +
 > +/* Return a NULL terminated array of all source files whose names
 > +   begin with matching TEXT.  The file names are looked up in the
 > +   symbol tables of this program.  If the answer is no matchess, then
 > +   the return value is an array which contains only a NULL pointer.  */
 > +
 > +char **
 > +make_source_files_completion_list (char *text, char *word)
 > +{
 > +  register struct symtab *s;
 > +  register struct partial_symtab *ps;
 > +  register struct objfile *objfile;
 > +  int first = 1;
 > +  int list_alloced = 1;
 > +  int list_used = 0;
 > +  size_t text_len = strlen (text);
 > +  char **list = (char **) xmalloc (list_alloced * sizeof (char *));
 > +  char *base_name;
 > +
 > +  list[0] = NULL;
 > +
 > +  if (!have_full_symbols () && !have_partial_symbols ())
 > +    return list;
 > +
 > +  ALL_SYMTABS (objfile, s)
 > +    {
 > +      if (not_interesting_fname (s->filename))
 > +	continue;
 > +      if (!filename_seen (s->filename, 1, &first)
 > +#if HAVE_DOS_BASED_FILE_SYSTEM
 > +	  && strncasecmp (s->filename, text, text_len) == 0
 > +#else
 > +	  && strncmp (s->filename, text, text_len) == 0
 > +#endif
 > +	  )
 > +	{
 > +	  /* This file matches for a completion; add it to the current
 > +	     list of matches.  */
 > +	  add_filename_to_list (s->filename, text, word,
 > +				&list, &list_used, &list_alloced);
 > +	}
 > +      else
 > +	{
 > +	  /* NOTE: We allow the user to type a base name when the
 > +	     debug info records leading directories, but not the other
 > +	     way around.  This is what subroutines of breakpoint
 > +	     command do when they parse file names.  */
 > +	  base_name = basename (s->filename);
 > +	  if (base_name != s->filename
 > +	      && !filename_seen (base_name, 1, &first)
 > +#if HAVE_DOS_BASED_FILE_SYSTEM
 > +	      && strncasecmp (base_name, text, text_len) == 0
 > +#else
 > +	      && strncmp (base_name, text, text_len) == 0
 > +#endif
 > +	      )
 > +	    add_filename_to_list (base_name, text, word,
 > +				  &list, &list_used, &list_alloced);
 > +	}
 > +    }
 > +
 > +  ALL_PSYMTABS (objfile, ps)
 > +    {
 > +      if (not_interesting_fname (ps->filename))
 > +	continue;
 > +      if (!ps->readin)
 > +	{
 > +	  if (!filename_seen (ps->filename, 1, &first)
 > +#if HAVE_DOS_BASED_FILE_SYSTEM
 > +	      && strncasecmp (ps->filename, text, text_len) == 0
 > +#else
 > +	      && strncmp (ps->filename, text, text_len) == 0
 > +#endif
 > +	      )
 > +	    {
 > +	      /* This file matches for a completion; add it to the
 > +		 current list of matches.  */
 > +	      add_filename_to_list (ps->filename, text, word,
 > +				    &list, &list_used, &list_alloced);
 > +
 > +	    }
 > +	  else
 > +	    {
 > +	      base_name = basename (ps->filename);
 > +	      if (base_name != ps->filename
 > +		  && !filename_seen (base_name, 1, &first)
 > +#if HAVE_DOS_BASED_FILE_SYSTEM
 > +		  && strncasecmp (base_name, text, text_len) == 0
 > +#else
 > +		  && strncmp (base_name, text, text_len) == 0
 > +#endif
 > +		  )
 > +		add_filename_to_list (base_name, text, word,
 > +				      &list, &list_used, &list_alloced);
 > +	    }
 > +	}
 > +    }
 > +
 > +  return list;
 > +}
 > +
 >  /* Determine if PC is in the prologue of a function.  The prologue is the area
 >     between the first instruction of a function, and the first executable line.
 >     Returns 1 if PC *might* be in prologue, 0 if definately *not* in prologue.
 > 
 > --- gdb/symtab.h~2	Fri Feb  2 22:01:16 2001
 > +++ gdb/symtab.h	Sat Apr 28 14:00:24 2001
 > @@ -1386,8 +1386,12 @@ extern void select_source_symtab (struct
 >  
 >  extern char **make_symbol_completion_list (char *, char *);
 >  
 > +extern char **make_file_symbol_completion_list (char *, char *, char *);
 > +
 >  extern struct symbol **make_symbol_overload_list (struct symbol *);
 >  
 > +extern char **make_source_files_completion_list (char *, char *);
 > +
 >  /* symtab.c */
 >  
 >  extern struct partial_symtab *find_main_psymtab (void);
 > 
 > --- gdb/breakpoint.c~2	Sun Mar 18 20:34:06 2001
 > +++ gdb/breakpoint.c	Sat May  5 19:05:36 2001
 > @@ -40,6 +40,7 @@
 >  #include "symfile.h"
 >  #include "objfiles.h"
 >  #include "linespec.h"
 > +#include "completer.h"
 >  #ifdef UI_OUT
 >  #include "ui-out.h"
 >  #endif
 > @@ -7520,24 +7521,29 @@ then no output is printed when it is hit
 >  Usage is `condition N COND', where N is an integer and COND is an\n\
 >  expression to be evaluated whenever breakpoint N is reached.  ");
 >  
 > -  add_com ("tbreak", class_breakpoint, tbreak_command,
 > -	   "Set a temporary breakpoint.  Args like \"break\" command.\n\
 > +  c = add_com ("tbreak", class_breakpoint, tbreak_command,
 > +	       "Set a temporary breakpoint.  Args like \"break\" command.\n\
 >  Like \"break\" except the breakpoint is only temporary,\n\
 >  so it will be deleted when hit.  Equivalent to \"break\" followed\n\
 >  by using \"enable delete\" on the breakpoint number.");
 > -  add_com ("txbreak", class_breakpoint, tbreak_at_finish_command,
 > -	   "Set temporary breakpoint at procedure exit.  Either there should\n\
 > +  c->completer = location_completer;
 > +
 > +  c = add_com ("txbreak", class_breakpoint, tbreak_at_finish_command,
 > +	       "Set temporary breakpoint at procedure exit.  Either there should\n\
 >  be no argument or the argument must be a depth.\n");
 > +  c->completer = location_completer;
 >  
 > -  add_com ("hbreak", class_breakpoint, hbreak_command,
 > -	   "Set a hardware assisted  breakpoint. Args like \"break\" command.\n\
 > +  c = add_com ("hbreak", class_breakpoint, hbreak_command,
 > +	       "Set a hardware assisted  breakpoint. Args like \"break\" command.\n\
 >  Like \"break\" except the breakpoint requires hardware support,\n\
 >  some target hardware may not have this support.");
 > +  c->completer = location_completer;
 >  
 > -  add_com ("thbreak", class_breakpoint, thbreak_command,
 > -	   "Set a temporary hardware assisted breakpoint. Args like \"break\" command.\n\
 > +  c = add_com ("thbreak", class_breakpoint, thbreak_command,
 > +	       "Set a temporary hardware assisted breakpoint. Args like \"break\" command.\n\
 >  Like \"hbreak\" except the breakpoint is only temporary,\n\
 >  so it will be deleted when hit.");
 > +  c->completer = location_completer;
 >  
 >    add_prefix_cmd ("enable", class_breakpoint, enable_command,
 >  		  "Enable some breakpoints.\n\
 > @@ -7639,8 +7645,8 @@ is executing in.\n\
 >  \n\
 >  See also the \"delete\" command which clears breakpoints by number.", NULL));
 >  
 > -  add_com ("break", class_breakpoint, break_command,
 > -	   concat ("Set breakpoint at specified line or function.\n\
 > +  c = add_com ("break", class_breakpoint, break_command,
 > +	       concat ("Set breakpoint at specified line or function.\n\
 >  Argument may be line number, function name, or \"*\" and an address.\n\
 >  If line number is specified, break at start of code for that line.\n\
 >  If function is specified, break at start of code for that function.\n\
 > @@ -7651,6 +7657,8 @@ This is useful for breaking on return to
 >  Multiple breakpoints at one place are permitted, and useful if conditional.\n\
 >  \n\
 >  Do \"help breakpoints\" for info on other commands dealing with breakpoints.", NULL));
 > +  c->completer = location_completer;
 > +
 >    add_com_alias ("b", "break", class_run, 1);
 >    add_com_alias ("br", "break", class_run, 1);
 >    add_com_alias ("bre", "break", class_run, 1);
 > @@ -7796,20 +7804,23 @@ Like \"catch\" except the catchpoint is 
 >  so it will be deleted when hit.  Equivalent to \"catch\" followed\n\
 >  by using \"enable delete\" on the catchpoint number.");
 >  
 > -  add_com ("watch", class_breakpoint, watch_command,
 > -	   "Set a watchpoint for an expression.\n\
 > +  c = add_com ("watch", class_breakpoint, watch_command,
 > +	       "Set a watchpoint for an expression.\n\
 >  A watchpoint stops execution of your program whenever the value of\n\
 >  an expression changes.");
 > +  c->completer = location_completer;
 >  
 > -  add_com ("rwatch", class_breakpoint, rwatch_command,
 > -	   "Set a read watchpoint for an expression.\n\
 > +  c = add_com ("rwatch", class_breakpoint, rwatch_command,
 > +	       "Set a read watchpoint for an expression.\n\
 >  A watchpoint stops execution of your program whenever the value of\n\
 >  an expression is read.");
 > +  c->completer = location_completer;
 >  
 > -  add_com ("awatch", class_breakpoint, awatch_command,
 > -	   "Set a watchpoint for an expression.\n\
 > +  c = add_com ("awatch", class_breakpoint, awatch_command,
 > +	       "Set a watchpoint for an expression.\n\
 >  A watchpoint stops execution of your program whenever the value of\n\
 >  an expression is either read or written.");
 > +  c->completer = location_completer;
 >  
 >    add_info ("watchpoints", breakpoints_info,
 >  	    "Synonym for ``info breakpoints''.");
 > 
 > --- gdb/tracepoint.c~2	Sat Feb 17 22:42:54 2001
 > +++ gdb/tracepoint.c	Sat May  5 19:33:50 2001
 > @@ -2774,12 +2774,13 @@ Arguments are tracepoint numbers, separa
 >  No argument means enable all tracepoints.",
 >  	   &enablelist);
 >  
 > -  add_com ("trace", class_trace, trace_command,
 > -	   "Set a tracepoint at a specified line or function or address.\n\
 > +  c = add_com ("trace", class_trace, trace_command,
 > +	       "Set a tracepoint at a specified line or function or address.\n\
 >  Argument may be a line number, function name, or '*' plus an address.\n\
 >  For a line number or function, trace at the start of its code.\n\
 >  If an address is specified, trace at that exact address.\n\n\
 >  Do \"help tracepoints\" for info on other tracepoint commands.");
 > +  c->completer = location_completer;
 >  
 >    add_com_alias ("tp", "trace", class_alias, 0);
 >    add_com_alias ("tr", "trace", class_alias, 1);
 > 
 > --- gdb/printcmd.c~2	Fri Dec 15 03:01:48 2000
 > +++ gdb/printcmd.c	Sat May  5 19:38:42 2001
 > @@ -37,6 +37,7 @@
 >  #include "annotate.h"
 >  #include "symfile.h"		/* for overlay functions */
 >  #include "objfiles.h"		/* ditto */
 > +#include "completer.h"		/* for completion functions */
 >  #ifdef UI_OUT
 >  #include "ui-out.h"
 >  #endif
 > @@ -2452,6 +2453,8 @@ print_insn (CORE_ADDR memaddr, struct ui
 >  void
 >  _initialize_printcmd (void)
 >  {
 > +  struct cmd_list_element *c;
 > +
 >    current_display_number = -1;
 >  
 >    add_info ("address", address_info,
 > @@ -2474,11 +2477,12 @@ Defaults for format and size letters are
 >  Default count is 1.  Default address is following last thing printed\n\
 >  with this command or \"print\".", NULL));
 >  
 > -  add_com ("disassemble", class_vars, disassemble_command,
 > -	   "Disassemble a specified section of memory.\n\
 > +  c = add_com ("disassemble", class_vars, disassemble_command,
 > +	       "Disassemble a specified section of memory.\n\
 >  Default is the function surrounding the pc of the selected frame.\n\
 >  With a single argument, the function surrounding that address is dumped.\n\
 >  Two arguments are taken as a range of memory to dump.");
 > +  c->completer = location_completer;
 >    if (xdb_commands)
 >      add_com_alias ("va", "disassemble", class_xdb, 0);
 >  
 > @@ -2556,11 +2560,12 @@ variable in the program being debugged. 
 >  You can see these environment settings with the \"show\" command.", NULL));
 >  
 >    /* "call" is the same as "set", but handy for dbx users to call fns. */
 > -  add_com ("call", class_vars, call_command,
 > -	   "Call a function in the program.\n\
 > +  c = add_com ("call", class_vars, call_command,
 > +	       "Call a function in the program.\n\
 >  The argument is the function name and arguments, in the notation of the\n\
 >  current working language.  The result is printed and saved in the value\n\
 >  history, if it is not void.");
 > +  c->completer = location_completer;
 >  
 >    add_cmd ("variable", class_vars, set_command,
 >  	   "Evaluate expression EXP and assign result to variable VAR, using assignment\n\
 > @@ -2571,7 +2576,7 @@ variable in the program being debugged. 
 >  This may usually be abbreviated to simply \"set\".",
 >  	   &setlist);
 >  
 > -  add_com ("print", class_vars, print_command,
 > +  c = add_com ("print", class_vars, print_command,
 >  	   concat ("Print value of expression EXP.\n\
 >  Variables accessible are those of the lexical environment of the selected\n\
 >  stack frame, plus all those whose scope is global or an entire file.\n\
 > @@ -2593,11 +2598,13 @@ resides in memory.\n",
 >  		   "\n\
 >  EXP may be preceded with /FMT, where FMT is a format letter\n\
 >  but no count or size letter (see \"x\" command).", NULL));
 > +  c->completer = location_completer;
 >    add_com_alias ("p", "print", class_vars, 1);
 >  
 > -  add_com ("inspect", class_vars, inspect_command,
 > +  c = add_com ("inspect", class_vars, inspect_command,
 >  	   "Same as \"print\" command, except that if you are running in the epoch\n\
 >  environment, the value is printed in its own window.");
 > +  c->completer = location_completer;
 >  
 >    add_show_from_set (
 >  		 add_set_cmd ("max-symbolic-offset", no_class, var_uinteger,
 > 
 > --- gdb/infcmd.c~2	Mon Feb 12 23:03:02 2001
 > +++ gdb/infcmd.c	Sat May  5 19:23:36 2001
 > @@ -1793,8 +1793,8 @@ _initialize_infcmd (void)
 >  {
 >    struct cmd_list_element *c;
 >  
 > -  c= add_com ("tty", class_run, tty_command,
 > -	      "Set terminal for future runs of program being debugged.");
 > +  c = add_com ("tty", class_run, tty_command,
 > +	       "Set terminal for future runs of program being debugged.");
 >    c->completer = filename_completer;
 >  
 >    add_show_from_set
 > @@ -1899,25 +1899,30 @@ Argument N means do this N times (or til
 >  Argument N means do this N times (or till program stops for another reason).");
 >    add_com_alias ("s", "step", class_run, 1);
 >  
 > -  add_com ("until", class_run, until_command,
 > -	   "Execute until the program reaches a source line greater than the current\n\
 > +  c = add_com ("until", class_run, until_command,
 > +	       "Execute until the program reaches a source line greater than the current\n\
 >  or a specified line or address or function (same args as break command).\n\
 >  Execution will also stop upon exit from the current stack frame.");
 > +  c->completer = location_completer;
 >    add_com_alias ("u", "until", class_run, 1);
 >  
 > -  add_com ("jump", class_run, jump_command,
 > -	   "Continue program being debugged at specified line or address.\n\
 > +  c = add_com ("jump", class_run, jump_command,
 > +	       "Continue program being debugged at specified line or address.\n\
 >  Give as argument either LINENUM or *ADDR, where ADDR is an expression\n\
 >  for an address to start at.");
 > +  c->completer = location_completer;
 >  
 >    if (xdb_commands)
 > -    add_com ("go", class_run, go_command,
 > -	     "Usage: go <location>\n\
 > +    {
 > +      c = add_com ("go", class_run, go_command,
 > +		   "Usage: go <location>\n\
 >  Continue program being debugged, stopping at specified line or \n\
 >  address.\n\
 >  Give as argument either LINENUM or *ADDR, where ADDR is an \n\
 >  expression for an address to start at.\n\
 >  This command is a combination of tbreak and jump.");
 > +      c->completer = location_completer;
 > +    }
 >  
 >    if (xdb_commands)
 >      add_com_alias ("g", "go", class_run, 1);
 > 

Re: [RFA] Improve completion of locations

[Eli Zaretskii]

On Tue, 8 May 2001, Elena Zannoni wrote:

> Eli, one minor problem with your patch is that your sources are not up
> to date. The #include of completer.h in tracepoint.c and infcmd.c was
> removed by JT on March 27. So you will have to add it back in when you
> commit the patch.

Okay, thanks for the heads-up.

> Using a gdb with your patch applied to debug another gdb I am getting
> something I don't understand:
> 
> (top-gdb) b symtab.<TAB>
> Display all 10987 possibilities? (y or n)
> 
> what is it trying to do (note the '.')?

Granted, I've bumped into this as well.  The immediate problem here is
that Readline passes an empty string to our completion function,
because `.' is not a word-constituent character.  The completer then
simply comes up with a list of all the symbols in the program.

I thought about writing special code for this specific case, which
would realize that this is a file name without an extension.  However,
it isn't clear that this is the right thing to do, because the user
could well mean to write symtab.foobar, meaning the field foobar of
the struct named symtab.  GDB has no simple way of knowing what case
it is faced with.  Also, the symbol completion is not smart enough to
see if there is indeed a symbol named symtab, if it is a struct or a
class, and only list members of that struct after the dot.

When presented with the same input, the original completion would also
do the same, so this isn't a regression.

If you (or someone else) has suggestions how to improve this, I'd be
glad to hear them.  But this would be an additional improvement.

> Using the ':' I get a list of completions which includes:
> (top-gdb) b symtab.c:
> __builtin_va_list                  int
> _initialize_symtab                 keep_going
> add_filename_to_list               long double
> block_found                        long int
> block_function                     long long int
> bound                              long long unsigned int
> builtin_type_error                 long unsigned int
> char                               lookup_block_symbol
> completion_list_add_name           lookup_partial_symbol
> complex double                     lookup_partial_symtab
> complex float                      lookup_symbol
> complex int                        lookup_symbol_aux
> complex long double                lookup_symtab
> contained_in                       lookup_symtab_1
> cplusplus_hint                     lookup_transparent_type
> [...many more...]
> 
> But why am I getting the types names, like 'char', 'long double', etc?
> They must be coming from the minsymbols, maybe.

I saw this, but my testing indicates that these symbols are indeed
inside the symtab for the file (symtab.c, in this case), not inside a
minsym.  In fact, the function make_file_symbol_completion_list which
I added doesn't even look at minsyms.  AFAIU, the symtab for a file
holds all the data types that were visible to the compiler when the
file was compiled.  If you see something different, please tell,
perhaps there's a bug in my code.

> Do we want those?  Probably not.

Probably, but not positively.  Someone could type something like this:

   (gdb) break symtab.c:*(long *)&foobar

> (top-gdb) b symtab
> symtab              symtab.h            symtab_fns          symtab_to_filename
> symtab.c            symtab_and_line     symtab_symbol_info  symtabs_and_lines
> 
> (top-gdb) b symt<TAB>
> symtab              symtab_and_line     symtab_to_filename  
> symtab.c            symtab_fns          symtabs_and_lines   
> symtab.h            symtab_symbol_info  symtoken            
> 
> (top-gdb) b symtab.c:make_<TAB>
> make_cleanup_free_search_symbols   make_symbol_completion_list
> make_file_symbol_completion_list   make_symbol_overload_list
> make_source_files_completion_list  
> 
> These all seem to work now and didn't before, which is really neat.
> 
> I think some more refinements are needed.

I'd be glad to make this more smart, but it's not simple.  By far the
nastiest problem is that Readline decides what is the word which is
being completed on _before_ our completion function is called.  This
defeats many nifty tricks we could do using the context of the
completion.

But I'm open to ideas which would make completion even more useful.

> Syntactically, there is a line I don't like :
> 
>  > +  (*list)[*list_used += 1] = NULL;
> 
> Could you just use '++'?

Yes; I will change that.

> Sorry, I don't have much more time right now to look at it in more
> detail. I think I am starting to understand this whole patch. I may
> have some more comments tomorrow.

Thanks.

Re: [RFA] Improve completion of locations

[Elena Zannoni]

Eli Zaretskii writes:
 > 
 > On Tue, 8 May 2001, Elena Zannoni wrote:
 > 
 > > Eli, one minor problem with your patch is that your sources are not up
 > > to date. The #include of completer.h in tracepoint.c and infcmd.c was
 > > removed by JT on March 27. So you will have to add it back in when you
 > > commit the patch.
 > 
 > Okay, thanks for the heads-up.
 > 
 > > Using a gdb with your patch applied to debug another gdb I am getting
 > > something I don't understand:
 > > 
 > > (top-gdb) b symtab.<TAB>
 > > Display all 10987 possibilities? (y or n)
 > > 
 > > what is it trying to do (note the '.')?
 > 
 > Granted, I've bumped into this as well.  The immediate problem here is
 > that Readline passes an empty string to our completion function,
 > because `.' is not a word-constituent character.  The completer then
 > simply comes up with a list of all the symbols in the program.
 > 

[Sorry I got sidetracked on other things...]

I was wondering how readline would get this right in case of
completion on filenames. I had a closer look, and also in that case it
gets it wrong. '.' is a word-break character, but not a filename-break
character. The first time rl_complete() is invoked by pressing TAB,
readline has the set of work-break characters installed. So it ends up
calling line_completion_function() with an empty string, just like
above.  But line completion function switches the set of break chars
from under readline's nose, installing the file-break ones, and
basically redoing what readline got wrong before. So later on, gdb
will try to complete on 'symtab.' and get it right. Fascinating.


 > I thought about writing special code for this specific case, which
 > would realize that this is a file name without an extension.  However,
 > it isn't clear that this is the right thing to do, because the user
 > could well mean to write symtab.foobar, meaning the field foobar of
 > the struct named symtab.  GDB has no simple way of knowing what case
 > it is faced with.  Also, the symbol completion is not smart enough to
 > see if there is indeed a symbol named symtab, if it is a struct or a
 > class, and only list members of that struct after the dot.

Yes, we could play a trick like for filename_completer, but you are
right, it could be a structure/class name. So we are stuck.

 > 
 > When presented with the same input, the original completion would also
 > do the same, so this isn't a regression.

BTW, I was playing with an older gdb, and a TAB after 'symtab.'
wasn't doing anything. Somehow the behavior has changed in the last
year.

 > 
 > If you (or someone else) has suggestions how to improve this, I'd be
 > glad to hear them.  But this would be an additional improvement.
 > 

I spent some time looking at this, and I don't see a way to do it
either, unless we invent a heuristic like, try on filenames, then on
symbols. But there should be some query mechanism for the user to
reject, say, the filenames. Too convoluted.

 > > Using the ':' I get a list of completions which includes:
 > > (top-gdb) b symtab.c:
 > > __builtin_va_list                  int
 > > _initialize_symtab                 keep_going
 > > add_filename_to_list               long double
 > > block_found                        long int
 > > block_function                     long long int
 > > bound                              long long unsigned int
 > > builtin_type_error                 long unsigned int
 > > char                               lookup_block_symbol
 > > completion_list_add_name           lookup_partial_symbol
 > > complex double                     lookup_partial_symtab
 > > complex float                      lookup_symbol
 > > complex int                        lookup_symbol_aux
 > > complex long double                lookup_symtab
 > > contained_in                       lookup_symtab_1
 > > cplusplus_hint                     lookup_transparent_type
 > > [...many more...]
 > > 
 > > But why am I getting the types names, like 'char', 'long double', etc?
 > > They must be coming from the minsymbols, maybe.
 > 
 > I saw this, but my testing indicates that these symbols are indeed
 > inside the symtab for the file (symtab.c, in this case), not inside a
 > minsym.  In fact, the function make_file_symbol_completion_list which
 > I added doesn't even look at minsyms.  AFAIU, the symtab for a file
 > holds all the data types that were visible to the compiler when the
 > file was compiled.  If you see something different, please tell,
 > perhaps there's a bug in my code.

No, my bad. Wasn't thinking straight.

 > 
 > > Do we want those?  Probably not.
 > 
 > Probably, but not positively.  Someone could type something like this:
 > 
 >    (gdb) break symtab.c:*(long *)&foobar
 > 

Yes, true. 

 > > (top-gdb) b symtab
 > > symtab              symtab.h            symtab_fns          symtab_to_filename
 > > symtab.c            symtab_and_line     symtab_symbol_info  symtabs_and_lines
 > > 
 > > (top-gdb) b symt<TAB>
 > > symtab              symtab_and_line     symtab_to_filename  
 > > symtab.c            symtab_fns          symtabs_and_lines   
 > > symtab.h            symtab_symbol_info  symtoken            
 > > 
 > > (top-gdb) b symtab.c:make_<TAB>
 > > make_cleanup_free_search_symbols   make_symbol_completion_list
 > > make_file_symbol_completion_list   make_symbol_overload_list
 > > make_source_files_completion_list  
 > > 
 > > These all seem to work now and didn't before, which is really neat.
 > > 
 > > I think some more refinements are needed.
 > 
 > I'd be glad to make this more smart, but it's not simple.  By far the
 > nastiest problem is that Readline decides what is the word which is
 > being completed on _before_ our completion function is called.  This
 > defeats many nifty tricks we could do using the context of the
 > completion.
 > 

Yes. If we could decide what set of break characters to install before the
tab is hit, it would be already a step in the right direction.
Or make readline's find_completion_word() a hook for a gdb function.

 > But I'm open to ideas which would make completion even more useful.
 > 

The basic problem is (as you said in another mail) that the definition
of location is really broad. MI tried some more strict syntax, but
that is an internal interface, not a user interface. I don't think a
more limited syntax in the UI would be long lived. Unless you make the
commands more specific, like break-on-line and break-on-function,
etc. But that also has its disadvantages.

 > > Syntactically, there is a line I don't like :
 > > 
 > >  > +  (*list)[*list_used += 1] = NULL;
 > > 
 > > Could you just use '++'?
 > 
 > Yes; I will change that.
 > 
 > > Sorry, I don't have much more time right now to look at it in more
 > > detail. I think I am starting to understand this whole patch. I may
 > > have some more comments tomorrow.
 > 
 > Thanks.
 > 

Anyway, now that I've looked stuff over more in detail, I think this
can go in. (With the fix for the ':').  But Fernando is the
completer.c maintainer, so we must give him a chance to go over the
patch as well.

Elena

Re: [RFA] Improve completion of locations

[Eli Zaretskii]

> From: Elena Zannoni <ezannoni@cygnus.com>
> Date: Sat, 12 May 2001 00:17:54 -0400
> 
> I was wondering how readline would get this right in case of
> completion on filenames. I had a closer look, and also in that case it
> gets it wrong. '.' is a word-break character, but not a filename-break
> character. The first time rl_complete() is invoked by pressing TAB,
> readline has the set of work-break characters installed. So it ends up
> calling line_completion_function() with an empty string, just like
> above.  But line completion function switches the set of break chars
> from under readline's nose, installing the file-break ones, and
> basically redoing what readline got wrong before. So later on, gdb
> will try to complete on 'symtab.' and get it right. Fascinating.

Indeed.  What this means is the the M-TAB completion, which doesn't
get the second chance, sometimes gets its act wrong.  This problem was
there since day one.

> BTW, I was playing with an older gdb, and a TAB after 'symtab.'
> wasn't doing anything. Somehow the behavior has changed in the last
> year.

What version of GDB was that?  I tried all the way down to 4.18, and
it still offers all the possible symbols given "symtab.<TAB>".  It
just that older versions take about forever to grind through all the
symbols, so you might think GDB is just sitting there idling.  Dan's
patch about a month ago made that code much, much faster.

>  > I'd be glad to make this more smart, but it's not simple.  By far the
>  > nastiest problem is that Readline decides what is the word which is
>  > being completed on _before_ our completion function is called.  This
>  > defeats many nifty tricks we could do using the context of the
>  > completion.
> 
> Yes. If we could decide what set of break characters to install before the
> tab is hit, it would be already a step in the right direction.
> Or make readline's find_completion_word() a hook for a gdb function.

Something like that.  Readline must cooperate with GDB better, for our
completion to be less error prone.  Right now, Readline does not
support the kind of mode that GDB needs, where it completes on
different types of objects and therefore needs to change the
word-constituent characters.  Bash completes only on file names, so it
never needs these complications.

I think we need to ask Readline maintainers to add a couple of
features in the next releases.

> Anyway, now that I've looked stuff over more in detail, I think this
> can go in. (With the fix for the ':').  But Fernando is the
> completer.c maintainer, so we must give him a chance to go over the
> patch as well.

Yes, I'm waiting for Fernando's approval.

Re: [RFA] Improve completion of locations

[Elena Zannoni]

Eli Zaretskii writes:
 > > From: Elena Zannoni <ezannoni@cygnus.com>
 > > Date: Sat, 12 May 2001 00:17:54 -0400
 > > 
 > > I was wondering how readline would get this right in case of
 > > completion on filenames. I had a closer look, and also in that case it
 > > gets it wrong. '.' is a word-break character, but not a filename-break
 > > character. The first time rl_complete() is invoked by pressing TAB,
 > > readline has the set of work-break characters installed. So it ends up
 > > calling line_completion_function() with an empty string, just like
 > > above.  But line completion function switches the set of break chars
 > > from under readline's nose, installing the file-break ones, and
 > > basically redoing what readline got wrong before. So later on, gdb
 > > will try to complete on 'symtab.' and get it right. Fascinating.
 > 
 > Indeed.  What this means is the the M-TAB completion, which doesn't
 > get the second chance, sometimes gets its act wrong.  This problem was
 > there since day one.

Ah. Not nice.

 > 
 > > BTW, I was playing with an older gdb, and a TAB after 'symtab.'
 > > wasn't doing anything. Somehow the behavior has changed in the last
 > > year.
 > 
 > What version of GDB was that?  I tried all the way down to 4.18, and
 > it still offers all the possible symbols given "symtab.<TAB>".  It
 > just that older versions take about forever to grind through all the
 > symbols, so you might think GDB is just sitting there idling.  Dan's
 > patch about a month ago made that code much, much faster.
 > 

It was an internal version, so it may have been different.

 > >  > I'd be glad to make this more smart, but it's not simple.  By far the
 > >  > nastiest problem is that Readline decides what is the word which is
 > >  > being completed on _before_ our completion function is called.  This
 > >  > defeats many nifty tricks we could do using the context of the
 > >  > completion.
 > > 
 > > Yes. If we could decide what set of break characters to install before the
 > > tab is hit, it would be already a step in the right direction.
 > > Or make readline's find_completion_word() a hook for a gdb function.
 > 
 > Something like that.  Readline must cooperate with GDB better, for our
 > completion to be less error prone.  Right now, Readline does not
 > support the kind of mode that GDB needs, where it completes on
 > different types of objects and therefore needs to change the
 > word-constituent characters.  Bash completes only on file names, so it
 > never needs these complications.
 > 
 > I think we need to ask Readline maintainers to add a couple of
 > features in the next releases.
 > 

Definitely. 

 > > Anyway, now that I've looked stuff over more in detail, I think this
 > > can go in. (With the fix for the ':').  But Fernando is the
 > > completer.c maintainer, so we must give him a chance to go over the
 > > patch as well.
 > 
 > Yes, I'm waiting for Fernando's approval.
 > 

Elena

Re: [RFA] Improve completion of locations

[Eli Zaretskii]

Fernando, I'm still waiting for your approval of the changes to
completer.c.  See:

  http://sources.redhat.com/ml/gdb-patches/2001-05/msg00058.html

(If I missed your comments, I apologize.)

Re: [RFA] Improve completion of locations

[Eli Zaretskii]

> Date: Sat, 05 May 2001 21:00:08 +0300
> From: "Eli Zaretskii" <eliz@is.elta.co.il>
> 
> I'm seeking approval for the patches below, which improve GDB's
> completion of locations in commands like "break LOCATION".  The two
> main improvements are:
> 
>   - GDB now considers file names as well as symbol names when you
>     type, e.g., "break foo".  If there's a symbol foo_bar and a file
>     foo-bar.c, both will be shown in the list of possible completions.
>     Only files recorded in the debug info are used for completing file
>     names for these commands.
> 
>   - When the user types "break foo.c:bar TAB", only symbols defined in
>     the source file foo.c whose names begin with "bar" will be
>     considered for completion.  This reduces the number of possible
>     completions by a large factor in many typical situations (I'd even
>     dare to say that it makes symbol completion a useful feature ;-).
> 
> This is the last installment of my recent completion-related frenzy.
> I'm now quite happy with GDB's completion ;-)
> 
> AFAICS, the responsible maintainers who need to approve these patches
> are Fernando, Elena, Michael Snyder, and someone for printcmd.c and
> infcmd.c.
> 
> 2001-05-05  Eli Zaretskii  <eliz@is.elta.co.il>
> 
> 	* completer.c (gdb_completer_loc_break_characters): New variable.
> 	(line_completion_function): If we are completing on locations,
> 	back up the start of word pointer past all characters which can
> 	appear in a location spec.
> 	(location_completer): New function.
> 
> 	* completer.h: Add prototype for location_completer.

It's been a month since I asked for approval of these patches, but the
changes for completer.[ch] are still not approved nor rejected.

Let me know how can I help you review these changes.

TIA

Re: [RFA] Improve completion of locations

[Fernando Nasser]

Eli Zaretskii wrote:
> 
> > Date: Sat, 05 May 2001 21:00:08 +0300
> > From: "Eli Zaretskii" <eliz@is.elta.co.il>
> >
> > I'm seeking approval for the patches below, which improve GDB's
> > completion of locations in commands like "break LOCATION".  The two
> > main improvements are:
> >
> >   - GDB now considers file names as well as symbol names when you
> >     type, e.g., "break foo".  If there's a symbol foo_bar and a file
> >     foo-bar.c, both will be shown in the list of possible completions.
> >     Only files recorded in the debug info are used for completing file
> >     names for these commands.
> >
> >   - When the user types "break foo.c:bar TAB", only symbols defined in
> >     the source file foo.c whose names begin with "bar" will be
> >     considered for completion.  This reduces the number of possible
> >     completions by a large factor in many typical situations (I'd even
> >     dare to say that it makes symbol completion a useful feature ;-).
> >
> > This is the last installment of my recent completion-related frenzy.
> > I'm now quite happy with GDB's completion ;-)
> >
> > AFAICS, the responsible maintainers who need to approve these patches
> > are Fernando, Elena, Michael Snyder, and someone for printcmd.c and
> > infcmd.c.
> >
> > 2001-05-05  Eli Zaretskii  <eliz@is.elta.co.il>
> >
> >       * completer.c (gdb_completer_loc_break_characters): New variable.
> >       (line_completion_function): If we are completing on locations,
> >       back up the start of word pointer past all characters which can
> >       appear in a location spec.
> >       (location_completer): New function.
> >
> >       * completer.h: Add prototype for location_completer.
> 
> It's been a month since I asked for approval of these patches, but the
> changes for completer.[ch] are still not approved nor rejected.
> 
> Let me know how can I help you review these changes.
> 
> TIA

Sorry Eli.  I have a deadline at work and I could not review your patch
yet.  I will try to do it by Friday or, if not possible, during the
weekend.

Thanks for your patience.

Regards,
Fernando



-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: [RFA] Improve completion of locations

[Eli Zaretskii]

> Date: Tue, 05 Jun 2001 09:07:31 -0400
> From: Fernando Nasser <fnasser@redhat.com>
> > 
> > It's been a month since I asked for approval of these patches, but the
> > changes for completer.[ch] are still not approved nor rejected.
> > 
> > Let me know how can I help you review these changes.
> > 
> > TIA
> 
> Sorry Eli.  I have a deadline at work and I could not review your patch
> yet.  I will try to do it by Friday or, if not possible, during the
> weekend.
> 
> Thanks for your patience.

Sure, will wait.  Thanks for letting me know.

Re: [RFA] Improve completion of locations

[Fernando Nasser]

Thanks Elena and Daniel for your reviews of Eli's patch.  I really
appreciate it.

Eli, Your original patch with the fix you've added as per Daniel
request, Elena's "++" suggestion and the adjusts to after JT's changes
is approved.

As I said, the evil is in the original definition of "linespecs" which
leads to so many ambiguous cases. So, we will have to live with
incremental fixes (and eventually unwanted side-effects).

And thank you very much for all the completer improvements you've sent.

Regards,
Fernando

P.S.: My apologies for the delay in responding.  I have a considerably
long queue of patches to review right now.

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: [RFA] Improve completion of locations

[Daniel Berlin]

Fernando Nasser <fnasser@redhat.com> writes:

> Thanks Elena and Daniel for your reviews of Eli's patch.  I really
> appreciate it.
> 
> Eli, Your original patch with the fix you've added as per Daniel
> request, Elena's "++" suggestion and the adjusts to after JT's changes
> is approved.
> 
> As I said, the evil is in the original definition of "linespecs" which
> leads to so many ambiguous cases. So, we will have to live with
> incremental fixes (and eventually unwanted side-effects).

Do you know where else besides list and break linespecs are used?

I have a parser/lexer ready to replace the decode_line_1 usage for
*breakpoints* (not list), but I want to make sure i catch all the
places i could use it.

> 
> And thank you very much for all the completer improvements you've sent.
> 
> Regards,
> Fernando
> 
> P.S.: My apologies for the delay in responding.  I have a considerably
> long queue of patches to review right now.
> 
> -- 
> Fernando Nasser
> Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
> 2323 Yonge Street, Suite #300
> Toronto, Ontario   M4P 2C9

-- 
"I watched the Indy 500, and I was thinking that if they left
earlier they wouldn't have to go so fast.
"-Steven Wright

Re: [RFA] Improve completion of locations

[Fernando Nasser]

Daniel Berlin wrote:
> 
> Do you know where else besides list and break linespecs are used?
> 
> I have a parser/lexer ready to replace the decode_line_1 usage for
> *breakpoints* (not list), but I want to make sure i catch all the
> places i could use it.
> 

As far as I remember, everyone else uses decode_line_1 for that.  The
completions code do not because of the readline() interactions.


-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: [RFA] Improve completion of locations

[Eli Zaretskii]

On Sun, 10 Jun 2001, Fernando Nasser wrote:

> Eli, Your original patch with the fix you've added as per Daniel
> request, Elena's "++" suggestion and the adjusts to after JT's changes
> is approved.

Thanks, I committed it.  The exact patch I committed, with all the 
corrections, is attached below.


2001-05-05  Eli Zaretskii  <eliz@is.elta.co.il>

	* completer.c (gdb_completer_loc_break_characters): New variable.
	(line_completion_function): If we are completing on locations,
	back up the start of word pointer past all characters which can
	appear in a location spec.
	(location_completer): New function.

	* completer.h: Add prototype for location_completer.

	* symtab.c (make_source_files_completion_list)
	(add_filename_to_list, not_interesting_fname): New functions.
	(filename_seen): New function, body extracted from
	output_source_filename.
	(output_source_filename): Call filename_seen to check if the file
	was already printed.
	(make_symbol_completion_list): If TEXT includes a
	double-quoted string, return an empty list, not NULL.
	(make_file_symbol_completion_list): New function, similar to
	make_symbol_completion_list but with an additional argument
	SRCFILE.

	* symtab.h (make_file_symbol_completion_list)
	(make_source_files_completion_list): Add prototypes.

	* breakpoint.c (_initialize_breakpoint): Make location_completer
	be the completion function for all commands which set breakpoints
	and watchpoints.
	(top-level): #include "completer.h".

	* tracepoint.c (_initialize_tracepoint): Make location_completer
	be the completion function for the "trace" command.
	(top-level): #include "completer.h".

	* printcmd.c (_initialize_printcmd): Make location_completer be
	the completion function for the "print", "inspect", "call", and
	"disassemble" commands.
	(top-level): #include "completer.h".

	* infcmd.c (_initialize_infcmd): Make location_completer be the
	completion function for the "go", "jump", and "until" commands.
	(top-level): #include "completer.h".


--- gdb/completer.h.~1~	Tue Mar  6 10:41:46 2001
+++ gdb/completer.h	Mon Jun 11 18:11:04 2001
@@ -23,6 +23,8 @@
 
 extern char **filename_completer (char *, char *);
 
+extern char **location_completer (char *, char *);
+
 extern char *get_gdb_completer_word_break_characters (void); 
 
 extern char *get_gdb_completer_quote_characters (void);


--- gdb/completer.c.~2~	Wed Jun  6 12:24:56 2001
+++ gdb/completer.c	Mon Jun 11 18:11:26 2001
@@ -22,12 +22,14 @@
 #include "symtab.h"
 #include "gdbtypes.h"
 #include "expression.h"
+#include "filenames.h"		/* for DOSish file names */
 
 /* FIXME: This is needed because of lookup_cmd_1().
    We should be calling a hook instead so we eliminate the CLI dependency. */
 #include "gdbcmd.h"
 
-/* Needed for rl_completer_word_break_characters() */
+/* Needed for rl_completer_word_break_characters() and for
+   filename_completion_function.  */
 #include <readline/readline.h>
 
 /* readline defines this.  */
@@ -72,6 +74,10 @@
 static char *gdb_completer_file_name_break_characters = " \t\n*|\"';:?><";
 #endif
 
+/* These are used when completing on locations, which can mix file
+   names and symbol names separated by a colon.  */
+static char *gdb_completer_loc_break_characters = " \t\n*|\"';:?><,";
+
 /* Characters that can be used to quote completion strings.  Note that we
    can't include '"' because the gdb C parser treats such quoted sequences
    as strings. */
@@ -95,8 +101,6 @@
 char **
 filename_completer (char *text, char *word)
 {
-  /* From readline.  */
-extern char *filename_completion_function (char *, int);
   int subsequent_name;
   char **return_val;
   int return_val_used;
@@ -170,6 +174,153 @@
   return return_val;
 }
 
+/* Complete on locations, which might be of two possible forms:
+
+       file:line
+   or
+       symbol+offset
+
+   This is intended to be used in commands that set breakpoints etc.  */
+char **
+location_completer (char *text, char *word)
+{
+  int n_syms = 0, n_files = 0;
+  char ** fn_list = NULL;
+  char ** list = NULL;
+  char *p;
+  int quote_found = 0;
+  int quoted = *text == '\'' || *text == '"';
+  int quote_char = '\0';
+  char *colon = NULL;
+  char *file_to_match = NULL;
+  char *symbol_start = text;
+  char *orig_text = text;
+  size_t text_len;
+
+  /* Do we have an unquoted colon, as in "break foo.c::bar"?  */
+  for (p = text; *p != '\0'; ++p)
+    {
+      if (*p == '\\' && p[1] == '\'')
+	p++;
+      else if (*p == '\'' || *p == '"')
+	{
+	  quote_found = *p;
+	  quote_char = *p++;
+	  while (*p != '\0' && *p != quote_found)
+	    {
+	      if (*p == '\\' && p[1] == quote_found)
+		p++;
+	      p++;
+	    }
+
+	  if (*p == quote_found)
+	    quote_found = 0;
+	  else
+	    break;		/* hit the end of text */
+	}
+#if HAVE_DOS_BASED_FILE_SYSTEM
+      /* If we have a DOS-style absolute file name at the beginning of
+	 TEXT, and the colon after the drive letter is the only colon
+	 we found, pretend the colon is not there.  */
+      else if (p < text + 3 && *p == ':' && p == text + 1 + quoted)
+	;
+#endif
+      else if (*p == ':' && !colon)
+	{
+	  colon = p;
+	  symbol_start = p + 1;
+	}
+      else if (strchr (gdb_completer_word_break_characters, *p))
+	symbol_start = p + 1;
+    }
+
+  if (quoted)
+    text++;
+  text_len = strlen (text);
+
+  /* Where is the file name?  */
+  if (colon)
+    {
+      char *s;
+
+      file_to_match = (char *) xmalloc (colon - text + 1);
+      strncpy (file_to_match, text, colon - text + 1);
+      /* Remove trailing colons and quotes from the file name.  */
+      for (s = file_to_match + (colon - text);
+	   s > file_to_match;
+	   s--)
+	if (*s == ':' || *s == quote_char)
+	  *s = '\0';
+    }
+  /* If the text includes a colon, they want completion only on a
+     symbol name after the colon.  Otherwise, we need to complete on
+     symbols as well as on files.  */
+  if (colon)
+    {
+      list = make_file_symbol_completion_list (symbol_start, word,
+					       file_to_match);
+      xfree (file_to_match);
+    }
+  else
+    {
+      list = make_symbol_completion_list (symbol_start, word);
+      /* If text includes characters which cannot appear in a file
+	 name, they cannot be asking for completion on files.  */
+      if (strcspn (text, gdb_completer_file_name_break_characters) == text_len)
+	fn_list = make_source_files_completion_list (text, text);
+    }
+
+  /* How many completions do we have in both lists?  */
+  if (fn_list)
+    for ( ; fn_list[n_files]; n_files++)
+      ;
+  if (list)
+    for ( ; list[n_syms]; n_syms++)
+      ;
+
+  /* Make list[] large enough to hold both lists, then catenate
+     fn_list[] onto the end of list[].  */
+  if (n_syms && n_files)
+    {
+      list = xrealloc (list, (n_syms + n_files + 1) * sizeof (char *));
+      memcpy (list + n_syms, fn_list, (n_files + 1) * sizeof (char *));
+      xfree (fn_list);
+    }
+  else if (n_files)
+    {
+      /* If we only have file names as possible completion, we should
+	 bring them in sync with what rl_complete expects.  The
+	 problem is that if the user types "break /foo/b TAB", and the
+	 possible completions are "/foo/bar" and "/foo/baz"
+	 rl_complete expects us to return "bar" and "baz", without the
+	 leading directories, as possible completions, because `word'
+	 starts at the "b".  But we ignore the value of `word' when we
+	 call make_source_files_completion_list above (because that
+	 would not DTRT when the completion results in both symbols
+	 and file names), so make_source_files_completion_list returns
+	 the full "/foo/bar" and "/foo/baz" strings.  This produces
+	 wrong results when, e.g., there's only one possible
+	 completion, because rl_complete will prepend "/foo/" to each
+	 candidate completion.  The loop below removes that leading
+	 part.  */
+      for (n_files = 0; fn_list[n_files]; n_files++)
+	{
+	  memmove (fn_list[n_files], fn_list[n_files] + (word - text),
+		   strlen (fn_list[n_files]) + 1 - (word - text));
+	}
+      /* Return just the file-name list as the result.  */
+      list = fn_list;
+    }
+  else if (!n_syms)
+    {
+      /* No completions at all.  As the final resort, try completing
+	 on the entire text as a symbol.  */
+      list = make_symbol_completion_list (orig_text, word);
+    }
+
+  return list;
+}
+
 /* Here are some useful test cases for completion.  FIXME: These should
    be put in the test suite.  They should be tested with both M-? and TAB.
 
@@ -362,7 +513,7 @@
 			     to complete the entire text after the
 			     command, just the last word.  To this
 			     end, we need to find the beginning of the
-			     file name starting at `word' and going
+			     file name by starting at `word' and going
 			     backwards.  */
 			  for (p = word;
 			       p > tmp_command
@@ -372,6 +523,16 @@
 			  rl_completer_word_break_characters =
 			    gdb_completer_file_name_break_characters;
 			}
+		      else if (c->completer == location_completer)
+			{
+			  /* Commands which complete on locations want to
+			     see the entire argument.  */
+			  for (p = word;
+			       p > tmp_command
+				 && p[-1] != ' ' && p[-1] != '\t';
+			       p--)
+			    ;
+			}
 		      list = (*c->completer) (p, word);
 		    }
 		}
@@ -430,6 +591,14 @@
 		      rl_completer_word_break_characters =
 			gdb_completer_file_name_break_characters;
 		    }
+		  else if (c->completer == location_completer)
+		    {
+		      for (p = word;
+			   p > tmp_command
+			     && p[-1] != ' ' && p[-1] != '\t';
+			   p--)
+			;
+		    }
 		  list = (*c->completer) (p, word);
 		}
 	    }


--- gdb/symtab.c.~1~	Mon May 14 21:10:34 2001
+++ gdb/symtab.c	Mon Jun 11 18:41:02 2001
@@ -2153,50 +2153,72 @@
 }
 \f
 
-/* Slave routine for sources_info.  Force line breaks at ,'s.
-   NAME is the name to print and *FIRST is nonzero if this is the first
-   name printed.  Set *FIRST to zero.  */
-static void
-output_source_filename (char *name, int *first)
+/* If FILE is not already in the table of files, return zero;
+   otherwise return non-zero.  Optionally add FILE to the table if ADD
+   is non-zero.  If *FIRST is non-zero, forget the old table
+   contents.  */
+static int
+filename_seen (const char *file, int add, int *first)
 {
-  /* Table of files printed so far.  Since a single source file can
-     result in several partial symbol tables, we need to avoid printing
-     it more than once.  Note: if some of the psymtabs are read in and
-     some are not, it gets printed both under "Source files for which
-     symbols have been read" and "Source files for which symbols will
-     be read in on demand".  I consider this a reasonable way to deal
-     with the situation.  I'm not sure whether this can also happen for
-     symtabs; it doesn't hurt to check.  */
-  static char **tab = NULL;
+  /* Table of files seen so far.  */
+  static const char **tab = NULL;
   /* Allocated size of tab in elements.
      Start with one 256-byte block (when using GNU malloc.c).
      24 is the malloc overhead when range checking is in effect.  */
   static int tab_alloc_size = (256 - 24) / sizeof (char *);
   /* Current size of tab in elements.  */
   static int tab_cur_size;
-
-  char **p;
+  const char **p;
 
   if (*first)
     {
       if (tab == NULL)
-	tab = (char **) xmalloc (tab_alloc_size * sizeof (*tab));
+	tab = (const char **) xmalloc (tab_alloc_size * sizeof (*tab));
       tab_cur_size = 0;
     }
 
-  /* Is NAME in tab?  */
+  /* Is FILE in tab?  */
   for (p = tab; p < tab + tab_cur_size; p++)
-    if (STREQ (*p, name))
-      /* Yes; don't print it again.  */
-      return;
-  /* No; add it to tab.  */
-  if (tab_cur_size == tab_alloc_size)
+    if (strcmp (*p, file) == 0)
+      return 1;
+
+  /* No; maybe add it to tab.  */
+  if (add)
     {
-      tab_alloc_size *= 2;
-      tab = (char **) xrealloc ((char *) tab, tab_alloc_size * sizeof (*tab));
+      if (tab_cur_size == tab_alloc_size)
+	{
+	  tab_alloc_size *= 2;
+	  tab = (const char **) xrealloc ((char *) tab,
+					  tab_alloc_size * sizeof (*tab));
+	}
+      tab[tab_cur_size++] = file;
     }
-  tab[tab_cur_size++] = name;
 
+  return 0;
+}
+
+/* Slave routine for sources_info.  Force line breaks at ,'s.
+   NAME is the name to print and *FIRST is nonzero if this is the first
+   name printed.  Set *FIRST to zero.  */
+static void
+output_source_filename (char *name, int *first)
+{
+  /* Since a single source file can result in several partial symbol
+     tables, we need to avoid printing it more than once.  Note: if
+     some of the psymtabs are read in and some are not, it gets
+     printed both under "Source files for which symbols have been
+     read" and "Source files for which symbols will be read in on
+     demand".  I consider this a reasonable way to deal with the
+     situation.  I'm not sure whether this can also happen for
+     symtabs; it doesn't hurt to check.  */
+
+  /* Was NAME already seen?  */
+  if (filename_seen (name, 1, first))
+    {
+      /* Yes; don't print it again.  */
+      return;
+    }
+  /* No; print it and reset *FIRST.  */
   if (*first)
     {
       *first = 0;
@@ -2871,9 +2893,9 @@
   }
 }
 
-/* Return a NULL terminated array of all symbols (regardless of class) which
-   begin by matching TEXT.  If the answer is no symbols, then the return value
-   is an array which contains only a NULL pointer.
+/* Return a NULL terminated array of all symbols (regardless of class)
+   which begin by matching TEXT.  If the answer is no symbols, then
+   the return value is an array which contains only a NULL pointer.
 
    Problem: All of the symbols have to be copied because readline frees them.
    I'm not going to worry about this; hopefully there won't be that many.  */
@@ -2927,7 +2949,11 @@
     else if (quote_found == '"')
       /* A double-quoted string is never a symbol, nor does it make sense
          to complete it any other way.  */
-      return NULL;
+      {
+	return_val = (char **) xmalloc (sizeof (char *));
+	return_val[0] = NULL;
+	return return_val;
+      }
     else
       {
 	/* It is not a quoted string.  Break it based on the characters
@@ -3059,6 +3085,277 @@
   return (return_val);
 }
 
+/* Like make_symbol_completion_list, but returns a list of symbols
+   defined in a source file FILE.  */
+
+char **
+make_file_symbol_completion_list (char *text, char *word, char *srcfile)
+{
+  register struct symbol *sym;
+  register struct symtab *s;
+  register struct block *b;
+  register int i;
+  /* The symbol we are completing on.  Points in same buffer as text.  */
+  char *sym_text;
+  /* Length of sym_text.  */
+  int sym_text_len;
+
+  /* Now look for the symbol we are supposed to complete on.
+     FIXME: This should be language-specific.  */
+  {
+    char *p;
+    char quote_found;
+    char *quote_pos = NULL;
+
+    /* First see if this is a quoted string.  */
+    quote_found = '\0';
+    for (p = text; *p != '\0'; ++p)
+      {
+	if (quote_found != '\0')
+	  {
+	    if (*p == quote_found)
+	      /* Found close quote.  */
+	      quote_found = '\0';
+	    else if (*p == '\\' && p[1] == quote_found)
+	      /* A backslash followed by the quote character
+	         doesn't end the string.  */
+	      ++p;
+	  }
+	else if (*p == '\'' || *p == '"')
+	  {
+	    quote_found = *p;
+	    quote_pos = p;
+	  }
+      }
+    if (quote_found == '\'')
+      /* A string within single quotes can be a symbol, so complete on it.  */
+      sym_text = quote_pos + 1;
+    else if (quote_found == '"')
+      /* A double-quoted string is never a symbol, nor does it make sense
+         to complete it any other way.  */
+      {
+	return_val = (char **) xmalloc (sizeof (char *));
+	return_val[0] = NULL;
+	return return_val;
+      }
+    else
+      {
+	/* It is not a quoted string.  Break it based on the characters
+	   which are in symbols.  */
+	while (p > text)
+	  {
+	    if (isalnum (p[-1]) || p[-1] == '_' || p[-1] == '\0')
+	      --p;
+	    else
+	      break;
+	  }
+	sym_text = p;
+      }
+  }
+
+  sym_text_len = strlen (sym_text);
+
+  return_val_size = 10;
+  return_val_index = 0;
+  return_val = (char **) xmalloc ((return_val_size + 1) * sizeof (char *));
+  return_val[0] = NULL;
+
+  /* Find the symtab for SRCFILE (this loads it if it was not yet read
+     in).  */
+  s = lookup_symtab (srcfile);
+  if (s == NULL)
+    {
+      /* Maybe they typed the file with leading directories, while the
+	 symbol tables record only its basename.  */
+      char *tail = basename (srcfile);
+
+      if (tail > srcfile)
+	s = lookup_symtab (tail);
+    }
+
+  /* If we have no symtab for that file, return an empty list.  */
+  if (s == NULL)
+    return (return_val);
+
+  /* Go through this symtab and check the externs and statics for
+     symbols which match.  */
+
+  b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
+  for (i = 0; i < BLOCK_NSYMS (b); i++)
+    {
+      sym = BLOCK_SYM (b, i);
+      COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
+    }
+
+  b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), STATIC_BLOCK);
+  for (i = 0; i < BLOCK_NSYMS (b); i++)
+    {
+      sym = BLOCK_SYM (b, i);
+      COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
+    }
+
+  return (return_val);
+}
+
+/* A helper function for make_source_files_completion_list.  It adds
+   another file name to a list of possible completions, growing the
+   list as necessary.  */
+
+static void
+add_filename_to_list (const char *fname, char *text, char *word,
+		      char ***list, int *list_used, int *list_alloced)
+{
+  char *new;
+  size_t fnlen = strlen (fname);
+
+  if (*list_used + 1 >= *list_alloced)
+    {
+      *list_alloced *= 2;
+      *list = (char **) xrealloc ((char *) *list,
+				  *list_alloced * sizeof (char *));
+    }
+
+  if (word == text)
+    {
+      /* Return exactly fname.  */
+      new = xmalloc (fnlen + 5);
+      strcpy (new, fname);
+    }
+  else if (word > text)
+    {
+      /* Return some portion of fname.  */
+      new = xmalloc (fnlen + 5);
+      strcpy (new, fname + (word - text));
+    }
+  else
+    {
+      /* Return some of TEXT plus fname.  */
+      new = xmalloc (fnlen + (text - word) + 5);
+      strncpy (new, word, text - word);
+      new[text - word] = '\0';
+      strcat (new, fname);
+    }
+  (*list)[*list_used] = new;
+  (*list)[++*list_used] = NULL;
+}
+
+static int
+not_interesting_fname (const char *fname)
+{
+  static const char *illegal_aliens[] = {
+    "_globals_",	/* inserted by coff_symtab_read */
+    NULL
+  };
+  int i;
+
+  for (i = 0; illegal_aliens[i]; i++)
+    {
+      if (strcmp (fname, illegal_aliens[i]) == 0)
+	return 1;
+    }
+  return 0;
+}
+
+/* Return a NULL terminated array of all source files whose names
+   begin with matching TEXT.  The file names are looked up in the
+   symbol tables of this program.  If the answer is no matchess, then
+   the return value is an array which contains only a NULL pointer.  */
+
+char **
+make_source_files_completion_list (char *text, char *word)
+{
+  register struct symtab *s;
+  register struct partial_symtab *ps;
+  register struct objfile *objfile;
+  int first = 1;
+  int list_alloced = 1;
+  int list_used = 0;
+  size_t text_len = strlen (text);
+  char **list = (char **) xmalloc (list_alloced * sizeof (char *));
+  char *base_name;
+
+  list[0] = NULL;
+
+  if (!have_full_symbols () && !have_partial_symbols ())
+    return list;
+
+  ALL_SYMTABS (objfile, s)
+    {
+      if (not_interesting_fname (s->filename))
+	continue;
+      if (!filename_seen (s->filename, 1, &first)
+#if HAVE_DOS_BASED_FILE_SYSTEM
+	  && strncasecmp (s->filename, text, text_len) == 0
+#else
+	  && strncmp (s->filename, text, text_len) == 0
+#endif
+	  )
+	{
+	  /* This file matches for a completion; add it to the current
+	     list of matches.  */
+	  add_filename_to_list (s->filename, text, word,
+				&list, &list_used, &list_alloced);
+	}
+      else
+	{
+	  /* NOTE: We allow the user to type a base name when the
+	     debug info records leading directories, but not the other
+	     way around.  This is what subroutines of breakpoint
+	     command do when they parse file names.  */
+	  base_name = basename (s->filename);
+	  if (base_name != s->filename
+	      && !filename_seen (base_name, 1, &first)
+#if HAVE_DOS_BASED_FILE_SYSTEM
+	      && strncasecmp (base_name, text, text_len) == 0
+#else
+	      && strncmp (base_name, text, text_len) == 0
+#endif
+	      )
+	    add_filename_to_list (base_name, text, word,
+				  &list, &list_used, &list_alloced);
+	}
+    }
+
+  ALL_PSYMTABS (objfile, ps)
+    {
+      if (not_interesting_fname (ps->filename))
+	continue;
+      if (!ps->readin)
+	{
+	  if (!filename_seen (ps->filename, 1, &first)
+#if HAVE_DOS_BASED_FILE_SYSTEM
+	      && strncasecmp (ps->filename, text, text_len) == 0
+#else
+	      && strncmp (ps->filename, text, text_len) == 0
+#endif
+	      )
+	    {
+	      /* This file matches for a completion; add it to the
+		 current list of matches.  */
+	      add_filename_to_list (ps->filename, text, word,
+				    &list, &list_used, &list_alloced);
+
+	    }
+	  else
+	    {
+	      base_name = basename (ps->filename);
+	      if (base_name != ps->filename
+		  && !filename_seen (base_name, 1, &first)
+#if HAVE_DOS_BASED_FILE_SYSTEM
+		  && strncasecmp (base_name, text, text_len) == 0
+#else
+		  && strncmp (base_name, text, text_len) == 0
+#endif
+		  )
+		add_filename_to_list (base_name, text, word,
+				      &list, &list_used, &list_alloced);
+	    }
+	}
+    }
+
+  return list;
+}
+
 /* Determine if PC is in the prologue of a function.  The prologue is the area
    between the first instruction of a function, and the first executable line.
    Returns 1 if PC *might* be in prologue, 0 if definately *not* in prologue.


--- gdb/symtab.h.~1~	Fri Apr 27 02:39:48 2001
+++ gdb/symtab.h	Mon Jun 11 18:11:04 2001
@@ -1359,8 +1359,12 @@
 
 extern char **make_symbol_completion_list (char *, char *);
 
+extern char **make_file_symbol_completion_list (char *, char *, char *);
+
 extern struct symbol **make_symbol_overload_list (struct symbol *);
 
+extern char **make_source_files_completion_list (char *, char *);
+
 /* symtab.c */
 
 extern struct partial_symtab *find_main_psymtab (void);


--- gdb/breakpoint.c.~1~	Wed May 23 03:13:24 2001
+++ gdb/breakpoint.c	Mon Jun 11 18:11:04 2001
@@ -40,6 +40,7 @@
 #include "symfile.h"
 #include "objfiles.h"
 #include "linespec.h"
+#include "completer.h"
 #ifdef UI_OUT
 #include "ui-out.h"
 #endif
@@ -7576,24 +7577,29 @@
 Usage is `condition N COND', where N is an integer and COND is an\n\
 expression to be evaluated whenever breakpoint N is reached.  ");
 
-  add_com ("tbreak", class_breakpoint, tbreak_command,
-	   "Set a temporary breakpoint.  Args like \"break\" command.\n\
+  c = add_com ("tbreak", class_breakpoint, tbreak_command,
+	       "Set a temporary breakpoint.  Args like \"break\" command.\n\
 Like \"break\" except the breakpoint is only temporary,\n\
 so it will be deleted when hit.  Equivalent to \"break\" followed\n\
 by using \"enable delete\" on the breakpoint number.");
-  add_com ("txbreak", class_breakpoint, tbreak_at_finish_command,
-	   "Set temporary breakpoint at procedure exit.  Either there should\n\
+  c->completer = location_completer;
+
+  c = add_com ("txbreak", class_breakpoint, tbreak_at_finish_command,
+	       "Set temporary breakpoint at procedure exit.  Either there should\n\
 be no argument or the argument must be a depth.\n");
+  c->completer = location_completer;
 
-  add_com ("hbreak", class_breakpoint, hbreak_command,
-	   "Set a hardware assisted  breakpoint. Args like \"break\" command.\n\
+  c = add_com ("hbreak", class_breakpoint, hbreak_command,
+	       "Set a hardware assisted  breakpoint. Args like \"break\" command.\n\
 Like \"break\" except the breakpoint requires hardware support,\n\
 some target hardware may not have this support.");
+  c->completer = location_completer;
 
-  add_com ("thbreak", class_breakpoint, thbreak_command,
-	   "Set a temporary hardware assisted breakpoint. Args like \"break\" command.\n\
+  c = add_com ("thbreak", class_breakpoint, thbreak_command,
+	       "Set a temporary hardware assisted breakpoint. Args like \"break\" command.\n\
 Like \"hbreak\" except the breakpoint is only temporary,\n\
 so it will be deleted when hit.");
+  c->completer = location_completer;
 
   add_prefix_cmd ("enable", class_breakpoint, enable_command,
 		  "Enable some breakpoints.\n\
@@ -7695,8 +7701,8 @@
 \n\
 See also the \"delete\" command which clears breakpoints by number.", NULL));
 
-  add_com ("break", class_breakpoint, break_command,
-	   concat ("Set breakpoint at specified line or function.\n\
+  c = add_com ("break", class_breakpoint, break_command,
+	       concat ("Set breakpoint at specified line or function.\n\
 Argument may be line number, function name, or \"*\" and an address.\n\
 If line number is specified, break at start of code for that line.\n\
 If function is specified, break at start of code for that function.\n\
@@ -7707,6 +7713,8 @@
 Multiple breakpoints at one place are permitted, and useful if conditional.\n\
 \n\
 Do \"help breakpoints\" for info on other commands dealing with breakpoints.", NULL));
+  c->completer = location_completer;
+
   add_com_alias ("b", "break", class_run, 1);
   add_com_alias ("br", "break", class_run, 1);
   add_com_alias ("bre", "break", class_run, 1);
@@ -7852,20 +7860,23 @@
 so it will be deleted when hit.  Equivalent to \"catch\" followed\n\
 by using \"enable delete\" on the catchpoint number.");
 
-  add_com ("watch", class_breakpoint, watch_command,
-	   "Set a watchpoint for an expression.\n\
+  c = add_com ("watch", class_breakpoint, watch_command,
+	       "Set a watchpoint for an expression.\n\
 A watchpoint stops execution of your program whenever the value of\n\
 an expression changes.");
+  c->completer = location_completer;
 
-  add_com ("rwatch", class_breakpoint, rwatch_command,
-	   "Set a read watchpoint for an expression.\n\
+  c = add_com ("rwatch", class_breakpoint, rwatch_command,
+	       "Set a read watchpoint for an expression.\n\
 A watchpoint stops execution of your program whenever the value of\n\
 an expression is read.");
+  c->completer = location_completer;
 
-  add_com ("awatch", class_breakpoint, awatch_command,
-	   "Set a watchpoint for an expression.\n\
+  c = add_com ("awatch", class_breakpoint, awatch_command,
+	       "Set a watchpoint for an expression.\n\
 A watchpoint stops execution of your program whenever the value of\n\
 an expression is either read or written.");
+  c->completer = location_completer;
 
   add_info ("watchpoints", breakpoints_info,
 	    "Synonym for ``info breakpoints''.");

--- gdb/tracepoint.c.~1~	Tue Apr 17 22:37:10 2001
+++ gdb/tracepoint.c	Mon Jun 11 18:11:20 2001
@@ -33,6 +33,7 @@
 #include "remote.h"
 #include "linespec.h"
 #include "regcache.h"
+#include "completer.h"
 #include "gdb-events.h"
 
 #include "ax.h"
@@ -2799,12 +2800,13 @@
 No argument means enable all tracepoints.",
 	   &enablelist);
 
-  add_com ("trace", class_trace, trace_command,
-	   "Set a tracepoint at a specified line or function or address.\n\
+  c = add_com ("trace", class_trace, trace_command,
+	       "Set a tracepoint at a specified line or function or address.\n\
 Argument may be a line number, function name, or '*' plus an address.\n\
 For a line number or function, trace at the start of its code.\n\
 If an address is specified, trace at that exact address.\n\n\
 Do \"help tracepoints\" for info on other tracepoint commands.");
+  c->completer = location_completer;
 
   add_com_alias ("tp", "trace", class_alias, 0);
   add_com_alias ("tr", "trace", class_alias, 1);


--- gdb/printcmd.c.~1~	Sat May 12 06:29:02 2001
+++ gdb/printcmd.c	Mon Jun 11 18:11:06 2001
@@ -37,6 +37,7 @@
 #include "annotate.h"
 #include "symfile.h"		/* for overlay functions */
 #include "objfiles.h"		/* ditto */
+#include "completer.h"		/* for completion functions */
 #ifdef UI_OUT
 #include "ui-out.h"
 #endif
@@ -2451,6 +2452,8 @@
 void
 _initialize_printcmd (void)
 {
+  struct cmd_list_element *c;
+
   current_display_number = -1;
 
   add_info ("address", address_info,
@@ -2473,11 +2476,12 @@
 Default count is 1.  Default address is following last thing printed\n\
 with this command or \"print\".", NULL));
 
-  add_com ("disassemble", class_vars, disassemble_command,
-	   "Disassemble a specified section of memory.\n\
+  c = add_com ("disassemble", class_vars, disassemble_command,
+	       "Disassemble a specified section of memory.\n\
 Default is the function surrounding the pc of the selected frame.\n\
 With a single argument, the function surrounding that address is dumped.\n\
 Two arguments are taken as a range of memory to dump.");
+  c->completer = location_completer;
   if (xdb_commands)
     add_com_alias ("va", "disassemble", class_xdb, 0);
 
@@ -2555,11 +2559,12 @@
 You can see these environment settings with the \"show\" command.", NULL));
 
   /* "call" is the same as "set", but handy for dbx users to call fns. */
-  add_com ("call", class_vars, call_command,
-	   "Call a function in the program.\n\
+  c = add_com ("call", class_vars, call_command,
+	       "Call a function in the program.\n\
 The argument is the function name and arguments, in the notation of the\n\
 current working language.  The result is printed and saved in the value\n\
 history, if it is not void.");
+  c->completer = location_completer;
 
   add_cmd ("variable", class_vars, set_command,
 	   "Evaluate expression EXP and assign result to variable VAR, using assignment\n\
@@ -2570,7 +2575,7 @@
 This may usually be abbreviated to simply \"set\".",
 	   &setlist);
 
-  add_com ("print", class_vars, print_command,
+  c = add_com ("print", class_vars, print_command,
 	   concat ("Print value of expression EXP.\n\
 Variables accessible are those of the lexical environment of the selected\n\
 stack frame, plus all those whose scope is global or an entire file.\n\
@@ -2592,11 +2597,13 @@
 		   "\n\
 EXP may be preceded with /FMT, where FMT is a format letter\n\
 but no count or size letter (see \"x\" command).", NULL));
+  c->completer = location_completer;
   add_com_alias ("p", "print", class_vars, 1);
 
-  add_com ("inspect", class_vars, inspect_command,
+  c = add_com ("inspect", class_vars, inspect_command,
 	   "Same as \"print\" command, except that if you are running in the epoch\n\
 environment, the value is printed in its own window.");
+  c->completer = location_completer;
 
   add_show_from_set (
 		 add_set_cmd ("max-symbolic-offset", no_class, var_uinteger,


--- gdb/infcmd.c.~1~	Wed May 16 22:26:28 2001
+++ gdb/infcmd.c	Mon Jun 11 18:11:20 2001
@@ -36,6 +36,7 @@
 #include "language.h"
 #include "symfile.h"
 #include "objfiles.h"
+#include "completer.h"
 #ifdef UI_OUT
 #include "ui-out.h"
 #endif
@@ -1794,8 +1795,8 @@
 {
   struct cmd_list_element *c;
 
-  c= add_com ("tty", class_run, tty_command,
-	      "Set terminal for future runs of program being debugged.");
+  c = add_com ("tty", class_run, tty_command,
+	       "Set terminal for future runs of program being debugged.");
   c->completer = filename_completer;
 
   c = add_set_cmd ("args", class_run, var_string_noescape,
@@ -1899,25 +1900,30 @@
 Argument N means do this N times (or till program stops for another reason).");
   add_com_alias ("s", "step", class_run, 1);
 
-  add_com ("until", class_run, until_command,
-	   "Execute until the program reaches a source line greater than the current\n\
+  c = add_com ("until", class_run, until_command,
+	       "Execute until the program reaches a source line greater than the current\n\
 or a specified line or address or function (same args as break command).\n\
 Execution will also stop upon exit from the current stack frame.");
+  c->completer = location_completer;
   add_com_alias ("u", "until", class_run, 1);
 
-  add_com ("jump", class_run, jump_command,
-	   "Continue program being debugged at specified line or address.\n\
+  c = add_com ("jump", class_run, jump_command,
+	       "Continue program being debugged at specified line or address.\n\
 Give as argument either LINENUM or *ADDR, where ADDR is an expression\n\
 for an address to start at.");
+  c->completer = location_completer;
 
   if (xdb_commands)
-    add_com ("go", class_run, go_command,
-	     "Usage: go <location>\n\
+    {
+      c = add_com ("go", class_run, go_command,
+		   "Usage: go <location>\n\
 Continue program being debugged, stopping at specified line or \n\
 address.\n\
 Give as argument either LINENUM or *ADDR, where ADDR is an \n\
 expression for an address to start at.\n\
 This command is a combination of tbreak and jump.");
+      c->completer = location_completer;
+    }
 
   if (xdb_commands)
     add_com_alias ("g", "go", class_run, 1);