[06/40] Expression completer should not match explicit location options

  Currently, the expression completer matches explicit location options,
which would only make sense for commands that work with linespecs, not
expressions.

I.e., currently, this:
 "p -functi"

Completes to:
 "p -function "

This patch fixes that, and adds regression tests.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* completer.c (expression_completer): Call
	linespec_location_completer instead of location_completer.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/printcmds.exp: Add tests.
---
 gdb/completer.c                      | 2 +-
 gdb/testsuite/gdb.base/printcmds.exp | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)
  

Pedro Alves <palves@redhat.com> writes:

> Currently, the expression completer matches explicit location options,
> which would only make sense for commands that work with linespecs, not
> expressions.
>
> I.e., currently, this:
>  "p -functi"
>
> Completes to:
>  "p -function "

I don't know much about completer, expression and explicit location, so
I don't know what do you fix here, anything wrong in the completion?

>
> This patch fixes that, and adds regression tests.
  

On 06/29/2017 09:29 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> Currently, the expression completer matches explicit location options,
>> which would only make sense for commands that work with linespecs, not
>> expressions.
>>
>> I.e., currently, this:
>>  "p -functi"
>>
>> Completes to:
>>  "p -function "
> 
> I don't know much about completer, expression and explicit location, so
> I don't know what do you fix here, anything wrong in the completion?
> 

This is fixing a mismatch between what the completer thinks the
print command understands, and what the command really understands.

"-function" is an explicit location option that is understood by
commands that take (linespecs and) explicit locations as argument.
I.e, breakpoint commands, and "list".  For example:

 (gdb) b -source file.c -function my_func

So for those commands, it makes sense that the completer
completes 

 "b -sour[TAB]" -> "b -source "
 "b -functi[TAB]" -> "b -function "

etc.

(
A bit orthogonal, but for clarity: currently, the explicit locations
completer doesn't help much with discovering the supported options,
because "b -[TAB]" doesn't show you the list of options.
It shows the _whole_ list of symbols in the program, which is
useless...  After the series, specifically after the
"A smarter linespec completer" patch, it will, though:

 (gdb) b -[TAB]
 -function      -label         -line          -probe         -probe-dtrace  -probe-stap    -qualified     -source        
)

However, commands that take expressions (not linespecs/locations) as
arguments, such as the "print" command, do _not_ understand
the "-source", "-function" etc. switches.  Instead, "-foo" is
understood as an expression applying unary minus on a symbol
named "foo" (think "print -1").
So it does not make sense for the associated completer to
complete on those options:

 (gdb) p -func[TAB]
 (gdb) p -function [RET]
 No symbol "function" in current context.

The patch fixes this by having the expression_completer function
bypass the function that completes explicit locations:

    /* Not ideal but it is what we used to do before...  */
 -  return location_completer (ignore, p, word);
 +  return linespec_location_completer (ignore, text, word);

(Note how location_completer calls into linespec_location_completer.)

>> This patch fixes that, and adds regression tests.

Thanks,
Pedro Alves
  

On 06/29/2017 11:56 AM, Pedro Alves wrote:

> The patch fixes this by having the expression_completer function
> bypass the function that completes explicit locations:
> 
>     /* Not ideal but it is what we used to do before...  */
>  -  return location_completer (ignore, p, word);
>  +  return linespec_location_completer (ignore, text, word);
> 

Hmm, reading this back made my realize that I'm dropping "p"
here, which is computed just above.  That doesn't make a difference
to the test added by 37cd5d19fecc (2008), which was the commit
that added that code.  Hmm, looks like that's a bit of dead code
we can remove.

Thanks,
Pedro Alves
  

Pedro Alves <palves@redhat.com> writes:

> commands that take expressions (not linespecs/locations) as
> arguments, such as the "print" command, do _not_ understand
> the "-source", "-function" etc. switches.  Instead, "-foo" is
> understood as an expression applying unary minus on a symbol
> named "foo" (think "print -1").

This paragraph is useful.  Thanks for the explanation.  Patch is good to
me.