* [PATCH] Simplify block_find_symbol
@ 2023-09-01 23:28 Tom Tromey
2023-09-06 11:50 ` Alexandra Petlanova Hajkova
0 siblings, 1 reply; 3+ messages in thread
From: Tom Tromey @ 2023-09-01 23:28 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Simplify block_find_symbol
2023-09-01 23:28 [PATCH] Simplify block_find_symbol Tom Tromey
@ 2023-09-06 11:50 ` Alexandra Petlanova Hajkova
2023-09-07 18:24 ` Tom Tromey
0 siblings, 1 reply; 3+ messages in thread
From: Alexandra Petlanova Hajkova @ 2023-09-06 11:50 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 8149 bytes --]
On Sat, Sep 2, 2023 at 1:29 AM Tom Tromey <tom@tromey.com> 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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Simplify block_find_symbol
2023-09-06 11:50 ` Alexandra Petlanova Hajkova
@ 2023-09-07 18:24 ` Tom Tromey
0 siblings, 0 replies; 3+ messages in thread
From: Tom Tromey @ 2023-09-07 18:24 UTC (permalink / raw)
To: Alexandra Petlanova Hajkova; +Cc: Tom Tromey, gdb-patches
>>>>> "Alexandra" == Alexandra Petlanova Hajkova <ahajkova@redhat.com> writes:
Alexandra> Things are simpler and nicer this way indeed. I tested this
Alexandra> on ppc64le Fedora Rawhide and I can see no regressions.
Thanks. I'm going to check it in.
Tom
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-07 20:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-01 23:28 [PATCH] Simplify block_find_symbol Tom Tromey
2023-09-06 11:50 ` Alexandra Petlanova Hajkova
2023-09-07 18:24 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).