Re: [PATCH] Add autocompletion for convenience vars in print and set

[Daniel Gutson <daniel dot gutson at tallertechnologies dot com>]

Second version.
Comments below:

On Tue, May 20, 2014 at 1:36 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Daniel" == Daniel Gutson <daniel.gutson@tallertechnologies.com> writes:
>
> Daniel> I could not find any testsuite where to add tests for this; if
> Daniel> there are, please let me know.
>
> See testsuite/gdb.base/completion.exp

Thanks, I ran all the tests and passed. I did not add a new test
case though since I didn't check how to add a new convenience var
and undefine it later from the test framework. (Should I try harder?)

>
> Daniel>  exp    :       VARIABLE
> Daniel>                         {
> Daniel> -                         write_dollar_variable (pstate, $1);
> Daniel> +                         if (!parse_completion)
> Daniel> +                           write_dollar_variable (pstate, $1);
> Daniel>                         }
>
> I think this isn't correct.  I think it won't work if you try to
> complete on a field name where the "LHS" has a convenience variable.
> That is something like:
>
>    set $var = (struct x *) malloc (...)
>    complete print $var.somethin

You were right, that was broken.

>
> Instead I think you need a new production, like "exp : VARIABLE COMPLETE".

I found another solution without touching the yacc file: adding a check
inside write_dollar_variable

>
> Daniel> +  if (p != NULL && *p == '$')
> Daniel> +    return complete_internalvar (p + 1);
>
> I'm not sure this is correct either, but offhand I don't know.
> Should it not look at "word"?

It actually works ("word" didn't have the $).

However, I noticed that 'set' cmd autocomplete doesn't work for
members of structures (neither with convenience vars nor regular
symbols). I'll address that in the next patch once this gets approved
and committed.

>
> Daniel> +  VEC (char_ptr) * ret = current_language->la_make_symbol_completion_list (
> Daniel> +                                                      text, word,
> Daniel> +                                                      TYPE_CODE_UNDEF);
>
> No space before "ret".
> The line breaks look odd, I would break before the "=" and not after the "(".
>
> thanks,
> Tom

2014-05-22  Daniel Gutson  <daniel.gutson@tallertechnologies.com>

gdb/
        * parse.c (write_dollar_variable): Do not create an internal
var when completing.
        * completer.c (expression_completer): Call complete_internalvar.
        * symtab.c (make_symbol_completion_list): Call complete_internalvar.

diff --git a/gdb/completer.c b/gdb/completer.c
index 94f70a9..6c5cdf8 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -20,6 +20,7 @@
 #include "symtab.h"
 #include "gdbtypes.h"
 #include "expression.h"
+#include "value.h"
 #include "filenames.h"		/* For DOSish file names.  */
 #include "language.h"
 #include "gdb_assert.h"
@@ -446,8 +447,11 @@ expression_completer (struct cmd_list_element *ignore,
        p--)
     ;
 
-  /* Not ideal but it is what we used to do before...  */
-  return location_completer (ignore, p, word);
+  if (p != NULL && *p == '$')
+    return complete_internalvar (p + 1);
+  else
+    /* Not ideal but it is what we used to do before...  */
+    return location_completer (ignore, p, word);
 }
 
 /* Here are some useful test cases for completion.  FIXME: These
diff --git a/gdb/parse.c b/gdb/parse.c
index 105d0cd..218be13 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -692,10 +692,12 @@ write_dollar_variable (struct parser_state *ps, struct stoken str)
     }
 
   /* Any other names are assumed to be debugger internal variables.  */
-
-  write_exp_elt_opcode (ps, OP_INTERNALVAR);
-  write_exp_elt_intern (ps, create_internalvar (copy_name (str) + 1));
-  write_exp_elt_opcode (ps, OP_INTERNALVAR);
+  if (!parse_completion)
+    {
+      write_exp_elt_opcode (ps, OP_INTERNALVAR);
+      write_exp_elt_intern (ps, create_internalvar (copy_name (str) + 1));
+      write_exp_elt_opcode (ps, OP_INTERNALVAR);
+    }
   return;
 handle_last:
   write_exp_elt_opcode (ps, OP_LAST);
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 15ac3d1..52eb64c 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4542,14 +4542,21 @@ default_make_symbol_completion_list (const char *text, const char *word,
 }
 
 /* Return a vector of all symbols (regardless of class) which begin by
-   matching TEXT.  If the answer is no symbols, then the return value
-   is NULL.  */
+   matching TEXT.  If the answer is no symbols, then check whether the
+   text is an internal var ($foo), if so, return what complete_internalvar
+   returns; otherwise the return value is NULL.  */
 
 VEC (char_ptr) *
 make_symbol_completion_list (const char *text, const char *word)
 {
-  return current_language->la_make_symbol_completion_list (text, word,
-							   TYPE_CODE_UNDEF);
+  VEC (char_ptr) *ret
+    = current_language->la_make_symbol_completion_list (text, word,
+                                                      TYPE_CODE_UNDEF);
+
+  if (ret == NULL && *text == '$')
+    ret = complete_internalvar (text + 1);
+
+  return ret;
 }
 
 /* Like make_symbol_completion_list, but only return STRUCT_DOMAIN