Re: [PATCH 2/2] gdb: resolve class name via DW_AT_signature in cooked index

[Andrew Burgess <aburgess@redhat.com>]

Simon,

Thanks for working on this.  This looks great, I have a couple of minor
comments.

Simon Marchi <simon.marchi@efficios.com> writes:

> From: Andrew Burgess <aburgess@redhat.com>

I'd be fine if you wanted to change yourself to be the author, you've
put a lot of work into this iteration.  But I feel you should at least
add a Co-Authored-By tag if nothing else :)


> diff --git a/gdb/dwarf2/cooked-index-entry.h b/gdb/dwarf2/cooked-index-entry.h
> index 60ea581cbbe5..a2e007b7cf69 100644
> --- a/gdb/dwarf2/cooked-index-entry.h
> +++ b/gdb/dwarf2/cooked-index-entry.h
> @@ -47,6 +47,8 @@ enum cooked_index_flag_enum : unsigned char
>    /* True if this is a function that has DW_AT_inline set in a way
>       that indicates it was inlined.  */
>    IS_INLINED = 64,
> +  /* True is m_name.deferred has a value rather than m_name.resolved.  */

Typo: 'True IF m_name.deferred ....'

> +  IS_NAME_DEFERRED = 128,

In cooked-index-entry.c you need to update:

  std::string to_string (cooked_index_flag flags)

which converts these flags to strings.



> +
> +/* See cooked-index.h.  */
> +
>  void
>  cooked_index::start_canonicalize_names ()
>  {
> @@ -124,10 +207,14 @@ cooked_index::start_canonicalize_names ()
>      {
>        group.add_task ([this, this_shard = shard.get ()] ()
>        {
> +	complaint_interceptor complaint_handler;
> +

I think you should add the complaint_interceptor to
cooked_index::start_resolve_deferred_parents and
cooked_index::start_prune_nameless_entries too.

It's not needed in those cases, but it's not needed here either.

My reasoning is that adding it is cheap, especially if no complaints are
emitted, and it's done in a worker thread, so we notice the impact even
less.

But, if we ever add a complaint in the future then you need to remember
to add the complaint_interceptor, or hope someone catches it during
review.

Given we're adding the infrastructure now, lets just add the
complaint_interceptor to all cases, then we're covered for future
growth.


With these fixes:

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew