Re: [PATCH 28/40] lookup_name_info::make_ignore_params

[Keith Seitz <keiths@redhat.com>]

On 06/02/2017 05:22 AM, Pedro Alves wrote:
> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
> 
> 	* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
> 	unittests/lookup_name_info-selftests.c.
> 	(SUBDIR_UNITTESTS_OBS): Add lookup_name_info-selftests.o.
> 	* cp-support.c: Include "selftest.h".
> 	(cp_remove_params_1): Rename from cp_remove_params.  Add
> 	'require_param's parameter, and handle it.
                      ^^
typo

> 	(cp_remove_params): Reimplement.
> 	(cp_remove_params_if_any): New.
> 	(selftests::quote): New.
> 	(selftests::check_remove_params): New.
> 	(selftests::test_cp_remove_params): New.
> 	(_initialize_cp_support): Install
> 	selftests::test_cp_remove_params.
> 	* cp-support.h (cp_remove_params_if_any): Declare.
> 	* dwarf2read.c :Include "selftest.h".
> 	(dw2_expand_symtabs_matching_symbol): Use
> 	lookup_name_info::make_ignore_params.
> 	(selftests::dw2_expand_symtabs_matching::mock_mapped_index)
> 	(selftests::dw2_expand_symtabs_matching::string_or_null)
> 	(selftests::dw2_expand_symtabs_matching::check_match)
> 	(selftests::dw2_expand_symtabs_matching::test_symbols)
> 	(selftests::dw2_expand_symtabs_matching::run_test): New.
> 	(_initialize_dwarf2_read): Register
> 	selftests::dw2_expand_symtabs_matching::run_test.
> 	* psymtab.c (psym_expand_symtabs_matching): Use
> 	lookup_name_info::make_ignore_params.
> 	* symtab.c (demangle_for_lookup_info::demangle_for_lookup_info):
> 	If the lookup name wants to ignore parameters, strip them.
> 	(compare_symbol_name): Remove sym_text/sym_text_len parameters and
> 	code handling '('.
> 	(completion_list_add_name): Don't pass down sym_text/sym_text_len.
> 	(default_collect_symbol_completion_matches_break_on): Don't try to
> 	strip parameters.
> 	* symtab.h (lookup_name_info::lookup_name_info): Add
> 	'ignore_parameters' parameter.
> 	(lookup_name_info::ignore_parameters)
> 	(lookup_name_info::make_ignore_params): New methods.
> 	(lookup_name_info::m_ignore_parameters): New field.

(class lookup_name_info) <ingore_parameters>: ?
(class lookup_name_into) <make_ignore_params>: ?
(class lookup_name_info) <m_ignore_params>: ?

> 	* unittests/lookup_name_info-selftests.c: New file.


> 	* dwarf2read.c :
> 	(selftests::dw2_expand_symtabs_matching::mock_mapped_index)
> 	(selftests::dw2_expand_symtabs_matching::string_or_null)
> 	(selftests::dw2_expand_symtabs_matching::check_match)
> 	(selftests::dw2_expand_symtabs_matching::test_symbols)
> 	(selftests::dw2_expand_symtabs_matching::run_test): New.
> 	(_initialize_dwarf2_read): Register
> 	selftests::dw2_expand_symtabs_matching::run_test.

Just some trivial things that I noticed (inline).

> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
> index 0095c6d..064a45b 100644
> --- a/gdb/cp-support.c
> +++ b/gdb/cp-support.c
> @@ -833,12 +834,14 @@ cp_func_name (const char *full_name)
>    return ret;
>  }
>  
> -/* DEMANGLED_NAME is the name of a function, including parameters and
> -   (optionally) a return type.  Return the name of the function without
> -   parameters or return type, or NULL if we can not parse the name.  */
> +/* Helper for cp_remove_params.  DEMANGLED_NAME is the name of a
> +   function, including parameters and (optionally) a return type.
> +   Return the name of the function without parameters or return type,
> +   or NULL if we can not parse the name.  If COMPLETION_MODE is true,
> +   then tolerate a non-existing or unbalanced parameter list.  */

s/COMPLETION_MODE/REQUIRE_PARAMS/ ?

> -gdb::unique_xmalloc_ptr<char>
> -cp_remove_params (const char *demangled_name)
> +static gdb::unique_xmalloc_ptr<char>
> +cp_remove_params_1 (const char *demangled_name, bool require_params)
>  {
>    bool done = false;
>    struct demangle_component *ret_comp;
> @@ -874,10 +877,60 @@ cp_remove_params (const char *demangled_name)
[snip]
> +/* DEMANGLED_NAME is the name of a function, (optionally) including
> +   parameters and (optionally) a return type.  Return the name of the
> +   function without parameters or return type, or NULL if we can not
> +   parse the name.  If COMPLETION_MODE is true, then tolerate a
> +   non-existing or unbalanced parameter list.  */
> +

Shouldn't this comment be in cp-support.h?

> +gdb::unique_xmalloc_ptr<char>
> +cp_remove_params_if_any (const char *qualified, bool completion_mode)
> +{
> +  /* Trying to remove parameters from the empty string fails.  If
> +     we're completing / matching everything, avoid returning NULL
[snip]
> +  /* Shouldn't this parse?  Looks like a bug in
> +     cp_demangled_name_to_comp.  */

Is there a bugzilla (or some other tracking) for this?

> +#if 0
> +  CHECK ("A::foo<void(int)>::func(int)",
> +	 "A::foo<void(int)>::func");
> +#else
> +  CHECK_INCOMPL ("A::foo<void(int)>::func(int)",
> +		 "A::foo");
> +#endif
> +
> +  CHECK_INCOMPL ("A::foo<void(int",
> +		 "A::foo");
> +
> +#undef CHECK
> +#undef CHECK_INCOMPL
> +}
> +
> +} // namespace selftests
> +
> +#endif /* GDB_SELF_CHECK */
> +
>  /* Don't allow just "maintenance cplus".  */
>  
>  static  void
> diff --git a/gdb/cp-support.h b/gdb/cp-support.h
> index dd42415..1cef3f7 100644
> --- a/gdb/cp-support.h
> +++ b/gdb/cp-support.h
> @@ -97,6 +97,9 @@ extern char *cp_func_name (const char *full_name);
>  
>  extern gdb::unique_xmalloc_ptr<char> cp_remove_params (const char *qualified);
>  
> +extern gdb::unique_xmalloc_ptr<char> cp_remove_params_if_any
> +  (const char *qualified, bool completion_mode);

[Add comment from cp-support.c?]

> +
>  extern struct symbol **make_symbol_overload_list (const char *,
>  						  const char *);
>  
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index 4c8b1f6..a452804 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -176,6 +178,16 @@ class lookup_name_info final
>    symbol_name_match_type match_type () const { return m_match_type; }
>    bool completion_mode () const { return m_completion_mode; }
>    const std::string &name () const { return m_name; }
> +  const bool ignore_parameters () const { return m_ignore_parameters; }
> +
> +  /* Return a version of this lookup name that is usable with
> +     comparisons against symbols have have no parameter info, such as
                                    ^^^^^^^^^
typo

> +     psymbols and GDB index symbols.  */
> +  lookup_name_info make_ignore_params () const
> +  {
> +    return lookup_name_info (m_name, m_match_type, m_completion_mode,
> +			     true /* ignore params */);
> +  }
>  
>    /* Get the search name hash for searches in language LANG.  */
>    unsigned int search_name_hash (language lang) const

Keith