Re: [PATCH v2] Factor out the common code in lookup_{static,global}_symbol

["Christian Biesinger via gdb-patches" <gdb-patches@sourceware.org>]

On Sun, Aug 25, 2019 at 9:20 PM Simon Marchi <simark@simark.ca> wrote:
>
> On 2019-08-05 11:33 a.m., Christian Biesinger via gdb-patches wrote:
> > @@ -2614,6 +2576,9 @@ struct global_sym_lookup_data
> >    /* The domain to use for our search.  */
> >    domain_enum domain;
> >
> > +  /* The block index in which to search */
>
> Period and two spaces:
>
>   /* The block index in which to search.  */

Done

> > -struct block_symbol
> > -lookup_global_symbol (const char *name,
> > -                   const struct block *block,
> > -                   const domain_enum domain)
> > +static struct block_symbol
> > +lookup_global_symbol_internal (const char *name,
> > +                            enum block_enum block_index,
> > +                            const struct block *block,
> > +                            const domain_enum domain)
>
> I'd rename that to lookup_symbol_internal, since it's used both in lookup_global_symbol and
> lookup_static_symbol.  If you think it's too general (the function doesn't look up local symbols),
> Maybe "lookup_global_or_static_symbol"?  And global_sym_lookup_data should be renamed accordingly.
>
> And since BLOCK is only used for looking up an objfile in the global case, I would suggest
> making this function accept the objfile instead of a block.  Otherwise, it could be confused
> as "lookup symbol in this block".  Also, if some code already has the objfile handy, they can
> pass it down, in which case we avoid the cost of lookup_objfile_from_block.

Done

> > +/* See symtab.h.  */
> > +
> > +struct block_symbol
> > +lookup_static_symbol (const char *name, const domain_enum domain)
> > +{
> > +  /* TODO: Should static symbol lookup start with a block as well, so we can
> > +     prefer a static symbol in the current CU?  */
> > +  return lookup_global_symbol_internal (name, STATIC_BLOCK, nullptr, domain);
>
> I'd say that we can add another argument when the need comes, it's not a stable API.

Removed