Re: [PATCH 1/4] [gdb/symtab] Fix htab_find_slot call in read_call_site_scope

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2021-10-04 08:46, Tom de Vries wrote:
> On 10/4/21 2:05 PM, Simon Marchi wrote:
>>>> Ah, I had not seen this comment.  So it was on purpose.  Still, I think
>>>> that it makes it more confusing than anything.  The patch LGTM.
>>>
>>> And, this follow-up commit reverts everything except the comment.
>>>
>>> Any comments?
>>>
>>> Thanks,
>>> - Tom
>>>
>>
>> Is there a problem with having the lookups done with just the pc? 
> 
> Well, there's the problem that I describe in the commit message.  I
> don't known of any other problem.
> 
>> If we
>> were to replace this with some C++ hash table, say std::unordered_map, it
>> would be std::unordered_map<CORE_ADDR /* pc */, call_site>. 
> 
> Right, because there's a separation between key and element.
> 
>> So doing
>> the lookups using just the pc in the htab makes sense to me.
> 
> I'm sorry, I don't really understand what you're trying to say here.
> 
> Do you agree with the patch?  Do you disagree with the patch.  Are you
> suggesting an alternative solution?

My thinking was: why not keep core_addr_eq and core_addr_hash, and pass
a pointer to a CORE_ADDR when calling htab_find_slot.  And drop the
requirement for call_site::pc to be the first member of call_site.  But
I now realize that this may not be a correct use of htab: htab_find
passes entries to the eq func.  So if we have core_addr_eq as the eq
func and htab passes a call_site* entry to core_addr_eq, it won't work.
Now, we don't actually use htab_find, so it would still work for us.
But that would be like setting up a trap for whoever tries to use
htab_find in the future.

So, the patche LGTM.

Simon