Re: [PATCH v2.1 2/3] Make "break foo" find "A::foo", A::B::foo", etc. [C++ and wild matching]

[Keith Seitz <keiths at redhat dot com>]

On 11/27/2017 04:14 PM, Pedro Alves wrote:
> On 11/28/2017 12:01 AM, Pedro Alves wrote:
>> On 11/27/2017 05:13 PM, Pedro Alves wrote:
>>> In v2:
>>>
>>>   - Implements Keith's suggestion of making "-qualified" a flag
>>>     option, instead of being a replacement for "-function".  This
>>>     means that "break -q filename.cc:function" interprets
>>>     "filename.cc:function" as a linespec with two components instead
>>>     of a (bogus) function name.  Basically, this folds in these
>>>     changes (with some additional cleanup here and there):
>>>      https://sourceware.org/ml/gdb-patches/2017-11/msg00621.html
>>>      https://sourceware.org/ml/gdb-patches/2017-11/msg00618.html
>>
>> Rereading this, I realized that the "-qualified" options wasn't being saved
>> by "save breakpoints".  There were a couple problems.  First,
>> linespec.c:canonicalize_linespec was losing the symbol_name_match_type.
>> While addressing this, I realized that we don't really need to add
>> a new field to ls_parser to record the desired symbol_name_match_type,
>> since we can just use the one in PARSER_EXPLICIT.
>> The second is that I missed updating the "-qualified" handling in
>> explicit_to_string_internal to take into account the fact that
>> "-qualified" now appears with traditional linespecs as well.

Hahaha. If I had a dime for every time I (almost?) forgot to check
"save-breakpoints," I'd have a couple of bucks by now...

>> Below's what I'm folding in to the patch, to address this.  New
>> testcase included.
> 
> And here's the resulting patch with that folded in.
> 

Just one little comment.

> diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
> index 833bdc0..6cb1d71 100644
> --- a/gdb/mi/mi-cmd-break.c
> +++ b/gdb/mi/mi-cmd-break.c
> @@ -337,7 +337,8 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
>      }
>    else
>      {
> -      location = string_to_event_location_basic (&address, current_language);
> +      location = string_to_event_location_basic (&address, current_language,
> +						 symbol_name_match_type::WILD);
>        if (*address)
>  	error (_("Garbage '%s' at end of location"), address);
>      }

At first, the use of ::WILD here seemed odd to me. MI is to be used by user
interfaces like Eclipse, I wondered why Eclipse would need WILD-card access
to symbol names.

But then I remembered that having access to full source code "database" is not
(or should not) be a requirement for using MI.

So that leaves me to assume that we intend a follow-on patch to address
adding a "-qualified"-like flag for MI, too. Not requesting changes. Just
thinking aloud.

[I would guess the same for the similar python and guile changes.]

Am I in left field?

That said, LGTM.

Keith