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

[Pedro Alves <palves at redhat dot com>]

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