Re: [PATCH 32/40] Make "break foo" find "A::foo", A::B::foo", etc. [C++ and wild matching]

Keith Seitz
2017-08-08T23:48:53Z

On 06/02/2017 05:22 AM, Pedro Alves wrote:

> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 70c0e02..328b6db 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -15694,7 +15694,10 @@ Explicit locations are similar to linespecs but use an option/argument\n\
> syntax to specify location parameters.\n\
> Example: To specify the start of the label named \"the_top\" in the\n\
> function \"fact\" in the file \"factorial.c\", use \"-source factorial.c\n\
> --function fact -label the_top\".\n"
> +-function fact -label the_top\".\n\
> +For C++, \"-function\" matches all functions with the given name ignoring\n\
^
comma

> +missing leading specifiers (namespaces and classes). You can override\n\
> +that by instead specifying a fully qualified name using \"-qualified\".\n"

I think this would read better if it read: "This behavior may be overridden
by using the \"-qualified\" flag and specifying a fully qualified name."
[I am not a fan of using informal writing in documentation.]

> /* This help string is used for the break, hbreak, tbreak and thbreak
> commands. It is defined as a macro to prevent duplication.
> diff --git a/gdb/completer.c b/gdb/completer.c
> index eabbce7..99e40a3 100644
> --- a/gdb/completer.c
> +++ b/gdb/completer.c
> @@ -609,6 +612,7 @@ static const char *const explicit_options[] =
> {
> "-source",
> "-function",
> + "-qualified",
> "-line",
> "-label",
> NULL

The "-qualified" option can be used with linespecs, too, right?

(gdb) b -qualified A::b (a linespec location)
(gdb) b -qualified -function A::b (an explicit location)

Actually, I see that it does not work (yet?). Consider:

1 struct A
2 {
3 int doit ()
4 {
5 int i;
6
7 for (i = 0; i < 10; ++i)
8 {
9 switch (i)
10 {
11 top:
12 case 5:
13 ++i;
14 goto top;
15 default: break;
16 }
17 }
18 return i;
19 }
20 };

(gdb) b A::doit:top
Breakpoint 1 at 0x400633: file simple-label.cc, line 11.
(gdb) b -function A::doit -label top
Note: breakpoint 1 also set at pc 0x400633.
Breakpoint 2 at 0x400633: file simple-label.cc, line 11.
(gdb) b -qualified A::doit:top
Function "A::doit:top" not defined.
Make breakpoint pending on future shared library load? (y or [n]) n
(gdb) b -qualified A::doit -label top
Note: breakpoints 1 and 2 also set at pc 0x400633.
Breakpoint 3 at 0x400633: file simple-label.cc, line 11.

Is there a reason to exclude linespecs? Perhaps naively, I would have thought to
make -qualified a parsing option instead of a replacement for -function.

> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
> index 397c738..84d8a6b 100644
> --- a/gdb/cp-support.c
> +++ b/gdb/cp-support.c
> @@ -1671,19 +1760,151 @@ cp_fq_symbol_name_matches (const char *symbol_search_name,
> return false;
> }
>
> +/* C++ symbol_name_matcher_ftype implementation for wild matches.
> + Defers work to cp_symbol_name_ncmp. */
^^^^^^^^^^^^^^^^^^^

I think that's supposed to be cp_symbol_name_matches_1.

> +
> +static bool
> +cp_symbol_name_matches (const char *symbol_search_name,
> + const lookup_name_info &lookup_name,
> + completion_match_result *comp_match_res)
> +{
> + const std::string &name = lookup_name.cplus ().lookup_name ();
> +
> + strncmp_iw_mode mode = (lookup_name.completion_mode ()
> + ? strncmp_iw_mode::NORMAL
> + : strncmp_iw_mode::MATCH_PARAMS);
> +
> + return cp_symbol_name_matches_1 (symbol_search_name,
> + name.c_str (), name.size (),
> + mode, comp_match_res);
> +}
> +
[snip]
> diff --git a/gdb/cp-support.h b/gdb/cp-support.h
> index 1cef3f7..600720f 100644
> --- a/gdb/cp-support.h
> +++ b/gdb/cp-support.h
> @@ -110,6 +110,8 @@ extern struct symbol **make_symbol_overload_list_adl (struct type **arg_types,
> extern struct type *cp_lookup_rtti_type (const char *name,
> struct block *block);
>
> +extern unsigned int cp_search_name_hash (const char *search_name);
> +

Shouldn't the comment from cp-support.c be here?

> extern symbol_name_matcher_ftype *cp_get_symbol_name_matcher
> (const lookup_name_info &lookup_name);
>
[snip]

WDYT?

Keith