Re: [PATCH 2/4] Don't erase empty indices in DWARF reader

[Joel Brobecker <brobecker@adacore.com>]

Hi Tom,

On Mon, Dec 19, 2022 at 10:09:06AM -0700, Tom Tromey via Gdb-patches wrote:
> >>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
> 
> >> The DWARF reader has some code to remove empty indices.  However, I
> >> think this code has been obsolete since some earlier changes to
> >> parallel_for_each.  This patch removes this code.
> 
> Andrew> Would NULL entries cause problems later in GDB?  Would it be worth
> Andrew> replacing this code with an assert that there are no NULL entries?  Or
> Andrew> would an attempt to create a NULL entry trigger an assert/error
> Andrew> elsewhere?
> 
> If it could happen, it would cause a crash when doing any kind of
> lookup, because cooked_index_vector iterates over the indices and calls
> methods on each one.
> 
> Andrew> Or maybe the cost of iterating over the list is what you want to remove
> Andrew> here?  In which case, could we guard an assert in '#ifdef DEVELOPER'?
> 
> I don't think there's any performance issue, as the size of the array is
> normally just the number of cores on the user machine.
> 
> It's more that this is a leftover and can't happen any more, due to the
> previous patch.  It seems to me that the contract of parallel_for_each
> should be that, if it returns results (i.e., not void), then each entry
> in the result must be the result of actually calling the callback --
> i.e., not some default.
> 
> That said, it's also fine to drop this patch.

If we believe the code is dead, let's drop it.

Maybe Andrew's assert suggestion could be helpful, although perhaps
a complaint might be a better compromise in this case, as I would say
a violation of this expectation would still mean a usable debugging
experience for the user, so let's not break things with an assert?

Thank you!
-- 
Joel