[RFA] Patch to limit field name completion candidates

[RFA] Patch to limit field name completion candidates

[Tom Tromey]

For a long time now I've accidentally typed things like:

    p somestruct.<TAB>

... and had to wait while gdb dumped all available symbols to gud.

It seemed to me that gdb should know that I'm trying to complete a
field expression and limit the completions to fields actually
occurring in the structure type on the left hand side.

This patch achieves this.  Daniel pointed me at an earlier attempt:

    http://www.sourceware.org/ml/gdb-patches/2002-04/msg00774.html

but this new patch works in a different way.  I did take a tiny bit of
code from the old patch, but I believe the author has assignments in
place, and anyhow the overlap is trivial.

How this works:

I added a new expression_completer and set this as the completion
function for various things, like "p".  This tries to complete a field
expression, but falls back to the old location_completer (which is
what all these commands currently use).

The expression completer sets a global flag, in_parse_field, and then
calls the expression parser.  The parsers are expected to look at this
flag and call mark_struct_expression at the right time.  Then, if this
was called, the generic code extracts the left-hand-side
subexpression, gets its type, and then does field completion as you'd
expect.

I only updated the C parser.  This code works by modifying the lexer
to return a special COMPLETE token in the important cases.  Note that
it completes both "p foo.TAB" and "p foo.somethingTAB" correctly --
the former by making an expression to a field with an empty name.

You may want to check the cleanup changes in parse_exp_in_context.
I'm not sure those are right.

I added one new test case.  There are no regressions with this change.

Tom

ChangeLog:
2008-05-12  Tom Tromey  <tromey@redhat.com>

	* value.h (evaluate_subexpression_type, extract_field_op):
	Declare.
	* printcmd.c (_initialize_printcmd): Use expression_completer for
	'p', 'inspect', 'call'.
	* parser-defs.h (parse_field_expression): Declare.
	* parse.c: Include exceptions.h.
	(in_parse_field, expout_last_struct): New globals.
	(mark_struct_expression): New function.
	(prefixify_expression): Return int.
	(prefixify_subexp): Return int.  Use expout_last_struct.
	(parse_exp_1): Update.
	(parse_exp_in_context): Add 'out_subexp' argument.  Handle
	in_parse_field.
	(parse_field_expression): New function.
	* expression.h (parse_field_expression): Declare.
	(in_parse_field): Likewise.
	* eval.c (evaluate_subexpression_type): New function.
	(extract_field_op): Likewise.
	* completer.h (expression_completer): Declare.
	* completer.c (expression_completer): New function.
	* c-exp.y (yyparse): Redefine.
	(COMPLETE): New token.
	(exp): New productions.
	(saw_name_at_eof, last_was_structop): New globals.
	(yylex): Return COMPLETE when needed.  Recognize in_parse_field.
	(c_parse): New function.
	* breakpoint.c (_initialize_breakpoint): Use expression_completer
	for watch, awatch, and rwatch.
	* Makefile.in (parse.o): Depend on exceptions_h.

testsuite/ChangeLog:
2008-05-12  Tom Tromey  <tromey@redhat.com>

	* gdb.base/break1.c (struct some_struct): New struct.
	(values): New global.
	* gdb.base/completion.exp: Add field name completion test.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.1018
diff -u -r1.1018 Makefile.in
--- Makefile.in	9 May 2008 17:02:01 -0000	1.1018
+++ Makefile.in	12 May 2008 23:51:42 -0000
@@ -2591,7 +2591,8 @@
 parse.o: parse.c $(defs_h) $(gdb_string_h) $(symtab_h) $(gdbtypes_h) \
 	$(frame_h) $(expression_h) $(value_h) $(command_h) $(language_h) \
 	$(f_lang_h) $(parser_defs_h) $(gdbcmd_h) $(symfile_h) $(inferior_h) \
-	$(doublest_h) $(gdb_assert_h) $(block_h) $(source_h) $(objfiles_h)
+	$(doublest_h) $(gdb_assert_h) $(block_h) $(source_h) $(objfiles_h) \
+	$(exceptions_h)
 p-exp.o: p-exp.c $(defs_h) $(gdb_string_h) $(expression_h) $(value_h) \
 	$(parser_defs_h) $(language_h) $(p_lang_h) $(bfd_h) $(symfile_h) \
 	$(objfiles_h) $(block_h)
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.321
diff -u -r1.321 breakpoint.c
--- breakpoint.c	4 May 2008 19:38:59 -0000	1.321
+++ breakpoint.c	12 May 2008 23:51:43 -0000
@@ -8433,19 +8433,19 @@
 Set a watchpoint for an expression.\n\
 A watchpoint stops execution of your program whenever the value of\n\
 an expression changes."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
 
   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."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
 
   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."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
 
   add_info ("watchpoints", breakpoints_info,
 	    _("Synonym for ``info breakpoints''."));
Index: c-exp.y
===================================================================
RCS file: /cvs/src/src/gdb/c-exp.y,v
retrieving revision 1.42
diff -u -r1.42 c-exp.y
--- c-exp.y	9 Jan 2008 19:27:15 -0000	1.42
+++ c-exp.y	12 May 2008 23:51:43 -0000
@@ -63,7 +63,7 @@
    generators need to be fixed instead of adding those names to this list. */
 
 #define	yymaxdepth c_maxdepth
-#define	yyparse	c_parse
+#define	yyparse	c_parse_internal
 #define	yylex	c_lex
 #define	yyerror	c_error
 #define	yylval	c_lval
@@ -180,6 +180,7 @@
 
 %token <sval> STRING
 %token <ssym> NAME /* BLOCKNAME defined below to give it higher precedence. */
+%token <voidval> COMPLETE
 %token <tsym> TYPENAME
 %type <sval> name
 %type <ssym> name_not_typename
@@ -301,6 +302,23 @@
 			  write_exp_elt_opcode (STRUCTOP_PTR); }
 	;
 
+exp	:	exp ARROW name COMPLETE
+			{ mark_struct_expression ();
+			  write_exp_elt_opcode (STRUCTOP_PTR);
+			  write_exp_string ($3);
+			  write_exp_elt_opcode (STRUCTOP_PTR); }
+	;
+
+exp	:	exp ARROW COMPLETE
+			{ struct stoken s;
+			  mark_struct_expression ();
+			  write_exp_elt_opcode (STRUCTOP_PTR);
+			  s.ptr = "";
+			  s.length = 0;
+			  write_exp_string (s);
+			  write_exp_elt_opcode (STRUCTOP_PTR); }
+	;
+
 exp	:	exp ARROW qualified_name
 			{ /* exp->type::name becomes exp->*(&type::name) */
 			  /* Note: this doesn't work if name is a
@@ -319,6 +337,23 @@
 			  write_exp_elt_opcode (STRUCTOP_STRUCT); }
 	;
 
+exp	:	exp '.' name COMPLETE
+			{ mark_struct_expression ();
+			  write_exp_elt_opcode (STRUCTOP_STRUCT);
+			  write_exp_string ($3);
+			  write_exp_elt_opcode (STRUCTOP_STRUCT); }
+	;
+
+exp	:	exp '.' COMPLETE
+			{ struct stoken s;
+			  mark_struct_expression ();
+			  write_exp_elt_opcode (STRUCTOP_STRUCT);
+			  s.ptr = "";
+			  s.length = 0;
+			  write_exp_string (s);
+			  write_exp_elt_opcode (STRUCTOP_STRUCT); }
+	;
+
 exp	:	exp '.' qualified_name
 			{ /* exp.type::name becomes exp.*(&type::name) */
 			  /* Note: this doesn't work if name is a
@@ -1341,6 +1376,16 @@
     {">=", GEQ, BINOP_END}
   };
 
+/* This is set if a NAME token appeared at the very end of the input
+   string, with no whitespace separating the name from the EOF.  This
+   is used only when parsing to do field name completion.  */
+static int saw_name_at_eof;
+
+/* This is set if the previously-returned token was a structure
+   operator -- either '.' or ARROW.  This is used only when parsing to
+   do field name completion.  */
+static int last_was_structop;
+
 /* Read one token, getting characters through lexptr.  */
 
 static int
@@ -1356,7 +1401,10 @@
   static int tempbufsize;
   char * token_string = NULL;
   int class_prefix = 0;
-   
+  int saw_structop = last_was_structop;
+
+  last_was_structop = 0;
+
  retry:
 
   /* Check if this is a macro invocation that we need to expand.  */
@@ -1388,6 +1436,8 @@
       {
 	lexptr += 2;
 	yylval.opcode = tokentab2[i].opcode;
+	if (in_parse_field && tokentab2[i].opcode == ARROW)
+	  last_was_structop = 1;
 	return tokentab2[i].token;
       }
 
@@ -1396,6 +1446,8 @@
     case 0:
       /* If we were just scanning the result of a macro expansion,
          then we need to resume scanning the original text.
+	 If we're parsing for field name completion, and the previous
+	 token allows such completion, return a COMPLETE token.
          Otherwise, we were already scanning the original text, and
          we're really done.  */
       if (scanning_macro_expansion ())
@@ -1403,6 +1455,13 @@
           finished_macro_expansion ();
           goto retry;
         }
+      else if (saw_name_at_eof)
+	{
+	  saw_name_at_eof = 0;
+	  return COMPLETE;
+	}
+      else if (saw_structop)
+	return COMPLETE;
       else
         return 0;
 
@@ -1475,7 +1534,11 @@
     case '.':
       /* Might be a floating point number.  */
       if (lexptr[1] < '0' || lexptr[1] > '9')
-	goto symbol;		/* Nope, must be a symbol. */
+	{
+	  if (in_parse_field)
+	    last_was_structop = 1;
+	  goto symbol;		/* Nope, must be a symbol. */
+	}
       /* FALL THRU into number case.  */
 
     case '0':
@@ -1812,10 +1875,20 @@
     /* Any other kind of symbol */
     yylval.ssym.sym = sym;
     yylval.ssym.is_a_field_of_this = is_a_field_of_this;
+    if (in_parse_field && *lexptr == '\0')
+      saw_name_at_eof = 1;
     return NAME;
   }
 }
 
+int
+c_parse (void)
+{
+  last_was_structop = 0;
+  saw_name_at_eof = 0;
+  return yyparse ();
+}
+
 void
 yyerror (msg)
      char *msg;
Index: completer.c
===================================================================
RCS file: /cvs/src/src/gdb/completer.c,v
retrieving revision 1.24
diff -u -r1.24 completer.c
--- completer.c	1 Jan 2008 22:53:09 -0000	1.24
+++ completer.c	12 May 2008 23:51:43 -0000
@@ -338,6 +338,41 @@
   return list;
 }
 
+/* Complete on expressions.  Often this means completing on symbol
+   names, but some language parsers also have support for completing
+   field names.  */
+char **
+expression_completer (char *text, char *word)
+{
+  struct type *type;
+  char *fieldname;
+
+  /* Perform a tentative parse of the expression, to see whether a
+     field completion is required.  */
+  fieldname = NULL;
+  type = parse_field_expression (text, &fieldname);
+  if (fieldname && type && (TYPE_CODE (type) == TYPE_CODE_UNION
+			    || TYPE_CODE (type) == TYPE_CODE_STRUCT))
+    {
+      int min = TYPE_N_BASECLASSES (type);
+      int alloc = TYPE_NFIELDS (type) - min;
+      int flen = strlen (fieldname);
+      int i, out = 0;
+      char **result = (char **) xmalloc ((alloc + 1) * sizeof (char *));
+      for (i = 0; i < alloc; ++i)
+	{
+	  char *name = TYPE_FIELD_NAME (type, min + i);
+	  if (name && ! strncmp (name, fieldname, flen))
+	    result[out++] = xstrdup (name);
+	}
+      result[out] = NULL;
+      return result;
+    }
+
+  /* Not ideal but it is what we used to do before... */
+  return location_completer (text, word);
+}
+
 /* Complete on command names.  Used by "help".  */
 char **
 command_completer (char *text, char *word)
@@ -520,7 +555,8 @@
 		      rl_completer_word_break_characters =
 			gdb_completer_file_name_break_characters;
 		    }
-		  else if (c->completer == location_completer)
+		  else if (c->completer == location_completer
+			   || c->completer == expression_completer)
 		    {
 		      /* Commands which complete on locations want to
 			 see the entire argument.  */
@@ -588,7 +624,8 @@
 		  rl_completer_word_break_characters =
 		    gdb_completer_file_name_break_characters;
 		}
-	      else if (c->completer == location_completer)
+	      else if (c->completer == location_completer
+		       || c->completer == expression_completer)
 		{
 		  for (p = word;
 		       p > tmp_command
Index: completer.h
===================================================================
RCS file: /cvs/src/src/gdb/completer.h,v
retrieving revision 1.13
diff -u -r1.13 completer.h
--- completer.h	1 Jan 2008 22:53:09 -0000	1.13
+++ completer.h	12 May 2008 23:51:43 -0000
@@ -25,6 +25,8 @@
 
 extern char **filename_completer (char *, char *);
 
+extern char **expression_completer (char *, char *);
+
 extern char **location_completer (char *, char *);
 
 extern char **command_completer (char *, char *);
Index: eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.83
diff -u -r1.83 eval.c
--- eval.c	22 Apr 2008 11:03:41 -0000	1.83
+++ eval.c	12 May 2008 23:51:44 -0000
@@ -175,6 +175,36 @@
   return evaluate_subexp (NULL_TYPE, exp, &pc, EVAL_AVOID_SIDE_EFFECTS);
 }
 
+/* Evaluate a subexpression, avoiding all memory references and
+   getting a value whose type alone is correct.  */
+
+struct value *
+evaluate_subexpression_type (struct expression *exp, int subexp)
+{
+  return evaluate_subexp (NULL_TYPE, exp, &subexp, EVAL_AVOID_SIDE_EFFECTS);
+}
+
+/* Extract a field operation from an expression.  If the subexpression
+   of EXP starting at *SUBEXP is not a structure dereference
+   operation, return NULL.  Otherwise, return the name of the
+   dereferenced field, and advance *SUBEXP to point to the
+   subexpression of the left-hand-side of the dereference.  This is
+   used when completing field names.  */
+
+char *
+extract_field_op (struct expression *exp, int *subexp)
+{
+  int tem;
+  char *result;
+  if (exp->elts[*subexp].opcode != STRUCTOP_STRUCT
+      && exp->elts[*subexp].opcode != STRUCTOP_PTR)
+    return NULL;
+  tem = longest_to_int (exp->elts[*subexp + 1].longconst);
+  result = &exp->elts[*subexp + 2].string;
+  (*subexp) += 1 + 3 + BYTES_TO_EXP_ELEM (tem + 1);
+  return result;
+}
+
 /* If the next expression is an OP_LABELED, skips past it,
    returning the label.  Otherwise, does nothing and returns NULL. */
 
Index: expression.h
===================================================================
RCS file: /cvs/src/src/gdb/expression.h,v
retrieving revision 1.26
diff -u -r1.26 expression.h
--- expression.h	1 Jan 2008 22:53:09 -0000	1.26
+++ expression.h	12 May 2008 23:51:44 -0000
@@ -389,8 +389,14 @@
 
 extern struct expression *parse_expression (char *);
 
+extern struct type *parse_field_expression (char *, char **);
+
 extern struct expression *parse_exp_1 (char **, struct block *, int);
 
+/* For use by parsers; set if we want to parse an expression and
+   attempt to complete a field name.  */
+extern int in_parse_field;
+
 /* The innermost context required by the stack and register variables
    we've encountered so far.  To use this, set it to NULL, then call
    parse_<whatever>, then look at it.  */
Index: parse.c
===================================================================
RCS file: /cvs/src/src/gdb/parse.c,v
retrieving revision 1.71
diff -u -r1.71 parse.c
--- parse.c	3 May 2008 22:30:51 -0000	1.71
+++ parse.c	12 May 2008 23:51:44 -0000
@@ -52,6 +52,7 @@
 #include "block.h"
 #include "source.h"
 #include "objfiles.h"
+#include "exceptions.h"
 
 /* Standard set of definitions for printing, dumping, prefixifying,
  * and evaluating expressions.  */
@@ -80,6 +81,15 @@
 int paren_depth;
 int comma_terminates;
 
+/* True if parsing an expression to find a field reference.  This is
+   only used by completion.  */
+int in_parse_field;
+
+/* The index of the last struct expression directly before a '.' or
+   '->'.  This is set when parsing and is only used when completing a
+   field name.  It is -1 if no dereference operation was found.  */
+static int expout_last_struct = -1;
+
 /* A temporary buffer for identifiers, so we can null-terminate them.
 
    We allocate this with xrealloc.  parse_exp_1 used to allocate with
@@ -100,13 +110,13 @@
 
 static void free_funcalls (void *ignore);
 
-static void prefixify_expression (struct expression *);
+static int prefixify_expression (struct expression *);
 
-static void prefixify_subexp (struct expression *, struct expression *, int,
-			      int);
+static int prefixify_subexp (struct expression *, struct expression *, int,
+			     int);
 
 static struct expression *parse_exp_in_context (char **, struct block *, int, 
-						int);
+						int, int *);
 
 void _initialize_parse (void);
 
@@ -460,6 +470,16 @@
     }
   write_exp_elt_opcode (UNOP_MEMVAL);
 }
+
+/* Mark the current index as the starting location of a structure
+   expression.  This is used when completing on field names.  */
+
+void
+mark_struct_expression (void)
+{
+  expout_last_struct = expout_ptr;
+}
+
 \f
 /* Recognize tokens that start with '$'.  These include:
 
@@ -666,7 +686,7 @@
 /* Reverse an expression from suffix form (in which it is constructed)
    to prefix form (in which we can conveniently print or execute it).  */
 
-static void
+static int
 prefixify_expression (struct expression *expr)
 {
   int len = sizeof (struct expression) + EXP_ELEM_TO_BYTES (expr->nelts);
@@ -678,7 +698,7 @@
   /* Copy the original expression into temp.  */
   memcpy (temp, expr, len);
 
-  prefixify_subexp (temp, expr, inpos, outpos);
+  return prefixify_subexp (temp, expr, inpos, outpos);
 }
 
 /* Return the number of exp_elements in the postfix subexpression 
@@ -877,7 +897,7 @@
    into OUTEXPR, starting at index OUTBEG.
    In the process, convert it from suffix to prefix form.  */
 
-static void
+static int
 prefixify_subexp (struct expression *inexpr,
 		  struct expression *outexpr, int inend, int outbeg)
 {
@@ -886,6 +906,7 @@
   int i;
   int *arglens;
   enum exp_opcode opcode;
+  int result = -1;
 
   operator_length (inexpr, inend, &oplen, &args);
 
@@ -896,6 +917,9 @@
 	  EXP_ELEM_TO_BYTES (oplen));
   outbeg += oplen;
 
+  if (expout_last_struct == inend)
+    result = outbeg - oplen;
+
   /* Find the lengths of the arg subexpressions.  */
   arglens = (int *) alloca (args * sizeof (int));
   for (i = args - 1; i >= 0; i--)
@@ -913,11 +937,21 @@
      outbeg does similarly in the output.  */
   for (i = 0; i < args; i++)
     {
+      int r;
       oplen = arglens[i];
       inend += oplen;
-      prefixify_subexp (inexpr, outexpr, inend, outbeg);
+      r = prefixify_subexp (inexpr, outexpr, inend, outbeg);
+      if (r != -1)
+	{
+	  /* Return immediately.  We probably have only parsed a
+	     partial expression, so we don't want to try to reverse
+	     the other operands.  */
+	  return r;
+	}
       outbeg += oplen;
     }
+
+  return result;
 }
 \f
 /* This page contains the two entry points to this file.  */
@@ -935,7 +969,7 @@
 struct expression *
 parse_exp_1 (char **stringptr, struct block *block, int comma)
 {
-  return parse_exp_in_context (stringptr, block, comma, 0);
+  return parse_exp_in_context (stringptr, block, comma, 0, NULL);
 }
 
 /* As for parse_exp_1, except that if VOID_CONTEXT_P, then
@@ -943,15 +977,18 @@
 
 static struct expression *
 parse_exp_in_context (char **stringptr, struct block *block, int comma, 
-		      int void_context_p)
+		      int void_context_p, int *out_subexp)
 {
+  volatile struct gdb_exception except;
   struct cleanup *old_chain;
+  int subexp;
 
   lexptr = *stringptr;
   prev_lexptr = NULL;
 
   paren_depth = 0;
   type_stack_depth = 0;
+  expout_last_struct = -1;
 
   comma_terminates = comma;
 
@@ -986,10 +1023,20 @@
   expout = (struct expression *)
     xmalloc (sizeof (struct expression) + EXP_ELEM_TO_BYTES (expout_size));
   expout->language_defn = current_language;
-  make_cleanup (free_current_contents, &expout);
 
-  if (current_language->la_parser ())
-    current_language->la_error (NULL);
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      if (current_language->la_parser ())
+	current_language->la_error (NULL);
+    }
+  if (except.reason < 0)
+    {
+      if (! in_parse_field)
+	{
+	  xfree (expout);
+	  throw_exception (except);
+	}
+    }
 
   discard_cleanups (old_chain);
 
@@ -1009,7 +1056,9 @@
     dump_raw_expression (expout, gdb_stdlog,
 			 "before conversion to prefix form");
 
-  prefixify_expression (expout);
+  subexp = prefixify_expression (expout);
+  if (out_subexp)
+    *out_subexp = subexp;
 
   current_language->la_post_parser (&expout, void_context_p);
 
@@ -1033,6 +1082,45 @@
   return exp;
 }
 
+/* Parse STRING as an expression.  If parsing ends in the middle of a
+   field reference, return the type of the left-hand-side of the
+   reference; furthermore, if the parsing ends in the field name,
+   return the field name in *NAME.  In all other cases, return NULL.  */
+
+struct type *
+parse_field_expression (char *string, char **name)
+{
+  struct expression *exp = NULL;
+  struct value *val;
+  int subexp;
+  volatile struct gdb_exception except;
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      in_parse_field = 1;
+      exp = parse_exp_in_context (&string, 0, 0, 0, &subexp);
+    }
+  in_parse_field = 0;
+  if (except.reason < 0 || ! exp)
+    return NULL;
+  if (expout_last_struct == -1)
+    {
+      xfree (exp);
+      return NULL;
+    }
+
+  *name = extract_field_op (exp, &subexp);
+  if (!*name)
+    {
+      xfree (exp);
+      return NULL;
+    }
+  val = evaluate_subexpression_type (exp, subexp);
+  xfree (exp);
+
+  return value_type (val);
+}
+
 /* A post-parser that does nothing */
 
 void
Index: parser-defs.h
===================================================================
RCS file: /cvs/src/src/gdb/parser-defs.h,v
retrieving revision 1.26
diff -u -r1.26 parser-defs.h
--- parser-defs.h	1 Jan 2008 22:53:12 -0000	1.26
+++ parser-defs.h	12 May 2008 23:51:44 -0000
@@ -138,6 +138,8 @@
 
 extern void write_dollar_variable (struct stoken str);
 
+extern void mark_struct_expression (void);
+
 extern char *find_template_name_end (char *);
 
 extern void start_arglist (void);
Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.119
diff -u -r1.119 printcmd.c
--- printcmd.c	6 May 2008 21:34:59 -0000	1.119
+++ printcmd.c	12 May 2008 23:51:45 -0000
@@ -2420,7 +2420,7 @@
 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."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
 
   add_cmd ("variable", class_vars, set_command, _("\
 Evaluate expression EXP and assign result to variable VAR, using assignment\n\
@@ -2453,13 +2453,13 @@
 \n\
 EXP may be preceded with /FMT, where FMT is a format letter\n\
 but no count or size letter (see \"x\" command)."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
   add_com_alias ("p", "print", class_vars, 1);
 
   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."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
 
   add_setshow_uinteger_cmd ("max-symbolic-offset", no_class,
 			    &max_symbolic_offset, _("\
Index: value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.114
diff -u -r1.114 value.h
--- value.h	6 May 2008 21:34:59 -0000	1.114
+++ value.h	12 May 2008 23:51:45 -0000
@@ -420,6 +420,11 @@
 
 extern struct value *evaluate_type (struct expression *exp);
 
+extern struct value *evaluate_subexpression_type (struct expression *exp,
+						  int subexp);
+
+extern char *extract_field_op (struct expression *exp, int *subexp);
+
 extern struct value *evaluate_subexp_with_coercion (struct expression *,
 						    int *, enum noside);
 
Index: testsuite/gdb.base/break1.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/break1.c,v
retrieving revision 1.4
diff -u -r1.4 break1.c
--- testsuite/gdb.base/break1.c	1 Jan 2008 22:53:18 -0000	1.4
+++ testsuite/gdb.base/break1.c	12 May 2008 23:51:46 -0000
@@ -41,3 +41,12 @@
 void marker3 (a, b) char *a, *b; {}	/* set breakpoint 18 here */
 void marker4 (d) long d; {}		/* set breakpoint 13 here */
 #endif
+
+/* A structure we use for field name completion tests.  */
+struct some_struct
+{
+  int a_field;
+  int b_field;
+};
+
+struct some_struct values[50];
Index: testsuite/gdb.base/completion.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/completion.exp,v
retrieving revision 1.29
diff -u -r1.29 completion.exp
--- testsuite/gdb.base/completion.exp	2 May 2008 20:30:49 -0000	1.29
+++ testsuite/gdb.base/completion.exp	12 May 2008 23:51:46 -0000
@@ -637,6 +637,22 @@
     timeout { fail "(timeout) complete (2) 'p no_var_named_this-'" }
 }
 
+send_gdb "p values\[0\].a\t"
+sleep 3
+gdb_expect  {
+        -re "^p values.0..a_field $"\
+            { send_gdb "\n"
+	      sleep 1
+              gdb_expect {
+                      -re "^.* = 0.*$gdb_prompt $"\
+                                        { pass "complete 'p values\[0\].a'"}
+                      -re ".*$gdb_prompt $" { fail "complete 'p values\[0\].a'"}
+                      timeout           {fail "(timeout) complete 'p values\[0\].a'"}
+                     }
+            }
+        -re ".*$gdb_prompt $"       { fail "complete 'p values\[0\].a'" }
+        timeout         { fail "(timeout) complete 'p values\[0\].a' 2" }
+        }
 
 # The following tests used to simply try to complete `${objdir}/file',
 # and so on.  The problem is that ${objdir} can be very long; the

Re: [RFA] Patch to limit field name completion candidates

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Date: Mon, 12 May 2008 17:52:06 -0600
> 
> For a long time now I've accidentally typed things like:
> 
>     p somestruct.<TAB>
> 
> ... and had to wait while gdb dumped all available symbols to gud.
> 
> It seemed to me that gdb should know that I'm trying to complete a
> field expression and limit the completions to fields actually
> occurring in the structure type on the left hand side.

Ooh, another completion junkie!  Good, I'm also hooked.

> I only updated the C parser.  This code works by modifying the lexer
> to return a special COMPLETE token in the important cases.  Note that
> it completes both "p foo.TAB" and "p foo.somethingTAB" correctly --
> the former by making an expression to a field with an empty name.

Thanks.  But what about the situation where I actually want to type

  p foo.c:bar

If I type "p foo.<TAB>", will I see "foo.c" as one of the possible
completions, after your change, whether there is or isn't also a
struct foo in the program?

Re: [RFA] Patch to limit field name completion candidates

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

>> I only updated the C parser.  This code works by modifying the lexer
>> to return a special COMPLETE token in the important cases.  Note that
>> it completes both "p foo.TAB" and "p foo.somethingTAB" correctly --
>> the former by making an expression to a field with an empty name.

Eli> Thanks.  But what about the situation where I actually want to type
Eli>   p foo.c:bar
Eli> If I type "p foo.<TAB>", will I see "foo.c" as one of the possible
Eli> completions, after your change, whether there is or isn't also a
Eli> struct foo in the program?

Nope, it won't.

I didn't realize this syntax was permissible.  And I can't seem to get
it to work on a simple test case:

Breakpoint 1, main () at s.c:8
8         return values[57].free2;
(gdb) p s.c:values[0]
No symbol "s" in current context.

How can I see this in action?

Tom

Re: [RFA] Patch to limit field name completion candidates

[Daniel Jacobowitz]

On Tue, May 13, 2008 at 09:31:20AM -0600, Tom Tromey wrote:
> Nope, it won't.
> 
> I didn't realize this syntax was permissible.  And I can't seem to get
> it to work on a simple test case:
> 
> Breakpoint 1, main () at s.c:8
> 8         return values[57].free2;
> (gdb) p s.c:values[0]
> No symbol "s" in current context.
> 
> How can I see this in action?

The only way I know of is 'filename.c'::bar, including the quotes
(see scope.exp).

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Patch to limit field name completion candidates

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Daniel> The only way I know of is 'filename.c'::bar, including the quotes
Daniel> (see scope.exp).

Thanks.  In that case, the answer to Eli is yes, this continues to
work as it did before.  The quotes mean that the field name completion
is not triggered, so we fall back to location_completer.

Tom

Re: [RFA] Patch to limit field name completion candidates

[Eli Zaretskii]

> Cc: gdb-patches@sources.redhat.com
> From: Tom Tromey <tromey@redhat.com>
> Date: Tue, 13 May 2008 09:31:20 -0600
> 
> Eli> Thanks.  But what about the situation where I actually want to type
> Eli>   p foo.c:bar
> Eli> If I type "p foo.<TAB>", will I see "foo.c" as one of the possible
> Eli> completions, after your change, whether there is or isn't also a
> Eli> struct foo in the program?
> 
> Nope, it won't.
> 
> I didn't realize this syntax was permissible.  And I can't seem to get
> it to work on a simple test case:
> 
> Breakpoint 1, main () at s.c:8
> 8         return values[57].free2;
> (gdb) p s.c:values[0]
> No symbol "s" in current context.
> 
> How can I see this in action?

What if you type p s<TAB>, does it work then?

Re: [RFA] Patch to limit field name completion candidates

[Eli Zaretskii]

> Date: Tue, 13 May 2008 11:36:48 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sources.redhat.com
> 
> The only way I know of is 'filename.c'::bar, including the quotes
> (see scope.exp).

Just "p 'filename.<TAB>" is enough.

Re: [RFA] Patch to limit field name completion candidates

[Daniel Jacobowitz]

On Mon, May 12, 2008 at 05:52:06PM -0600, Tom Tromey wrote:
> How this works:
> 
> I added a new expression_completer and set this as the completion
> function for various things, like "p".  This tries to complete a field
> expression, but falls back to the old location_completer (which is
> what all these commands currently use).
> 
> The expression completer sets a global flag, in_parse_field, and then
> calls the expression parser.  The parsers are expected to look at this
> flag and call mark_struct_expression at the right time.  Then, if this
> was called, the generic code extracts the left-hand-side
> subexpression, gets its type, and then does field completion as you'd
> expect.
> 
> I only updated the C parser.  This code works by modifying the lexer
> to return a special COMPLETE token in the important cases.  Note that
> it completes both "p foo.TAB" and "p foo.somethingTAB" correctly --
> the former by making an expression to a field with an empty name.

Hi Tom,

Here's a few things I noticed.

We should complete after pointers and references, not just structs.
Especially true for arrow, as that is not especially useful on
structs.

I am concerned that relying on the parser to parse incomplete
expressions will not work out.  The bodies won't be run until the rule
is reduced, and there may not be enough context in what the user has
typed to reduce the field completion.  I'm not sure that's a real
problem, but I worry that it will be fragile.  Still - clearly better
than nothing.

I tried "p (*gdb_stdout).<tab>" and got no completions.  On the
testcase, "p (values[0]).<tab>" works so I am not sure what the
problem is.  Could you take a look at it?

Oh, and at least a NEWS entry would be good - and probably a manual
change too.

>     into OUTEXPR, starting at index OUTBEG.
>     In the process, convert it from suffix to prefix form.  */
>  
> -static void
> +static int
>  prefixify_subexp (struct expression *inexpr,
>  		  struct expression *outexpr, int inend, int outbeg)
>  {

Please document the return value.

Otherwise, the code looks fine.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Patch to limit field name completion candidates

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Daniel> We should complete after pointers and references, not just structs.
Daniel> Especially true for arrow, as that is not especially useful on
Daniel> structs.

Whoops, thanks.  I fixed this to work the same way as
lookup_struct_elt_type.

Daniel> I am concerned that relying on the parser to parse incomplete
Daniel> expressions will not work out.  The bodies won't be run until the rule
Daniel> is reduced, and there may not be enough context in what the user has
Daniel> typed to reduce the field completion.  I'm not sure that's a real
Daniel> problem, but I worry that it will be fragile.  Still - clearly better
Daniel> than nothing.

I thought about this too, but I couldn't think of a failing case.

We aren't parsing complete expressions, true, but I think by the time
the COMPLETE token is reduced, we will have reduced the entire LHS of
the struct op expression.  And, we throw away the rest of the
expression -- all the malformed parts.

So, I think it ought to be safe in all cases.

Daniel> I tried "p (*gdb_stdout).<tab>" and got no completions.  On the
Daniel> testcase, "p (values[0]).<tab>" works so I am not sure what the
Daniel> problem is.  Could you take a look at it?

Done.  The problem was not calling check_type.

Daniel> Oh, and at least a NEWS entry would be good - and probably a
Daniel> manual change too.

I added a NEWS entry.  I am not sure where in the manual to add
anything though.  FWIW I didn't do this originally since it seems more
like a QoI thing than a feature requiring documentation.

>> -static void
>> +static int
>> prefixify_subexp (struct expression *inexpr,
>> struct expression *outexpr, int inend, int outbeg)
>> {

Daniel> Please document the return value.

I updated this comment and a couple others.

Here's a new patch, let me know what you think.

I can help language maintainers update their parsers if need be.

Tom

ChangeLog:
2008-06-05  Tom Tromey  <tromey@redhat.com>

	* value.h (evaluate_subexpression_type, extract_field_op):
	Declare.
	* printcmd.c (_initialize_printcmd): Use expression_completer for
	'p', 'inspect', 'call'.
	* parser-defs.h (parse_field_expression): Declare.
	* parse.c: Include exceptions.h.
	(in_parse_field, expout_last_struct): New globals.
	(mark_struct_expression): New function.
	(prefixify_expression): Return int.
	(prefixify_subexp): Return int.  Use expout_last_struct.
	(parse_exp_1): Update.
	(parse_exp_in_context): Add 'out_subexp' argument.  Handle
	in_parse_field.
	(parse_field_expression): New function.
	* expression.h (parse_field_expression): Declare.
	(in_parse_field): Likewise.
	* eval.c (evaluate_subexpression_type): New function.
	(extract_field_op): Likewise.
	* completer.h (expression_completer): Declare.
	* completer.c (expression_completer): New function.
	* c-exp.y (yyparse): Redefine.
	(COMPLETE): New token.
	(exp): New productions.
	(saw_name_at_eof, last_was_structop): New globals.
	(yylex): Return COMPLETE when needed.  Recognize in_parse_field.
	(c_parse): New function.
	* breakpoint.c (_initialize_breakpoint): Use expression_completer
	for watch, awatch, and rwatch.
	* Makefile.in (parse.o): Depend on exceptions_h.

testsuite/ChangeLog:
2008-06-05  Tom Tromey  <tromey@redhat.com>

	* gdb.base/break1.c (struct some_struct): New struct.
	(values): New global.
	* gdb.base/completion.exp: Add field name completion test.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.1018
diff -u -r1.1018 Makefile.in
--- Makefile.in	9 May 2008 17:02:01 -0000	1.1018
+++ Makefile.in	5 Jun 2008 19:14:11 -0000
@@ -2591,7 +2591,8 @@
 parse.o: parse.c $(defs_h) $(gdb_string_h) $(symtab_h) $(gdbtypes_h) \
 	$(frame_h) $(expression_h) $(value_h) $(command_h) $(language_h) \
 	$(f_lang_h) $(parser_defs_h) $(gdbcmd_h) $(symfile_h) $(inferior_h) \
-	$(doublest_h) $(gdb_assert_h) $(block_h) $(source_h) $(objfiles_h)
+	$(doublest_h) $(gdb_assert_h) $(block_h) $(source_h) $(objfiles_h) \
+	$(exceptions_h)
 p-exp.o: p-exp.c $(defs_h) $(gdb_string_h) $(expression_h) $(value_h) \
 	$(parser_defs_h) $(language_h) $(p_lang_h) $(bfd_h) $(symfile_h) \
 	$(objfiles_h) $(block_h)
Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.273
diff -u -r1.273 NEWS
--- NEWS	9 May 2008 19:17:46 -0000	1.273
+++ NEWS	5 Jun 2008 19:14:12 -0000
@@ -3,6 +3,9 @@
 
 *** Changes since GDB 6.8
 
+* In expressions, completion of field operations will now return only
+the allowable field names.
+
 * New remote packets
 
 qSearch:memory:
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.321
diff -u -r1.321 breakpoint.c
--- breakpoint.c	4 May 2008 19:38:59 -0000	1.321
+++ breakpoint.c	5 Jun 2008 19:14:13 -0000
@@ -8433,19 +8433,19 @@
 Set a watchpoint for an expression.\n\
 A watchpoint stops execution of your program whenever the value of\n\
 an expression changes."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
 
   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."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
 
   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."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
 
   add_info ("watchpoints", breakpoints_info,
 	    _("Synonym for ``info breakpoints''."));
Index: c-exp.y
===================================================================
RCS file: /cvs/src/src/gdb/c-exp.y,v
retrieving revision 1.42
diff -u -r1.42 c-exp.y
--- c-exp.y	9 Jan 2008 19:27:15 -0000	1.42
+++ c-exp.y	5 Jun 2008 19:14:13 -0000
@@ -63,7 +63,7 @@
    generators need to be fixed instead of adding those names to this list. */
 
 #define	yymaxdepth c_maxdepth
-#define	yyparse	c_parse
+#define	yyparse	c_parse_internal
 #define	yylex	c_lex
 #define	yyerror	c_error
 #define	yylval	c_lval
@@ -180,6 +180,7 @@
 
 %token <sval> STRING
 %token <ssym> NAME /* BLOCKNAME defined below to give it higher precedence. */
+%token <voidval> COMPLETE
 %token <tsym> TYPENAME
 %type <sval> name
 %type <ssym> name_not_typename
@@ -301,6 +302,23 @@
 			  write_exp_elt_opcode (STRUCTOP_PTR); }
 	;
 
+exp	:	exp ARROW name COMPLETE
+			{ mark_struct_expression ();
+			  write_exp_elt_opcode (STRUCTOP_PTR);
+			  write_exp_string ($3);
+			  write_exp_elt_opcode (STRUCTOP_PTR); }
+	;
+
+exp	:	exp ARROW COMPLETE
+			{ struct stoken s;
+			  mark_struct_expression ();
+			  write_exp_elt_opcode (STRUCTOP_PTR);
+			  s.ptr = "";
+			  s.length = 0;
+			  write_exp_string (s);
+			  write_exp_elt_opcode (STRUCTOP_PTR); }
+	;
+
 exp	:	exp ARROW qualified_name
 			{ /* exp->type::name becomes exp->*(&type::name) */
 			  /* Note: this doesn't work if name is a
@@ -319,6 +337,23 @@
 			  write_exp_elt_opcode (STRUCTOP_STRUCT); }
 	;
 
+exp	:	exp '.' name COMPLETE
+			{ mark_struct_expression ();
+			  write_exp_elt_opcode (STRUCTOP_STRUCT);
+			  write_exp_string ($3);
+			  write_exp_elt_opcode (STRUCTOP_STRUCT); }
+	;
+
+exp	:	exp '.' COMPLETE
+			{ struct stoken s;
+			  mark_struct_expression ();
+			  write_exp_elt_opcode (STRUCTOP_STRUCT);
+			  s.ptr = "";
+			  s.length = 0;
+			  write_exp_string (s);
+			  write_exp_elt_opcode (STRUCTOP_STRUCT); }
+	;
+
 exp	:	exp '.' qualified_name
 			{ /* exp.type::name becomes exp.*(&type::name) */
 			  /* Note: this doesn't work if name is a
@@ -1341,6 +1376,16 @@
     {">=", GEQ, BINOP_END}
   };
 
+/* This is set if a NAME token appeared at the very end of the input
+   string, with no whitespace separating the name from the EOF.  This
+   is used only when parsing to do field name completion.  */
+static int saw_name_at_eof;
+
+/* This is set if the previously-returned token was a structure
+   operator -- either '.' or ARROW.  This is used only when parsing to
+   do field name completion.  */
+static int last_was_structop;
+
 /* Read one token, getting characters through lexptr.  */
 
 static int
@@ -1356,7 +1401,10 @@
   static int tempbufsize;
   char * token_string = NULL;
   int class_prefix = 0;
-   
+  int saw_structop = last_was_structop;
+
+  last_was_structop = 0;
+
  retry:
 
   /* Check if this is a macro invocation that we need to expand.  */
@@ -1388,6 +1436,8 @@
       {
 	lexptr += 2;
 	yylval.opcode = tokentab2[i].opcode;
+	if (in_parse_field && tokentab2[i].opcode == ARROW)
+	  last_was_structop = 1;
 	return tokentab2[i].token;
       }
 
@@ -1396,6 +1446,8 @@
     case 0:
       /* If we were just scanning the result of a macro expansion,
          then we need to resume scanning the original text.
+	 If we're parsing for field name completion, and the previous
+	 token allows such completion, return a COMPLETE token.
          Otherwise, we were already scanning the original text, and
          we're really done.  */
       if (scanning_macro_expansion ())
@@ -1403,6 +1455,13 @@
           finished_macro_expansion ();
           goto retry;
         }
+      else if (saw_name_at_eof)
+	{
+	  saw_name_at_eof = 0;
+	  return COMPLETE;
+	}
+      else if (saw_structop)
+	return COMPLETE;
       else
         return 0;
 
@@ -1475,7 +1534,11 @@
     case '.':
       /* Might be a floating point number.  */
       if (lexptr[1] < '0' || lexptr[1] > '9')
-	goto symbol;		/* Nope, must be a symbol. */
+	{
+	  if (in_parse_field)
+	    last_was_structop = 1;
+	  goto symbol;		/* Nope, must be a symbol. */
+	}
       /* FALL THRU into number case.  */
 
     case '0':
@@ -1812,10 +1875,20 @@
     /* Any other kind of symbol */
     yylval.ssym.sym = sym;
     yylval.ssym.is_a_field_of_this = is_a_field_of_this;
+    if (in_parse_field && *lexptr == '\0')
+      saw_name_at_eof = 1;
     return NAME;
   }
 }
 
+int
+c_parse (void)
+{
+  last_was_structop = 0;
+  saw_name_at_eof = 0;
+  return yyparse ();
+}
+
 void
 yyerror (msg)
      char *msg;
Index: c-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/c-lang.c,v
retrieving revision 1.53
diff -u -r1.53 c-lang.c
--- c-lang.c	6 Apr 2008 08:56:36 -0000	1.53
+++ c-lang.c	5 Jun 2008 19:14:13 -0000
@@ -261,13 +261,6 @@
 void *expression_macro_lookup_baton;
 
 
-static struct macro_definition *
-null_macro_lookup (const char *name, void *baton)
-{
-  return 0;
-}
-
-
 static int
 c_preprocess_and_parse (void)
 {
@@ -279,17 +272,11 @@
     scope = sal_macro_scope (find_pc_line (expression_context_pc, 0));
   else
     scope = default_macro_scope ();
+  if (! scope)
+    scope = user_macro_scope ();
 
-  if (scope)
-    {
-      expression_macro_lookup_func = standard_macro_lookup;
-      expression_macro_lookup_baton = (void *) scope;
-    }
-  else
-    {
-      expression_macro_lookup_func = null_macro_lookup;
-      expression_macro_lookup_baton = 0;      
-    }
+  expression_macro_lookup_func = standard_macro_lookup;
+  expression_macro_lookup_baton = (void *) scope;
 
   gdb_assert (! macro_original_text);
   make_cleanup (scan_macro_cleanup, 0);
Index: completer.c
===================================================================
RCS file: /cvs/src/src/gdb/completer.c,v
retrieving revision 1.24
diff -u -r1.24 completer.c
--- completer.c	1 Jan 2008 22:53:09 -0000	1.24
+++ completer.c	5 Jun 2008 19:14:13 -0000
@@ -338,6 +338,53 @@
   return list;
 }
 
+/* Complete on expressions.  Often this means completing on symbol
+   names, but some language parsers also have support for completing
+   field names.  */
+char **
+expression_completer (char *text, char *word)
+{
+  struct type *type;
+  char *fieldname;
+
+  /* Perform a tentative parse of the expression, to see whether a
+     field completion is required.  */
+  fieldname = NULL;
+  type = parse_field_expression (text, &fieldname);
+  if (fieldname && type)
+    {
+      for (;;)
+	{
+	  CHECK_TYPEDEF (type);
+	  if (TYPE_CODE (type) != TYPE_CODE_PTR
+	      && TYPE_CODE (type) != TYPE_CODE_REF)
+	    break;
+	  type = TYPE_TARGET_TYPE (type);
+	}
+
+      if (TYPE_CODE (type) == TYPE_CODE_UNION
+	  || TYPE_CODE (type) == TYPE_CODE_STRUCT)
+	{
+	  int min = TYPE_N_BASECLASSES (type);
+	  int alloc = TYPE_NFIELDS (type) - min;
+	  int flen = strlen (fieldname);
+	  int i, out = 0;
+	  char **result = (char **) xmalloc ((alloc + 1) * sizeof (char *));
+	  for (i = 0; i < alloc; ++i)
+	    {
+	      char *name = TYPE_FIELD_NAME (type, min + i);
+	      if (name && ! strncmp (name, fieldname, flen))
+		result[out++] = xstrdup (name);
+	    }
+	  result[out] = NULL;
+	  return result;
+	}
+    }
+
+  /* Not ideal but it is what we used to do before... */
+  return location_completer (text, word);
+}
+
 /* Complete on command names.  Used by "help".  */
 char **
 command_completer (char *text, char *word)
@@ -520,7 +567,8 @@
 		      rl_completer_word_break_characters =
 			gdb_completer_file_name_break_characters;
 		    }
-		  else if (c->completer == location_completer)
+		  else if (c->completer == location_completer
+			   || c->completer == expression_completer)
 		    {
 		      /* Commands which complete on locations want to
 			 see the entire argument.  */
@@ -588,7 +636,8 @@
 		  rl_completer_word_break_characters =
 		    gdb_completer_file_name_break_characters;
 		}
-	      else if (c->completer == location_completer)
+	      else if (c->completer == location_completer
+		       || c->completer == expression_completer)
 		{
 		  for (p = word;
 		       p > tmp_command
Index: completer.h
===================================================================
RCS file: /cvs/src/src/gdb/completer.h,v
retrieving revision 1.13
diff -u -r1.13 completer.h
--- completer.h	1 Jan 2008 22:53:09 -0000	1.13
+++ completer.h	5 Jun 2008 19:14:13 -0000
@@ -25,6 +25,8 @@
 
 extern char **filename_completer (char *, char *);
 
+extern char **expression_completer (char *, char *);
+
 extern char **location_completer (char *, char *);
 
 extern char **command_completer (char *, char *);
Index: eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.83
diff -u -r1.83 eval.c
--- eval.c	22 Apr 2008 11:03:41 -0000	1.83
+++ eval.c	5 Jun 2008 19:14:13 -0000
@@ -175,6 +175,36 @@
   return evaluate_subexp (NULL_TYPE, exp, &pc, EVAL_AVOID_SIDE_EFFECTS);
 }
 
+/* Evaluate a subexpression, avoiding all memory references and
+   getting a value whose type alone is correct.  */
+
+struct value *
+evaluate_subexpression_type (struct expression *exp, int subexp)
+{
+  return evaluate_subexp (NULL_TYPE, exp, &subexp, EVAL_AVOID_SIDE_EFFECTS);
+}
+
+/* Extract a field operation from an expression.  If the subexpression
+   of EXP starting at *SUBEXP is not a structure dereference
+   operation, return NULL.  Otherwise, return the name of the
+   dereferenced field, and advance *SUBEXP to point to the
+   subexpression of the left-hand-side of the dereference.  This is
+   used when completing field names.  */
+
+char *
+extract_field_op (struct expression *exp, int *subexp)
+{
+  int tem;
+  char *result;
+  if (exp->elts[*subexp].opcode != STRUCTOP_STRUCT
+      && exp->elts[*subexp].opcode != STRUCTOP_PTR)
+    return NULL;
+  tem = longest_to_int (exp->elts[*subexp + 1].longconst);
+  result = &exp->elts[*subexp + 2].string;
+  (*subexp) += 1 + 3 + BYTES_TO_EXP_ELEM (tem + 1);
+  return result;
+}
+
 /* If the next expression is an OP_LABELED, skips past it,
    returning the label.  Otherwise, does nothing and returns NULL. */
 
Index: expression.h
===================================================================
RCS file: /cvs/src/src/gdb/expression.h,v
retrieving revision 1.26
diff -u -r1.26 expression.h
--- expression.h	1 Jan 2008 22:53:09 -0000	1.26
+++ expression.h	5 Jun 2008 19:14:13 -0000
@@ -389,8 +389,14 @@
 
 extern struct expression *parse_expression (char *);
 
+extern struct type *parse_field_expression (char *, char **);
+
 extern struct expression *parse_exp_1 (char **, struct block *, int);
 
+/* For use by parsers; set if we want to parse an expression and
+   attempt to complete a field name.  */
+extern int in_parse_field;
+
 /* The innermost context required by the stack and register variables
    we've encountered so far.  To use this, set it to NULL, then call
    parse_<whatever>, then look at it.  */
Index: parse.c
===================================================================
RCS file: /cvs/src/src/gdb/parse.c,v
retrieving revision 1.71
diff -u -r1.71 parse.c
--- parse.c	3 May 2008 22:30:51 -0000	1.71
+++ parse.c	5 Jun 2008 19:14:13 -0000
@@ -52,6 +52,7 @@
 #include "block.h"
 #include "source.h"
 #include "objfiles.h"
+#include "exceptions.h"
 
 /* Standard set of definitions for printing, dumping, prefixifying,
  * and evaluating expressions.  */
@@ -80,6 +81,15 @@
 int paren_depth;
 int comma_terminates;
 
+/* True if parsing an expression to find a field reference.  This is
+   only used by completion.  */
+int in_parse_field;
+
+/* The index of the last struct expression directly before a '.' or
+   '->'.  This is set when parsing and is only used when completing a
+   field name.  It is -1 if no dereference operation was found.  */
+static int expout_last_struct = -1;
+
 /* A temporary buffer for identifiers, so we can null-terminate them.
 
    We allocate this with xrealloc.  parse_exp_1 used to allocate with
@@ -100,13 +110,13 @@
 
 static void free_funcalls (void *ignore);
 
-static void prefixify_expression (struct expression *);
+static int prefixify_expression (struct expression *);
 
-static void prefixify_subexp (struct expression *, struct expression *, int,
-			      int);
+static int prefixify_subexp (struct expression *, struct expression *, int,
+			     int);
 
 static struct expression *parse_exp_in_context (char **, struct block *, int, 
-						int);
+						int, int *);
 
 void _initialize_parse (void);
 
@@ -460,6 +470,16 @@
     }
   write_exp_elt_opcode (UNOP_MEMVAL);
 }
+
+/* Mark the current index as the starting location of a structure
+   expression.  This is used when completing on field names.  */
+
+void
+mark_struct_expression (void)
+{
+  expout_last_struct = expout_ptr;
+}
+
 \f
 /* Recognize tokens that start with '$'.  These include:
 
@@ -664,9 +684,13 @@
 }
 \f
 /* Reverse an expression from suffix form (in which it is constructed)
-   to prefix form (in which we can conveniently print or execute it).  */
+   to prefix form (in which we can conveniently print or execute it).
+   Ordinarily this always returns -1.  However, if EXPOUT_LAST_STRUCT
+   is not -1 (i.e., we are trying to complete a field name), it will
+   return the index of the subexpression which is the left-hand-side
+   of the struct operation at EXPOUT_LAST_STRUCT.  */
 
-static void
+static int
 prefixify_expression (struct expression *expr)
 {
   int len = sizeof (struct expression) + EXP_ELEM_TO_BYTES (expr->nelts);
@@ -678,7 +702,7 @@
   /* Copy the original expression into temp.  */
   memcpy (temp, expr, len);
 
-  prefixify_subexp (temp, expr, inpos, outpos);
+  return prefixify_subexp (temp, expr, inpos, outpos);
 }
 
 /* Return the number of exp_elements in the postfix subexpression 
@@ -875,9 +899,12 @@
 
 /* Copy the subexpression ending just before index INEND in INEXPR
    into OUTEXPR, starting at index OUTBEG.
-   In the process, convert it from suffix to prefix form.  */
+   In the process, convert it from suffix to prefix form.
+   If EXPOUT_LAST_STRUCT is -1, then this function always returns -1.
+   Otherwise, it returns the index of the subexpression which is the
+   left-hand-side of the expression at EXPOUT_LAST_STRUCT.  */
 
-static void
+static int
 prefixify_subexp (struct expression *inexpr,
 		  struct expression *outexpr, int inend, int outbeg)
 {
@@ -886,6 +913,7 @@
   int i;
   int *arglens;
   enum exp_opcode opcode;
+  int result = -1;
 
   operator_length (inexpr, inend, &oplen, &args);
 
@@ -896,6 +924,9 @@
 	  EXP_ELEM_TO_BYTES (oplen));
   outbeg += oplen;
 
+  if (expout_last_struct == inend)
+    result = outbeg - oplen;
+
   /* Find the lengths of the arg subexpressions.  */
   arglens = (int *) alloca (args * sizeof (int));
   for (i = args - 1; i >= 0; i--)
@@ -913,11 +944,21 @@
      outbeg does similarly in the output.  */
   for (i = 0; i < args; i++)
     {
+      int r;
       oplen = arglens[i];
       inend += oplen;
-      prefixify_subexp (inexpr, outexpr, inend, outbeg);
+      r = prefixify_subexp (inexpr, outexpr, inend, outbeg);
+      if (r != -1)
+	{
+	  /* Return immediately.  We probably have only parsed a
+	     partial expression, so we don't want to try to reverse
+	     the other operands.  */
+	  return r;
+	}
       outbeg += oplen;
     }
+
+  return result;
 }
 \f
 /* This page contains the two entry points to this file.  */
@@ -935,23 +976,30 @@
 struct expression *
 parse_exp_1 (char **stringptr, struct block *block, int comma)
 {
-  return parse_exp_in_context (stringptr, block, comma, 0);
+  return parse_exp_in_context (stringptr, block, comma, 0, NULL);
 }
 
 /* As for parse_exp_1, except that if VOID_CONTEXT_P, then
-   no value is expected from the expression.  */
+   no value is expected from the expression.
+   OUT_SUBEXP is set when attempting to complete a field name; in this
+   case it is set to the index of the subexpression on the
+   left-hand-side of the struct op.  If not doing such completion, it
+   is left untouched.  */
 
 static struct expression *
 parse_exp_in_context (char **stringptr, struct block *block, int comma, 
-		      int void_context_p)
+		      int void_context_p, int *out_subexp)
 {
+  volatile struct gdb_exception except;
   struct cleanup *old_chain;
+  int subexp;
 
   lexptr = *stringptr;
   prev_lexptr = NULL;
 
   paren_depth = 0;
   type_stack_depth = 0;
+  expout_last_struct = -1;
 
   comma_terminates = comma;
 
@@ -986,10 +1034,20 @@
   expout = (struct expression *)
     xmalloc (sizeof (struct expression) + EXP_ELEM_TO_BYTES (expout_size));
   expout->language_defn = current_language;
-  make_cleanup (free_current_contents, &expout);
 
-  if (current_language->la_parser ())
-    current_language->la_error (NULL);
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      if (current_language->la_parser ())
+	current_language->la_error (NULL);
+    }
+  if (except.reason < 0)
+    {
+      if (! in_parse_field)
+	{
+	  xfree (expout);
+	  throw_exception (except);
+	}
+    }
 
   discard_cleanups (old_chain);
 
@@ -1009,7 +1067,9 @@
     dump_raw_expression (expout, gdb_stdlog,
 			 "before conversion to prefix form");
 
-  prefixify_expression (expout);
+  subexp = prefixify_expression (expout);
+  if (out_subexp)
+    *out_subexp = subexp;
 
   current_language->la_post_parser (&expout, void_context_p);
 
@@ -1033,6 +1093,45 @@
   return exp;
 }
 
+/* Parse STRING as an expression.  If parsing ends in the middle of a
+   field reference, return the type of the left-hand-side of the
+   reference; furthermore, if the parsing ends in the field name,
+   return the field name in *NAME.  In all other cases, return NULL.  */
+
+struct type *
+parse_field_expression (char *string, char **name)
+{
+  struct expression *exp = NULL;
+  struct value *val;
+  int subexp;
+  volatile struct gdb_exception except;
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      in_parse_field = 1;
+      exp = parse_exp_in_context (&string, 0, 0, 0, &subexp);
+    }
+  in_parse_field = 0;
+  if (except.reason < 0 || ! exp)
+    return NULL;
+  if (expout_last_struct == -1)
+    {
+      xfree (exp);
+      return NULL;
+    }
+
+  *name = extract_field_op (exp, &subexp);
+  if (!*name)
+    {
+      xfree (exp);
+      return NULL;
+    }
+  val = evaluate_subexpression_type (exp, subexp);
+  xfree (exp);
+
+  return value_type (val);
+}
+
 /* A post-parser that does nothing */
 
 void
Index: parser-defs.h
===================================================================
RCS file: /cvs/src/src/gdb/parser-defs.h,v
retrieving revision 1.26
diff -u -r1.26 parser-defs.h
--- parser-defs.h	1 Jan 2008 22:53:12 -0000	1.26
+++ parser-defs.h	5 Jun 2008 19:14:13 -0000
@@ -138,6 +138,8 @@
 
 extern void write_dollar_variable (struct stoken str);
 
+extern void mark_struct_expression (void);
+
 extern char *find_template_name_end (char *);
 
 extern void start_arglist (void);
Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.119
diff -u -r1.119 printcmd.c
--- printcmd.c	6 May 2008 21:34:59 -0000	1.119
+++ printcmd.c	5 Jun 2008 19:14:13 -0000
@@ -2420,7 +2420,7 @@
 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."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
 
   add_cmd ("variable", class_vars, set_command, _("\
 Evaluate expression EXP and assign result to variable VAR, using assignment\n\
@@ -2453,13 +2453,13 @@
 \n\
 EXP may be preceded with /FMT, where FMT is a format letter\n\
 but no count or size letter (see \"x\" command)."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
   add_com_alias ("p", "print", class_vars, 1);
 
   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."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
 
   add_setshow_uinteger_cmd ("max-symbolic-offset", no_class,
 			    &max_symbolic_offset, _("\
Index: value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.114
diff -u -r1.114 value.h
--- value.h	6 May 2008 21:34:59 -0000	1.114
+++ value.h	5 Jun 2008 19:14:13 -0000
@@ -420,6 +420,11 @@
 
 extern struct value *evaluate_type (struct expression *exp);
 
+extern struct value *evaluate_subexpression_type (struct expression *exp,
+						  int subexp);
+
+extern char *extract_field_op (struct expression *exp, int *subexp);
+
 extern struct value *evaluate_subexp_with_coercion (struct expression *,
 						    int *, enum noside);
 
Index: testsuite/gdb.base/break1.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/break1.c,v
retrieving revision 1.4
diff -u -r1.4 break1.c
--- testsuite/gdb.base/break1.c	1 Jan 2008 22:53:18 -0000	1.4
+++ testsuite/gdb.base/break1.c	5 Jun 2008 19:14:18 -0000
@@ -41,3 +41,12 @@
 void marker3 (a, b) char *a, *b; {}	/* set breakpoint 18 here */
 void marker4 (d) long d; {}		/* set breakpoint 13 here */
 #endif
+
+/* A structure we use for field name completion tests.  */
+struct some_struct
+{
+  int a_field;
+  int b_field;
+};
+
+struct some_struct values[50];
Index: testsuite/gdb.base/completion.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/completion.exp,v
retrieving revision 1.29
diff -u -r1.29 completion.exp
--- testsuite/gdb.base/completion.exp	2 May 2008 20:30:49 -0000	1.29
+++ testsuite/gdb.base/completion.exp	5 Jun 2008 19:14:18 -0000
@@ -637,6 +637,22 @@
     timeout { fail "(timeout) complete (2) 'p no_var_named_this-'" }
 }
 
+send_gdb "p values\[0\].a\t"
+sleep 3
+gdb_expect  {
+        -re "^p values.0..a_field $"\
+            { send_gdb "\n"
+	      sleep 1
+              gdb_expect {
+                      -re "^.* = 0.*$gdb_prompt $"\
+                                        { pass "complete 'p values\[0\].a'"}
+                      -re ".*$gdb_prompt $" { fail "complete 'p values\[0\].a'"}
+                      timeout           {fail "(timeout) complete 'p values\[0\].a'"}
+                     }
+            }
+        -re ".*$gdb_prompt $"       { fail "complete 'p values\[0\].a'" }
+        timeout         { fail "(timeout) complete 'p values\[0\].a' 2" }
+        }
 
 # The following tests used to simply try to complete `${objdir}/file',
 # and so on.  The problem is that ${objdir} can be very long; the

Re: [RFA] Patch to limit field name completion candidates

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Date: Thu, 05 Jun 2008 13:21:02 -0600
> 
> --- NEWS	9 May 2008 19:17:46 -0000	1.273
> +++ NEWS	5 Jun 2008 19:14:12 -0000
> @@ -3,6 +3,9 @@
>  
>  *** Changes since GDB 6.8
>  
> +* In expressions, completion of field operations will now return only
> +the allowable field names.

Thanks, but this text leaves too many questions unanswered (what is
``allowable field names''? allowable by whom? what are ``field
operations''? etc.).  I think an example will go a long way towards
making this clear.

Re: [RFA] Patch to limit field name completion candidates

[Daniel Jacobowitz]

On Thu, Jun 05, 2008 at 01:21:02PM -0600, Tom Tromey wrote:
> Daniel> I am concerned that relying on the parser to parse incomplete
> Daniel> expressions will not work out.  The bodies won't be run until the rule
> Daniel> is reduced, and there may not be enough context in what the user has
> Daniel> typed to reduce the field completion.  I'm not sure that's a real
> Daniel> problem, but I worry that it will be fragile.  Still - clearly better
> Daniel> than nothing.
> 
> I thought about this too, but I couldn't think of a failing case.
> 
> We aren't parsing complete expressions, true, but I think by the time
> the COMPLETE token is reduced, we will have reduced the entire LHS of
> the struct op expression.  And, we throw away the rest of the
> expression -- all the malformed parts.
> 
> So, I think it ought to be safe in all cases.

Safe, yes.  That's not the failure mode I was worried about.  I'm
wondering if we will ever error out before we reduce the COMPLETE.
But it seems to work so far.

> Daniel> Oh, and at least a NEWS entry would be good - and probably a
> Daniel> manual change too.
> 
> I added a NEWS entry.  I am not sure where in the manual to add
> anything though.  FWIW I didn't do this originally since it seems more
> like a QoI thing than a feature requiring documentation.

Yes, I understand - but at the same time, the more of our QoI we crow
about, the more users will take advantage of it, and the cooler
they'll think GDB is.  If it was worth implementing, it's also worth
documenting.

Another example near the bottom of Command Completion would probably
do it.

The patch and test are OK!  Work out the NEWS and manual bits with
Eli, and you're home free.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Patch to limit field name completion candidates

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> Thanks, but this text leaves too many questions unanswered (what is
Eli> ``allowable field names''? allowable by whom? what are ``field
Eli> operations''? etc.).  I think an example will go a long way towards
Eli> making this clear.

How about this?

* When completing in expressions, gdb will attempt to limit
completions to allowable fields, where appropriate.  For instance,
consider:

    # struct example { int f1; double f2; };
    # struct example variable;
    (gdb) p variable.

If the user types TAB at the end of this command line, the available
completions will be "f1" and "f2".



Tom

Re: [RFA] Patch to limit field name completion candidates

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

>> We aren't parsing complete expressions, true, but I think by the time
>> the COMPLETE token is reduced, we will have reduced the entire LHS of
>> the struct op expression.  And, we throw away the rest of the
>> expression -- all the malformed parts.
>> 
>> So, I think it ought to be safe in all cases.

Daniel> Safe, yes.  That's not the failure mode I was worried about.  I'm
Daniel> wondering if we will ever error out before we reduce the COMPLETE.
Daniel> But it seems to work so far.

In that case, nothing will call mark_struct_expression, and so the
completion machinery will not attempt field name completion.

Daniel> Another example near the bottom of Command Completion would probably
Daniel> do it.

Ok.  I will add this.

Tom

Re: [RFA] Patch to limit field name completion candidates

[Daniel Jacobowitz]

On Thu, Jun 05, 2008 at 02:03:10PM -0600, Tom Tromey wrote:
> Daniel> Safe, yes.  That's not the failure mode I was worried about.  I'm
> Daniel> wondering if we will ever error out before we reduce the COMPLETE.
> Daniel> But it seems to work so far.
> 
> In that case, nothing will call mark_struct_expression, and so the
> completion machinery will not attempt field name completion.

Right.  But this would appear to be a bug from the user's point of
view.  We'll just call it a hypothetical limitation of the
implementation :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Patch to limit field name completion candidates

[Tom Tromey]

Daniel> The patch and test are OK!  Work out the NEWS and manual bits with
Daniel> Eli, and you're home free.

The NEWS part is pending.

Eli, how does this look for the manual?

Tom

2008-06-05  Tom Tromey  <tromey@redhat.com>

	* gdb.texinfo (Completion): Add field name example.

*** gdb.texinfo	10 May 2008 09:06:02 -0600	1.495
--- gdb.texinfo	05 Jun 2008 14:40:33 -0600	
***************
*** 1,6 ****
  \input texinfo      @c -*-texinfo-*-
  @c Copyright (C) 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1998,
! @c 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006
  @c Free Software Foundation, Inc.
  @c
  @c %**start of header
--- 1,6 ----
  \input texinfo      @c -*-texinfo-*-
  @c Copyright (C) 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1998,
! @c 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2008
  @c Free Software Foundation, Inc.
  @c
  @c %**start of header
***************
*** 1521,1526 ****
--- 1521,1538 ----
  overload-resolution off} to disable overload resolution;
  see @ref{Debugging C Plus Plus, ,@value{GDBN} Features for C@t{++}}.
  
+ @cindex completion of field names
+ @cindex field name completion
+ When completing in an expression which looks up a field in a
+ structure, @value{GDBN} also tries to limit completions to the field
+ names available in the type of the left-hand-side:
+ 
+ @smallexample
+ (@value{GDBP}) p gdb_stdout.@key{M-?}
+ magic      to_delete  to_fputs   to_put     to_rewind  
+ to_data    to_flush   to_isatty  to_read    to_write   
+ @end smallexample
+ 
  
  @node Help
  @section Getting Help

Re: [RFA] Patch to limit field name completion candidates

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

>> In that case, nothing will call mark_struct_expression, and so the
>> completion machinery will not attempt field name completion.

Daniel> Right.  But this would appear to be a bug from the user's point of
Daniel> view.  We'll just call it a hypothetical limitation of the
Daniel> implementation :-)

Ah, yeah, I see what you mean: there can definitely be losing cases.
For instance if you try to complete "nonexistingThing->", you'll just
get all the symbols again.

I'm not sure what to do here... I guess we could try to get completion
to print an error to the user.  This could affect only the situation
where we thought we could complete a field name based on the syntax,
but then failed during "semantic analysis".  (I don't know offhand if
readline can do this for us or not.)

Another thing not accounted for in this implementation is that for
some languages you may want to complete more things.  For instance,
this would have to be expanded to work properly for Java, because in
Java you can have a type name, a field name, or method name after ".".
I think this isn't a big problem; the current code is, IMO, a decent
step in that direction.

Tom

Re: [RFA] Patch to limit field name completion candidates

[Daniel Jacobowitz]

On Thu, Jun 05, 2008 at 06:49:03PM -0600, Tom Tromey wrote:
> Another thing not accounted for in this implementation is that for
> some languages you may want to complete more things.  For instance,
> this would have to be expanded to work properly for Java, because in
> Java you can have a type name, a field name, or method name after ".".
> I think this isn't a big problem; the current code is, IMO, a decent
> step in that direction.

Oh gosh.  I had something important to comment on in my review and I
totally forgot it.

You don't recurse into base classes... for C++ this is obviously a
substantial problem.  Then there's the additional question of dynamic
type.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Patch to limit field name completion candidates

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Daniel> You don't recurse into base classes... for C++ this is
Daniel> obviously a substantial problem.

Thanks -- I will look at this.

Daniel> Then there's the additional question of dynamic type.

I did think about this one, but forgot to mention it.

I don't see a good way to make it work in general.  It could be made
to work sometimes, but not always.  In particular I think we could add
a "don't evaluate side effects" mode to expression evaluation -- this
would do an ok job except for the case of an expression involving an
inferior function call.  In that case I think we could still only use
the static type.

Tom

Re: [RFA] Patch to limit field name completion candidates

[Eli Zaretskii]

> Cc: gdb-patches@sources.redhat.com
> From: Tom Tromey <tromey@redhat.com>
> Date: Thu, 05 Jun 2008 14:01:49 -0600
> 
> How about this?
> 
> * When completing in expressions, gdb will attempt to limit
> completions to allowable fields, where appropriate.  For instance,
> consider:
> 
>     # struct example { int f1; double f2; };
>     # struct example variable;
>     (gdb) p variable.
> 
> If the user types TAB at the end of this command line, the available
> completions will be "f1" and "f2".

This is fine, but please say "structure fields" in the 1st sentence
above (the one that follows the asterisk).

Re: [RFA] Patch to limit field name completion candidates

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Date: Thu, 05 Jun 2008 14:49:35 -0600
> 
> + @cindex completion of field names
> + @cindex field name completion
> + When completing in an expression which looks up a field in a
> + structure, @value{GDBN} also tries to limit completions to the field
> + names available in the type of the left-hand-side:
> + 
> + @smallexample
> + (@value{GDBP}) p gdb_stdout.@key{M-?}
> + magic      to_delete  to_fputs   to_put     to_rewind  
> + to_data    to_flush   to_isatty  to_read    to_write   
> + @end smallexample
> + 

  Otherwise, 

This is good.  I have three comments:

 . in the @cindex entries, I think it's better to use "structure
   fields" instead of just "fields", at least in one of them;

 . please use @kbd{M-?}, not @key (there's no key labeled literally
   with M-?);

 . not every GDB user knows by heart the definition of the ui_file
   structure, so I'd add the following passage right after your text:

 @noindent
 This is because the @code{gdb_stdout} is a variable of the type
 @code{struct ui_file} that is defined in @value{GDBN} sources as
 follows:

 @smallexample
 struct ui_file
 @{
    int *magic;
    ui_file_flush_ftype *to_flush;
    ui_file_write_ftype *to_write;
    ui_file_fputs_ftype *to_fputs;
    ui_file_read_ftype *to_read;
    ui_file_delete_ftype *to_delete;
    ui_file_isatty_ftype *to_isatty;
    ui_file_rewind_ftype *to_rewind;
    ui_file_put_ftype *to_put;
    void *to_data;
 @}
 @end smallexample

Re: [RFA] Patch to limit field name completion candidates

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Date: Thu, 05 Jun 2008 20:41:21 -0600
> 
> >>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:
> 
> Daniel> You don't recurse into base classes... for C++ this is
> Daniel> obviously a substantial problem.
> 
> Thanks -- I will look at this.
> 
> Daniel> Then there's the additional question of dynamic type.
> 
> I did think about this one, but forgot to mention it.
> 
> I don't see a good way to make it work in general.  It could be made
> to work sometimes, but not always.  In particular I think we could add
> a "don't evaluate side effects" mode to expression evaluation -- this
> would do an ok job except for the case of an expression involving an
> inferior function call.  In that case I think we could still only use
> the static type.

These limitations should be mentioned in the manual, at least as a
@footnote, I think.

Re: [RFA] Patch to limit field name completion candidates

[Eli Zaretskii]

> Date: Fri, 06 Jun 2008 13:12:02 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: gdb-patches@sources.redhat.com
> 
>   Otherwise, 

Please disregard this "Otherwise" thing, which I forgot to delete
before sending.  The rest of my text is correct.

Sorry.

Re: [RFA] Patch to limit field name completion candidates

[Tom Tromey]

Eli> These limitations should be mentioned in the manual, at least as a
Eli> @footnote, I think.

Good idea.

Here is an updated patch.  I've updated the NEWS and manual bits -- I
used "struct or union" and made the index entries refer to both struct
and union.

Also, this changes the patch to search base classes for field names.

Tom

ChangeLog:
2008-06-06  Tom Tromey  <tromey@redhat.com>

	* value.h (evaluate_subexpression_type, extract_field_op):
	Declare.
	* printcmd.c (_initialize_printcmd): Use expression_completer for
	'p', 'inspect', 'call'.
	* parser-defs.h (parse_field_expression): Declare.
	* parse.c: Include exceptions.h.
	(in_parse_field, expout_last_struct): New globals.
	(mark_struct_expression): New function.
	(prefixify_expression): Return int.
	(prefixify_subexp): Return int.  Use expout_last_struct.
	(parse_exp_1): Update.
	(parse_exp_in_context): Add 'out_subexp' argument.  Handle
	in_parse_field.
	(parse_field_expression): New function.
	* expression.h (parse_field_expression): Declare.
	(in_parse_field): Likewise.
	* eval.c (evaluate_subexpression_type): New function.
	(extract_field_op): Likewise.
	* completer.h (expression_completer): Declare.
	* completer.c (expression_completer): New function.
	(count_struct_fields, add_struct_fields): New functions.
	* c-exp.y (yyparse): Redefine.
	(COMPLETE): New token.
	(exp): New productions.
	(saw_name_at_eof, last_was_structop): New globals.
	(yylex): Return COMPLETE when needed.  Recognize in_parse_field.
	(c_parse): New function.
	* breakpoint.c (_initialize_breakpoint): Use expression_completer
	for watch, awatch, and rwatch.
	* Makefile.in (parse.o): Depend on exceptions_h.

doc/ChangeLog:
2008-06-06  Tom Tromey  <tromey@redhat.com>

	* gdb.texinfo (Completion): Add field name example.

testsuite/ChangeLog:
2008-06-06  Tom Tromey  <tromey@redhat.com>

	* gdb.base/break1.c (struct some_struct): New struct.
	(values): New global.
	* gdb.base/completion.exp: Add field name completion test.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.1018
diff -u -r1.1018 Makefile.in
--- Makefile.in	9 May 2008 17:02:01 -0000	1.1018
+++ Makefile.in	6 Jun 2008 17:39:42 -0000
@@ -2591,7 +2591,8 @@
 parse.o: parse.c $(defs_h) $(gdb_string_h) $(symtab_h) $(gdbtypes_h) \
 	$(frame_h) $(expression_h) $(value_h) $(command_h) $(language_h) \
 	$(f_lang_h) $(parser_defs_h) $(gdbcmd_h) $(symfile_h) $(inferior_h) \
-	$(doublest_h) $(gdb_assert_h) $(block_h) $(source_h) $(objfiles_h)
+	$(doublest_h) $(gdb_assert_h) $(block_h) $(source_h) $(objfiles_h) \
+	$(exceptions_h)
 p-exp.o: p-exp.c $(defs_h) $(gdb_string_h) $(expression_h) $(value_h) \
 	$(parser_defs_h) $(language_h) $(p_lang_h) $(bfd_h) $(symfile_h) \
 	$(objfiles_h) $(block_h)
Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.273
diff -u -r1.273 NEWS
--- NEWS	9 May 2008 19:17:46 -0000	1.273
+++ NEWS	6 Jun 2008 17:39:42 -0000
@@ -3,6 +3,17 @@
 
 *** Changes since GDB 6.8
 
+* When completing in expressions, gdb will attempt to limit
+completions to allowable structure or union fields, where appropriate.
+For instance, consider:
+
+    # struct example { int f1; double f2; };
+    # struct example variable;
+    (gdb) p variable.
+
+If the user types TAB at the end of this command line, the available
+completions will be "f1" and "f2".
+
 * New remote packets
 
 qSearch:memory:
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.321
diff -u -r1.321 breakpoint.c
--- breakpoint.c	4 May 2008 19:38:59 -0000	1.321
+++ breakpoint.c	6 Jun 2008 17:39:43 -0000
@@ -8433,19 +8433,19 @@
 Set a watchpoint for an expression.\n\
 A watchpoint stops execution of your program whenever the value of\n\
 an expression changes."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
 
   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."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
 
   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."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
 
   add_info ("watchpoints", breakpoints_info,
 	    _("Synonym for ``info breakpoints''."));
Index: c-exp.y
===================================================================
RCS file: /cvs/src/src/gdb/c-exp.y,v
retrieving revision 1.42
diff -u -r1.42 c-exp.y
--- c-exp.y	9 Jan 2008 19:27:15 -0000	1.42
+++ c-exp.y	6 Jun 2008 17:39:43 -0000
@@ -63,7 +63,7 @@
    generators need to be fixed instead of adding those names to this list. */
 
 #define	yymaxdepth c_maxdepth
-#define	yyparse	c_parse
+#define	yyparse	c_parse_internal
 #define	yylex	c_lex
 #define	yyerror	c_error
 #define	yylval	c_lval
@@ -180,6 +180,7 @@
 
 %token <sval> STRING
 %token <ssym> NAME /* BLOCKNAME defined below to give it higher precedence. */
+%token <voidval> COMPLETE
 %token <tsym> TYPENAME
 %type <sval> name
 %type <ssym> name_not_typename
@@ -301,6 +302,23 @@
 			  write_exp_elt_opcode (STRUCTOP_PTR); }
 	;
 
+exp	:	exp ARROW name COMPLETE
+			{ mark_struct_expression ();
+			  write_exp_elt_opcode (STRUCTOP_PTR);
+			  write_exp_string ($3);
+			  write_exp_elt_opcode (STRUCTOP_PTR); }
+	;
+
+exp	:	exp ARROW COMPLETE
+			{ struct stoken s;
+			  mark_struct_expression ();
+			  write_exp_elt_opcode (STRUCTOP_PTR);
+			  s.ptr = "";
+			  s.length = 0;
+			  write_exp_string (s);
+			  write_exp_elt_opcode (STRUCTOP_PTR); }
+	;
+
 exp	:	exp ARROW qualified_name
 			{ /* exp->type::name becomes exp->*(&type::name) */
 			  /* Note: this doesn't work if name is a
@@ -319,6 +337,23 @@
 			  write_exp_elt_opcode (STRUCTOP_STRUCT); }
 	;
 
+exp	:	exp '.' name COMPLETE
+			{ mark_struct_expression ();
+			  write_exp_elt_opcode (STRUCTOP_STRUCT);
+			  write_exp_string ($3);
+			  write_exp_elt_opcode (STRUCTOP_STRUCT); }
+	;
+
+exp	:	exp '.' COMPLETE
+			{ struct stoken s;
+			  mark_struct_expression ();
+			  write_exp_elt_opcode (STRUCTOP_STRUCT);
+			  s.ptr = "";
+			  s.length = 0;
+			  write_exp_string (s);
+			  write_exp_elt_opcode (STRUCTOP_STRUCT); }
+	;
+
 exp	:	exp '.' qualified_name
 			{ /* exp.type::name becomes exp.*(&type::name) */
 			  /* Note: this doesn't work if name is a
@@ -1341,6 +1376,16 @@
     {">=", GEQ, BINOP_END}
   };
 
+/* This is set if a NAME token appeared at the very end of the input
+   string, with no whitespace separating the name from the EOF.  This
+   is used only when parsing to do field name completion.  */
+static int saw_name_at_eof;
+
+/* This is set if the previously-returned token was a structure
+   operator -- either '.' or ARROW.  This is used only when parsing to
+   do field name completion.  */
+static int last_was_structop;
+
 /* Read one token, getting characters through lexptr.  */
 
 static int
@@ -1356,7 +1401,10 @@
   static int tempbufsize;
   char * token_string = NULL;
   int class_prefix = 0;
-   
+  int saw_structop = last_was_structop;
+
+  last_was_structop = 0;
+
  retry:
 
   /* Check if this is a macro invocation that we need to expand.  */
@@ -1388,6 +1436,8 @@
       {
 	lexptr += 2;
 	yylval.opcode = tokentab2[i].opcode;
+	if (in_parse_field && tokentab2[i].opcode == ARROW)
+	  last_was_structop = 1;
 	return tokentab2[i].token;
       }
 
@@ -1396,6 +1446,8 @@
     case 0:
       /* If we were just scanning the result of a macro expansion,
          then we need to resume scanning the original text.
+	 If we're parsing for field name completion, and the previous
+	 token allows such completion, return a COMPLETE token.
          Otherwise, we were already scanning the original text, and
          we're really done.  */
       if (scanning_macro_expansion ())
@@ -1403,6 +1455,13 @@
           finished_macro_expansion ();
           goto retry;
         }
+      else if (saw_name_at_eof)
+	{
+	  saw_name_at_eof = 0;
+	  return COMPLETE;
+	}
+      else if (saw_structop)
+	return COMPLETE;
       else
         return 0;
 
@@ -1475,7 +1534,11 @@
     case '.':
       /* Might be a floating point number.  */
       if (lexptr[1] < '0' || lexptr[1] > '9')
-	goto symbol;		/* Nope, must be a symbol. */
+	{
+	  if (in_parse_field)
+	    last_was_structop = 1;
+	  goto symbol;		/* Nope, must be a symbol. */
+	}
       /* FALL THRU into number case.  */
 
     case '0':
@@ -1812,10 +1875,20 @@
     /* Any other kind of symbol */
     yylval.ssym.sym = sym;
     yylval.ssym.is_a_field_of_this = is_a_field_of_this;
+    if (in_parse_field && *lexptr == '\0')
+      saw_name_at_eof = 1;
     return NAME;
   }
 }
 
+int
+c_parse (void)
+{
+  last_was_structop = 0;
+  saw_name_at_eof = 0;
+  return yyparse ();
+}
+
 void
 yyerror (msg)
      char *msg;
Index: completer.c
===================================================================
RCS file: /cvs/src/src/gdb/completer.c,v
retrieving revision 1.24
diff -u -r1.24 completer.c
--- completer.c	1 Jan 2008 22:53:09 -0000	1.24
+++ completer.c	6 Jun 2008 17:39:44 -0000
@@ -338,6 +338,90 @@
   return list;
 }
 
+/* Helper for expression_completer which recursively counts the number
+   of named fields in a structure or union type.  */
+static int
+count_struct_fields (struct type *type)
+{
+  int i, result = 0;
+
+  CHECK_TYPEDEF (type);
+  for (i = 0; i < TYPE_NFIELDS (type); ++i)
+    {
+      if (i < TYPE_N_BASECLASSES (type))
+	result += count_struct_fields (TYPE_BASECLASS (type, i));
+      else if (TYPE_FIELD_NAME (type, i))
+	++result;
+    }
+  return result;
+}
+
+/* Helper for expression_completer which recursively adds field names
+   from TYPE, a struct or union type, to the array OUTPUT.  This
+   function assumes that OUTPUT is correctly-sized.  */
+static void
+add_struct_fields (struct type *type, int *nextp, char **output,
+		   char *fieldname, int namelen)
+{
+  int i;
+
+  CHECK_TYPEDEF (type);
+  for (i = 0; i < TYPE_NFIELDS (type); ++i)
+    {
+      if (i < TYPE_N_BASECLASSES (type))
+	add_struct_fields (TYPE_BASECLASS (type, i), nextp, output,
+			   fieldname, namelen);
+      else if (TYPE_FIELD_NAME (type, i)
+	       && ! strncmp (TYPE_FIELD_NAME (type, i), fieldname, namelen))
+	{
+	  output[*nextp] = xstrdup (TYPE_FIELD_NAME (type, i));
+	  ++*nextp;
+	}
+    }
+}
+
+/* Complete on expressions.  Often this means completing on symbol
+   names, but some language parsers also have support for completing
+   field names.  */
+char **
+expression_completer (char *text, char *word)
+{
+  struct type *type;
+  char *fieldname;
+
+  /* Perform a tentative parse of the expression, to see whether a
+     field completion is required.  */
+  fieldname = NULL;
+  type = parse_field_expression (text, &fieldname);
+  if (fieldname && type)
+    {
+      for (;;)
+	{
+	  CHECK_TYPEDEF (type);
+	  if (TYPE_CODE (type) != TYPE_CODE_PTR
+	      && TYPE_CODE (type) != TYPE_CODE_REF)
+	    break;
+	  type = TYPE_TARGET_TYPE (type);
+	}
+
+      if (TYPE_CODE (type) == TYPE_CODE_UNION
+	  || TYPE_CODE (type) == TYPE_CODE_STRUCT)
+	{
+	  int alloc = count_struct_fields (type);
+	  int flen = strlen (fieldname);
+	  int out = 0;
+	  char **result = (char **) xmalloc ((alloc + 1) * sizeof (char *));
+
+	  add_struct_fields (type, &out, result, fieldname, flen);
+	  result[out] = NULL;
+	  return result;
+	}
+    }
+
+  /* Not ideal but it is what we used to do before... */
+  return location_completer (text, word);
+}
+
 /* Complete on command names.  Used by "help".  */
 char **
 command_completer (char *text, char *word)
@@ -520,7 +604,8 @@
 		      rl_completer_word_break_characters =
 			gdb_completer_file_name_break_characters;
 		    }
-		  else if (c->completer == location_completer)
+		  else if (c->completer == location_completer
+			   || c->completer == expression_completer)
 		    {
 		      /* Commands which complete on locations want to
 			 see the entire argument.  */
@@ -588,7 +673,8 @@
 		  rl_completer_word_break_characters =
 		    gdb_completer_file_name_break_characters;
 		}
-	      else if (c->completer == location_completer)
+	      else if (c->completer == location_completer
+		       || c->completer == expression_completer)
 		{
 		  for (p = word;
 		       p > tmp_command
Index: completer.h
===================================================================
RCS file: /cvs/src/src/gdb/completer.h,v
retrieving revision 1.13
diff -u -r1.13 completer.h
--- completer.h	1 Jan 2008 22:53:09 -0000	1.13
+++ completer.h	6 Jun 2008 17:39:44 -0000
@@ -25,6 +25,8 @@
 
 extern char **filename_completer (char *, char *);
 
+extern char **expression_completer (char *, char *);
+
 extern char **location_completer (char *, char *);
 
 extern char **command_completer (char *, char *);
Index: eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.83
diff -u -r1.83 eval.c
--- eval.c	22 Apr 2008 11:03:41 -0000	1.83
+++ eval.c	6 Jun 2008 17:39:44 -0000
@@ -175,6 +175,36 @@
   return evaluate_subexp (NULL_TYPE, exp, &pc, EVAL_AVOID_SIDE_EFFECTS);
 }
 
+/* Evaluate a subexpression, avoiding all memory references and
+   getting a value whose type alone is correct.  */
+
+struct value *
+evaluate_subexpression_type (struct expression *exp, int subexp)
+{
+  return evaluate_subexp (NULL_TYPE, exp, &subexp, EVAL_AVOID_SIDE_EFFECTS);
+}
+
+/* Extract a field operation from an expression.  If the subexpression
+   of EXP starting at *SUBEXP is not a structure dereference
+   operation, return NULL.  Otherwise, return the name of the
+   dereferenced field, and advance *SUBEXP to point to the
+   subexpression of the left-hand-side of the dereference.  This is
+   used when completing field names.  */
+
+char *
+extract_field_op (struct expression *exp, int *subexp)
+{
+  int tem;
+  char *result;
+  if (exp->elts[*subexp].opcode != STRUCTOP_STRUCT
+      && exp->elts[*subexp].opcode != STRUCTOP_PTR)
+    return NULL;
+  tem = longest_to_int (exp->elts[*subexp + 1].longconst);
+  result = &exp->elts[*subexp + 2].string;
+  (*subexp) += 1 + 3 + BYTES_TO_EXP_ELEM (tem + 1);
+  return result;
+}
+
 /* If the next expression is an OP_LABELED, skips past it,
    returning the label.  Otherwise, does nothing and returns NULL. */
 
Index: expression.h
===================================================================
RCS file: /cvs/src/src/gdb/expression.h,v
retrieving revision 1.26
diff -u -r1.26 expression.h
--- expression.h	1 Jan 2008 22:53:09 -0000	1.26
+++ expression.h	6 Jun 2008 17:39:44 -0000
@@ -389,8 +389,14 @@
 
 extern struct expression *parse_expression (char *);
 
+extern struct type *parse_field_expression (char *, char **);
+
 extern struct expression *parse_exp_1 (char **, struct block *, int);
 
+/* For use by parsers; set if we want to parse an expression and
+   attempt to complete a field name.  */
+extern int in_parse_field;
+
 /* The innermost context required by the stack and register variables
    we've encountered so far.  To use this, set it to NULL, then call
    parse_<whatever>, then look at it.  */
Index: parse.c
===================================================================
RCS file: /cvs/src/src/gdb/parse.c,v
retrieving revision 1.71
diff -u -r1.71 parse.c
--- parse.c	3 May 2008 22:30:51 -0000	1.71
+++ parse.c	6 Jun 2008 17:39:44 -0000
@@ -52,6 +52,7 @@
 #include "block.h"
 #include "source.h"
 #include "objfiles.h"
+#include "exceptions.h"
 
 /* Standard set of definitions for printing, dumping, prefixifying,
  * and evaluating expressions.  */
@@ -80,6 +81,15 @@
 int paren_depth;
 int comma_terminates;
 
+/* True if parsing an expression to find a field reference.  This is
+   only used by completion.  */
+int in_parse_field;
+
+/* The index of the last struct expression directly before a '.' or
+   '->'.  This is set when parsing and is only used when completing a
+   field name.  It is -1 if no dereference operation was found.  */
+static int expout_last_struct = -1;
+
 /* A temporary buffer for identifiers, so we can null-terminate them.
 
    We allocate this with xrealloc.  parse_exp_1 used to allocate with
@@ -100,13 +110,13 @@
 
 static void free_funcalls (void *ignore);
 
-static void prefixify_expression (struct expression *);
+static int prefixify_expression (struct expression *);
 
-static void prefixify_subexp (struct expression *, struct expression *, int,
-			      int);
+static int prefixify_subexp (struct expression *, struct expression *, int,
+			     int);
 
 static struct expression *parse_exp_in_context (char **, struct block *, int, 
-						int);
+						int, int *);
 
 void _initialize_parse (void);
 
@@ -460,6 +470,16 @@
     }
   write_exp_elt_opcode (UNOP_MEMVAL);
 }
+
+/* Mark the current index as the starting location of a structure
+   expression.  This is used when completing on field names.  */
+
+void
+mark_struct_expression (void)
+{
+  expout_last_struct = expout_ptr;
+}
+
 \f
 /* Recognize tokens that start with '$'.  These include:
 
@@ -664,9 +684,13 @@
 }
 \f
 /* Reverse an expression from suffix form (in which it is constructed)
-   to prefix form (in which we can conveniently print or execute it).  */
+   to prefix form (in which we can conveniently print or execute it).
+   Ordinarily this always returns -1.  However, if EXPOUT_LAST_STRUCT
+   is not -1 (i.e., we are trying to complete a field name), it will
+   return the index of the subexpression which is the left-hand-side
+   of the struct operation at EXPOUT_LAST_STRUCT.  */
 
-static void
+static int
 prefixify_expression (struct expression *expr)
 {
   int len = sizeof (struct expression) + EXP_ELEM_TO_BYTES (expr->nelts);
@@ -678,7 +702,7 @@
   /* Copy the original expression into temp.  */
   memcpy (temp, expr, len);
 
-  prefixify_subexp (temp, expr, inpos, outpos);
+  return prefixify_subexp (temp, expr, inpos, outpos);
 }
 
 /* Return the number of exp_elements in the postfix subexpression 
@@ -875,9 +899,12 @@
 
 /* Copy the subexpression ending just before index INEND in INEXPR
    into OUTEXPR, starting at index OUTBEG.
-   In the process, convert it from suffix to prefix form.  */
+   In the process, convert it from suffix to prefix form.
+   If EXPOUT_LAST_STRUCT is -1, then this function always returns -1.
+   Otherwise, it returns the index of the subexpression which is the
+   left-hand-side of the expression at EXPOUT_LAST_STRUCT.  */
 
-static void
+static int
 prefixify_subexp (struct expression *inexpr,
 		  struct expression *outexpr, int inend, int outbeg)
 {
@@ -886,6 +913,7 @@
   int i;
   int *arglens;
   enum exp_opcode opcode;
+  int result = -1;
 
   operator_length (inexpr, inend, &oplen, &args);
 
@@ -896,6 +924,9 @@
 	  EXP_ELEM_TO_BYTES (oplen));
   outbeg += oplen;
 
+  if (expout_last_struct == inend)
+    result = outbeg - oplen;
+
   /* Find the lengths of the arg subexpressions.  */
   arglens = (int *) alloca (args * sizeof (int));
   for (i = args - 1; i >= 0; i--)
@@ -913,11 +944,21 @@
      outbeg does similarly in the output.  */
   for (i = 0; i < args; i++)
     {
+      int r;
       oplen = arglens[i];
       inend += oplen;
-      prefixify_subexp (inexpr, outexpr, inend, outbeg);
+      r = prefixify_subexp (inexpr, outexpr, inend, outbeg);
+      if (r != -1)
+	{
+	  /* Return immediately.  We probably have only parsed a
+	     partial expression, so we don't want to try to reverse
+	     the other operands.  */
+	  return r;
+	}
       outbeg += oplen;
     }
+
+  return result;
 }
 \f
 /* This page contains the two entry points to this file.  */
@@ -935,23 +976,30 @@
 struct expression *
 parse_exp_1 (char **stringptr, struct block *block, int comma)
 {
-  return parse_exp_in_context (stringptr, block, comma, 0);
+  return parse_exp_in_context (stringptr, block, comma, 0, NULL);
 }
 
 /* As for parse_exp_1, except that if VOID_CONTEXT_P, then
-   no value is expected from the expression.  */
+   no value is expected from the expression.
+   OUT_SUBEXP is set when attempting to complete a field name; in this
+   case it is set to the index of the subexpression on the
+   left-hand-side of the struct op.  If not doing such completion, it
+   is left untouched.  */
 
 static struct expression *
 parse_exp_in_context (char **stringptr, struct block *block, int comma, 
-		      int void_context_p)
+		      int void_context_p, int *out_subexp)
 {
+  volatile struct gdb_exception except;
   struct cleanup *old_chain;
+  int subexp;
 
   lexptr = *stringptr;
   prev_lexptr = NULL;
 
   paren_depth = 0;
   type_stack_depth = 0;
+  expout_last_struct = -1;
 
   comma_terminates = comma;
 
@@ -986,10 +1034,20 @@
   expout = (struct expression *)
     xmalloc (sizeof (struct expression) + EXP_ELEM_TO_BYTES (expout_size));
   expout->language_defn = current_language;
-  make_cleanup (free_current_contents, &expout);
 
-  if (current_language->la_parser ())
-    current_language->la_error (NULL);
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      if (current_language->la_parser ())
+	current_language->la_error (NULL);
+    }
+  if (except.reason < 0)
+    {
+      if (! in_parse_field)
+	{
+	  xfree (expout);
+	  throw_exception (except);
+	}
+    }
 
   discard_cleanups (old_chain);
 
@@ -1009,7 +1067,9 @@
     dump_raw_expression (expout, gdb_stdlog,
 			 "before conversion to prefix form");
 
-  prefixify_expression (expout);
+  subexp = prefixify_expression (expout);
+  if (out_subexp)
+    *out_subexp = subexp;
 
   current_language->la_post_parser (&expout, void_context_p);
 
@@ -1033,6 +1093,45 @@
   return exp;
 }
 
+/* Parse STRING as an expression.  If parsing ends in the middle of a
+   field reference, return the type of the left-hand-side of the
+   reference; furthermore, if the parsing ends in the field name,
+   return the field name in *NAME.  In all other cases, return NULL.  */
+
+struct type *
+parse_field_expression (char *string, char **name)
+{
+  struct expression *exp = NULL;
+  struct value *val;
+  int subexp;
+  volatile struct gdb_exception except;
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      in_parse_field = 1;
+      exp = parse_exp_in_context (&string, 0, 0, 0, &subexp);
+    }
+  in_parse_field = 0;
+  if (except.reason < 0 || ! exp)
+    return NULL;
+  if (expout_last_struct == -1)
+    {
+      xfree (exp);
+      return NULL;
+    }
+
+  *name = extract_field_op (exp, &subexp);
+  if (!*name)
+    {
+      xfree (exp);
+      return NULL;
+    }
+  val = evaluate_subexpression_type (exp, subexp);
+  xfree (exp);
+
+  return value_type (val);
+}
+
 /* A post-parser that does nothing */
 
 void
Index: parser-defs.h
===================================================================
RCS file: /cvs/src/src/gdb/parser-defs.h,v
retrieving revision 1.26
diff -u -r1.26 parser-defs.h
--- parser-defs.h	1 Jan 2008 22:53:12 -0000	1.26
+++ parser-defs.h	6 Jun 2008 17:39:44 -0000
@@ -138,6 +138,8 @@
 
 extern void write_dollar_variable (struct stoken str);
 
+extern void mark_struct_expression (void);
+
 extern char *find_template_name_end (char *);
 
 extern void start_arglist (void);
Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.119
diff -u -r1.119 printcmd.c
--- printcmd.c	6 May 2008 21:34:59 -0000	1.119
+++ printcmd.c	6 Jun 2008 17:39:44 -0000
@@ -2420,7 +2420,7 @@
 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."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
 
   add_cmd ("variable", class_vars, set_command, _("\
 Evaluate expression EXP and assign result to variable VAR, using assignment\n\
@@ -2453,13 +2453,13 @@
 \n\
 EXP may be preceded with /FMT, where FMT is a format letter\n\
 but no count or size letter (see \"x\" command)."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
   add_com_alias ("p", "print", class_vars, 1);
 
   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."));
-  set_cmd_completer (c, location_completer);
+  set_cmd_completer (c, expression_completer);
 
   add_setshow_uinteger_cmd ("max-symbolic-offset", no_class,
 			    &max_symbolic_offset, _("\
Index: value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.114
diff -u -r1.114 value.h
--- value.h	6 May 2008 21:34:59 -0000	1.114
+++ value.h	6 Jun 2008 17:39:44 -0000
@@ -420,6 +420,11 @@
 
 extern struct value *evaluate_type (struct expression *exp);
 
+extern struct value *evaluate_subexpression_type (struct expression *exp,
+						  int subexp);
+
+extern char *extract_field_op (struct expression *exp, int *subexp);
+
 extern struct value *evaluate_subexp_with_coercion (struct expression *,
 						    int *, enum noside);
 
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.495
diff -u -r1.495 gdb.texinfo
--- doc/gdb.texinfo	9 May 2008 17:02:01 -0000	1.495
+++ doc/gdb.texinfo	6 Jun 2008 17:39:47 -0000
@@ -1,6 +1,6 @@
 \input texinfo      @c -*-texinfo-*-
 @c Copyright (C) 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1998,
-@c 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006
+@c 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2008
 @c Free Software Foundation, Inc.
 @c
 @c %**start of header
@@ -1521,6 +1521,44 @@
 overload-resolution off} to disable overload resolution;
 see @ref{Debugging C Plus Plus, ,@value{GDBN} Features for C@t{++}}.
 
+@cindex completion of structure field names
+@cindex structure field name completion
+@cindex completion of union field names
+@cindex union field name completion
+When completing in an expression which looks up a field in a
+structure, @value{GDBN} also tries@footnote{The completer can be
+confused by certain kinds of invalid expressions.  Also, it only
+examines the static type of the expression, not the dynamic type.} to
+limit completions to the field names available in the type of the
+left-hand-side:
+
+@smallexample
+(@value{GDBP}) p gdb_stdout.@kbd{M-?}
+magic      to_delete  to_fputs   to_put     to_rewind  
+to_data    to_flush   to_isatty  to_read    to_write   
+@end smallexample
+
+@noindent
+This is because the @code{gdb_stdout} is a variable of the type
+@code{struct ui_file} that is defined in @value{GDBN} sources as
+follows:
+
+@smallexample
+struct ui_file
+@{
+   int *magic;
+   ui_file_flush_ftype *to_flush;
+   ui_file_write_ftype *to_write;
+   ui_file_fputs_ftype *to_fputs;
+   ui_file_read_ftype *to_read;
+   ui_file_delete_ftype *to_delete;
+   ui_file_isatty_ftype *to_isatty;
+   ui_file_rewind_ftype *to_rewind;
+   ui_file_put_ftype *to_put;
+   void *to_data;
+@}
+@end smallexample
+
 
 @node Help
 @section Getting Help
Index: testsuite/gdb.base/break1.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/break1.c,v
retrieving revision 1.4
diff -u -r1.4 break1.c
--- testsuite/gdb.base/break1.c	1 Jan 2008 22:53:18 -0000	1.4
+++ testsuite/gdb.base/break1.c	6 Jun 2008 17:39:49 -0000
@@ -41,3 +41,12 @@
 void marker3 (a, b) char *a, *b; {}	/* set breakpoint 18 here */
 void marker4 (d) long d; {}		/* set breakpoint 13 here */
 #endif
+
+/* A structure we use for field name completion tests.  */
+struct some_struct
+{
+  int a_field;
+  int b_field;
+};
+
+struct some_struct values[50];
Index: testsuite/gdb.base/completion.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/completion.exp,v
retrieving revision 1.29
diff -u -r1.29 completion.exp
--- testsuite/gdb.base/completion.exp	2 May 2008 20:30:49 -0000	1.29
+++ testsuite/gdb.base/completion.exp	6 Jun 2008 17:39:49 -0000
@@ -637,6 +637,22 @@
     timeout { fail "(timeout) complete (2) 'p no_var_named_this-'" }
 }
 
+send_gdb "p values\[0\].a\t"
+sleep 3
+gdb_expect  {
+        -re "^p values.0..a_field $"\
+            { send_gdb "\n"
+	      sleep 1
+              gdb_expect {
+                      -re "^.* = 0.*$gdb_prompt $"\
+                                        { pass "complete 'p values\[0\].a'"}
+                      -re ".*$gdb_prompt $" { fail "complete 'p values\[0\].a'"}
+                      timeout           {fail "(timeout) complete 'p values\[0\].a'"}
+                     }
+            }
+        -re ".*$gdb_prompt $"       { fail "complete 'p values\[0\].a'" }
+        timeout         { fail "(timeout) complete 'p values\[0\].a' 2" }
+        }
 
 # The following tests used to simply try to complete `${objdir}/file',
 # and so on.  The problem is that ${objdir} can be very long; the

Re: [RFA] Patch to limit field name completion candidates

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: Tom Tromey <tromey@redhat.com>
> Date: Fri, 06 Jun 2008 11:50:10 -0600
> 
> Eli> These limitations should be mentioned in the manual, at least as a
> Eli> @footnote, I think.
> 
> Good idea.
> 
> Here is an updated patch.  I've updated the NEWS and manual bits -- I
> used "struct or union" and made the index entries refer to both struct
> and union.
> 
> Also, this changes the patch to search base classes for field names.

Thanks, I'm happy now.

Re: [RFA] Patch to limit field name completion candidates

[Daniel Jacobowitz]

On Fri, Jun 06, 2008 at 10:44:41PM +0300, Eli Zaretskii wrote:
> Thanks, I'm happy now.

Me, too.

-- 
Daniel Jacobowitz
CodeSourcery