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

[Simon Marchi <simon.marchi@efficios.com>]

On 9/24/26 6:39 AM, Andrew Burgess wrote:
> 
> 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 :)

Done (Co-Authored-By).

>> 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 ....'

Done.

>> +  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.

Done.

>> +
>> +/* 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.

I agree, done.

> With these fixes:
> 
> Approved-By: Andrew Burgess <aburgess@redhat.com>
> 
> Thanks,
> Andrew

Thanks for the review, I will push it later today (to both master and
gdb-18-branch).  Hopefully Tom has some time to give it a quick look
today, just to get another pair of eyes on it.

Simon