[4/9] gdb: use a switch on m_kind in add_matching_symbols
Checks
Commit Message
add_matching_symbols applies some filters that only make sense for some
kinds of searches: the type regexp for variables and functions, the
LOC_UNRESOLVED / enum constant filter for variables, and the line number
filter for modules. Some of these check m_kind, others check the
symbol's domain. Since the earlier sym->matches check only lets through
symbols whose domain corresponds to m_kind, the two are equivalent.
Group these filters in a single switch on m_kind. Add a comment for
the module filter, which skips module declarations, because it wasn't
obvious to me at first sight.
Change-Id: Idf1b366b4cfd16187848b64075a0067190c111fc
---
gdb/symtab.c | 33 +++++++++++++++++++--------------
1 file changed, 19 insertions(+), 14 deletions(-)
Comments
Seems like you missed doing reply all, so re-adding gdb-patches.
On 2026-09-30 08:32, Rohr, Stephan wrote:
>
>
>> -----Original Message-----
>> From: Simon Marchi <simon.marchi@efficios.com>
>> Sent: Tuesday, 29 September 2026 21:39
>> To: gdb-patches@sourceware.org
>> Cc: Simon Marchi <simon.marchi@efficios.com>
>> Subject: [PATCH 4/9] gdb: use a switch on m_kind in add_matching_symbols
>>
>> add_matching_symbols applies some filters that only make sense for some
>> kinds of searches: the type regexp for variables and functions, the
>> LOC_UNRESOLVED / enum constant filter for variables, and the line number
>> filter for modules. Some of these check m_kind, others check the
>> symbol's domain. Since the earlier sym->matches check only lets through
>> symbols whose domain corresponds to m_kind, the two are equivalent.
>>
>> Group these filters in a single switch on m_kind. Add a comment for
>> the module filter, which skips module declarations, because it wasn't
>> obvious to me at first sight.
>>
>> Change-Id: Idf1b366b4cfd16187848b64075a0067190c111fc
>> ---
>> gdb/symtab.c | 33 +++++++++++++++++++--------------
>> 1 file changed, 19 insertions(+), 14 deletions(-)
>>
>> diff --git a/gdb/symtab.c b/gdb/symtab.c
>> index 4b2c4be8d117..2bb1cbf1b9ad 100644
>> --- a/gdb/symtab.c
>> +++ b/gdb/symtab.c
>> @@ -4860,25 +4860,30 @@
>> global_symbol_searcher::add_matching_symbols
>> nullptr, 0) != 0)
>> continue;
>>
>> - if (((sym->domain () == VAR_DOMAIN
>> - || sym->domain () == FUNCTION_DOMAIN)
>> - && type_regex.has_value ()
>> - && !treg_matches_sym_type_name (*type_regex, sym)))
>> - continue;
>> -
>> - if (m_kind == symbol_search_kind::VARIABLE)
>> + switch (m_kind)
>> {
>> + case symbol_search_kind::VARIABLE:
>> + /* LOC_CONST can be used for more than just enums, e.g.,
>> + C++ static const members. We only want to skip enums
>> + here. */
>> if (sym->loc_class () == LOC_UNRESOLVED
>> - /* LOC_CONST can be used for more than
>> - just enums, e.g., c++ static const
>> - members. We only want to skip enums
>> - here. */
>> || (sym->loc_class () == LOC_CONST
>> - && (sym->type ()->code () == TYPE_CODE_ENUM)))
>> + && sym->type ()->code () == TYPE_CODE_ENUM))
>> continue;
>> + [[fallthrough]];
>> +
>> + case symbol_search_kind::FUNCTION:
>> + if (type_regex.has_value ()
>> + && !treg_matches_sym_type_name (*type_regex, sym))
>> + continue;
>> + break;
>> +
>> + case symbol_search_kind::MODULE:
>> + /* Skip module declarations, keep only definitions. */
>> + if (sym->line () == 0)
>> + continue;
>> + break;
>
> For consistency, I think adding the fourth case would be good:
>
> case symbol_search_kind::TYPE:
> break;
>
> If the -Wno-switch is ever to be removed, this won't produce a warning.
I don't agree with this one, we don't need it precisely because we don't
use -Wswitch. If we ever want to enable -Wswitch, there are a ton of
sites we'll need to adjust, this one is no different.
Simon
@@ -4860,25 +4860,30 @@ global_symbol_searcher::add_matching_symbols
nullptr, 0) != 0)
continue;
- if (((sym->domain () == VAR_DOMAIN
- || sym->domain () == FUNCTION_DOMAIN)
- && type_regex.has_value ()
- && !treg_matches_sym_type_name (*type_regex, sym)))
- continue;
-
- if (m_kind == symbol_search_kind::VARIABLE)
+ switch (m_kind)
{
+ case symbol_search_kind::VARIABLE:
+ /* LOC_CONST can be used for more than just enums, e.g.,
+ C++ static const members. We only want to skip enums
+ here. */
if (sym->loc_class () == LOC_UNRESOLVED
- /* LOC_CONST can be used for more than
- just enums, e.g., c++ static const
- members. We only want to skip enums
- here. */
|| (sym->loc_class () == LOC_CONST
- && (sym->type ()->code () == TYPE_CODE_ENUM)))
+ && sym->type ()->code () == TYPE_CODE_ENUM))
continue;
+ [[fallthrough]];
+
+ case symbol_search_kind::FUNCTION:
+ if (type_regex.has_value ()
+ && !treg_matches_sym_type_name (*type_regex, sym))
+ continue;
+ break;
+
+ case symbol_search_kind::MODULE:
+ /* Skip module declarations, keep only definitions. */
+ if (sym->line () == 0)
+ continue;
+ break;
}
- if (sym->domain () == MODULE_DOMAIN && sym->line () == 0)
- continue;
/* Match, insert if not already in the results. */
if (result_set->size () < m_max_search_results)