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

[Keith Seitz <keiths at redhat dot com>]

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