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

Keith Seitz
2017-08-08T20:55:29Z

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