On Sat, Sep 2, 2023 at 1:29 AM Tom Tromey wrote: > block_find_symbol takes a callback function, but only two callbacks > are ever passed to it -- and they are similar enough that it seems > cleaner to just have block_find_symbol do the work itself. Also, > block_find_symbol can take a lookup_name_info as an argument, > following the general idea of pushing the construction of these > objects as high in the call chain as feasible. > > Regression tested on x86-64 Fedora 38. > --- > gdb/block.c | 43 +++++++++++-------------------------------- > gdb/block.h | 36 ++++++------------------------------ > gdb/symfile-debug.c | 10 +++------- > gdb/symtab.c | 13 +++++++------ > 4 files changed, 27 insertions(+), 75 deletions(-) > > diff --git a/gdb/block.c b/gdb/block.c > index 8e1b6ec88d4..6ada69c388f 100644 > --- a/gdb/block.c > +++ b/gdb/block.c > @@ -778,46 +778,25 @@ block_lookup_symbol_primary (const struct block > *block, const char *name, > /* See block.h. */ > > struct symbol * > -block_find_symbol (const struct block *block, const char *name, > - const domain_enum domain, > - block_symbol_matcher_ftype *matcher, void *data) > +block_find_symbol (const struct block *block, const lookup_name_info > &name, > + const domain_enum domain, struct symbol **stub) > { > - lookup_name_info lookup_name (name, symbol_name_match_type::FULL); > - > /* Verify BLOCK is STATIC_BLOCK or GLOBAL_BLOCK. */ > gdb_assert (block->superblock () == NULL > || block->superblock ()->superblock () == NULL); > > - for (struct symbol *sym : block_iterator_range (block, &lookup_name)) > + for (struct symbol *sym : block_iterator_range (block, &name)) > { > - /* MATCHER is deliberately called second here so that it never sees > - a non-domain-matching symbol. */ > - if (sym->matches (domain) > - && matcher (sym, data)) > - return sym; > - } > - return NULL; > -} > - > -/* See block.h. */ > + if (!sym->matches (domain)) > + continue; > > -int > -block_find_non_opaque_type (struct symbol *sym, void *data) > -{ > - return !TYPE_IS_OPAQUE (sym->type ()); > -} > - > -/* See block.h. */ > - > -int > -block_find_non_opaque_type_preferred (struct symbol *sym, void *data) > -{ > - struct symbol **best = (struct symbol **) data; > + if (!TYPE_IS_OPAQUE (sym->type ())) > + return sym; > > - if (!TYPE_IS_OPAQUE (sym->type ())) > - return 1; > - *best = sym; > - return 0; > + if (stub != nullptr) > + *stub = sym; > + } > + return nullptr; > } > > /* See block.h. */ > diff --git a/gdb/block.h b/gdb/block.h > index f132d351bb6..3a197e63754 100644 > --- a/gdb/block.h > +++ b/gdb/block.h > @@ -556,39 +556,15 @@ extern struct symbol *block_lookup_symbol_primary > (const struct block *block, > const char *name, > const domain_enum > domain); > > -/* The type of the MATCHER argument to block_find_symbol. */ > - > -typedef int (block_symbol_matcher_ftype) (struct symbol *, void *); > - > -/* Find symbol NAME in BLOCK and in DOMAIN that satisfies MATCHER. > - DATA is passed unchanged to MATCHER. > - BLOCK must be STATIC_BLOCK or GLOBAL_BLOCK. */ > +/* Find symbol NAME in BLOCK and in DOMAIN. This will return a > + matching symbol whose type is not a "opaque", see TYPE_IS_OPAQUE. > + If STUB is non-NULL, an otherwise matching symbol whose type is a > + opaque will be stored here. */ > > extern struct symbol *block_find_symbol (const struct block *block, > - const char *name, > + const lookup_name_info &name, > const domain_enum domain, > - block_symbol_matcher_ftype > *matcher, > - void *data); > - > -/* A matcher function for block_find_symbol to find only symbols with > - non-opaque types. */ > - > -extern int block_find_non_opaque_type (struct symbol *sym, void *data); > - > -/* A matcher function for block_find_symbol to prefer symbols with > - non-opaque types. The way to use this function is as follows: > - > - struct symbol *with_opaque = NULL; > - struct symbol *sym > - = block_find_symbol (block, name, domain, > - block_find_non_opaque_type_preferred, > &with_opaque); > - > - At this point if SYM is non-NULL then a non-opaque type has been found. > - Otherwise, if WITH_OPAQUE is non-NULL then an opaque type has been > found. > - Otherwise, the symbol was not found. */ > - > -extern int block_find_non_opaque_type_preferred (struct symbol *sym, > - void *data); > + struct symbol **stub); > > /* Given a vector of pairs, allocate and build an obstack allocated > blockranges struct for a block. */ > diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c > index 9db5c47a8ce..233cf3390e5 100644 > --- a/gdb/symfile-debug.c > +++ b/gdb/symfile-debug.c > @@ -260,23 +260,19 @@ objfile::lookup_symbol (block_enum kind, const char > *name, domain_enum domain) > const struct blockvector *bv = stab->blockvector (); > const struct block *block = bv->block (kind); > > - sym = block_find_symbol (block, name, domain, > - block_find_non_opaque_type_preferred, > - &with_opaque); > + sym = block_find_symbol (block, lookup_name, domain, &with_opaque); > > /* Some caution must be observed with overloaded functions > and methods, since the index will not contain any overload > information (but NAME might contain it). */ > > - if (sym != NULL > - && symbol_matches_search_name (sym, lookup_name)) > + if (sym != nullptr) > { > retval = stab; > /* Found it. */ > return false; > } > - if (with_opaque != NULL > - && symbol_matches_search_name (with_opaque, lookup_name)) > + if (with_opaque != nullptr) > retval = stab; > > /* Keep looking through other psymtabs. */ > diff --git a/gdb/symtab.c b/gdb/symtab.c > index d8ce2bf8482..29ca418d1e9 100644 > --- a/gdb/symtab.c > +++ b/gdb/symtab.c > @@ -2675,9 +2675,10 @@ basic_lookup_transparent_type_quick (struct objfile > *objfile, > > bv = cust->blockvector (); > block = bv->block (block_index); > - sym = block_find_symbol (block, name, STRUCT_DOMAIN, > - block_find_non_opaque_type, NULL); > - if (sym == NULL) > + > + lookup_name_info lookup_name (name, symbol_name_match_type::FULL); > + sym = block_find_symbol (block, lookup_name, STRUCT_DOMAIN, nullptr); > + if (sym == nullptr) > error_in_psymtab_expansion (block_index, name, cust); > gdb_assert (!TYPE_IS_OPAQUE (sym->type ())); > return sym->type (); > @@ -2696,13 +2697,13 @@ basic_lookup_transparent_type_1 (struct objfile > *objfile, > const struct block *block; > const struct symbol *sym; > > + lookup_name_info lookup_name (name, symbol_name_match_type::FULL); > for (compunit_symtab *cust : objfile->compunits ()) > { > bv = cust->blockvector (); > block = bv->block (block_index); > - sym = block_find_symbol (block, name, STRUCT_DOMAIN, > - block_find_non_opaque_type, NULL); > - if (sym != NULL) > + sym = block_find_symbol (block, lookup_name, STRUCT_DOMAIN, > nullptr); > + if (sym != nullptr) > { > gdb_assert (!TYPE_IS_OPAQUE (sym->type ())); > return sym->type (); > -- > 2.41.0 > > Things are simpler and nicer this way indeed. I tested this on ppc64le Fedora Rawhide and I can see no regressions.