Re: [PATCH 7/9] gdb: add matching helpers to global_symbol_searcher

[Simon Marchi <simon.marchi@efficios.com>]

(I failed to reply-all the first time, adding gdb-patches this time)

On 2026-09-30 09:51, Rohr, Stephan wrote:
>> @@ -4824,8 +4901,6 @@ global_symbol_searcher::add_matching_symbols
>>  	 const std::optional<compiled_regex> &type_regex,
>>  	 std::set<symbol_search> *result_set) const
>>  {
>> -  domain_search_flags domain = to_search_flags (m_kind);
> 
> IIUC this is moved to "symbol_matches"?  This implies "to_search_flags" is run
> for every symbol.  This looks unnecessary.  We could add an additional argument to
> "symbol_matches".

I thought about that, but I am not sure it matters in practice, since
to_search_flags compiles down to so few instructions.

If you insist that we shouldn't do this call for every minsym, I would
add an m_domain field and pre-compute it.  But I don't think it's
needed.

>> diff --git a/gdb/symtab.h b/gdb/symtab.h
>> index acbe41d131b7..5dd0475e4491 100644
>> --- a/gdb/symtab.h
>> +++ b/gdb/symtab.h
>> @@ -2675,6 +2675,19 @@ class global_symbol_searcher
>>    /* Compile M_SYMBOL_TYPE_REGEXP, if set.  */
>>    std::optional<compiled_regex> compile_type_regex () const;
>>
>> +  /* Return true if SYMTAB matches one of M_FILENAMES.  */
>> +  bool symtab_matches_filenames (symtab *symtab) const;
>> +
>> +  /* Return true if SYM is of type M_KIND, is in a file matching
> 
> Rephrase to "is of kind M_KIND"? 

Done.

>> +     M_FILENAMES, and matches NAME_REGEX and TYPE_REGEX.  */
>> +  bool symbol_matches (const symbol *sym,
>> +		       const std::optional<compiled_regex> &name_regex,
>> +		       const std::optional<compiled_regex> &type_regex) const;
>> +
>> +  /* Return true if MSYMBOL is of type M_KIND and matches NAME_REGEX.
>> */
> 
> Same here.

Done.

>> +  bool msymbol_matches (const minimal_symbol *msymbol,
>> +			const std::optional<compiled_regex> &name_regex)
>> const;
> 
> The function also checks the "created_by_gdb" flag and "is_suitable_msymbol".
> Should this be mentioned in the comment, too?

is_suitable_symbol basically checks m_kind, so it's already covered in
the comment.

As for created_by_gdb... It's subjective, but I don't think that a
function doc like this necessarily needs to talk about all the details
of the implementation.  But anyway, I added a mention:

  /* Return true if MSYMBOL is of kind M_KIND and matches NAME_REGEX.

     This function also filters out the minimal symbols artificially created by
     GDB.  */

Simon