[6/9] gdb: update comment of global_symbol_searcher::expand_symtabs

Message ID 20260929194119.155169-7-simon.marchi@efficios.com
State New
Headers
Series Cleanups in global_symbol_searcher |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed

Commit Message

Simon Marchi Sept. 29, 2026, 7:39 p.m. UTC
  Update the comment of expand_symtabs to describe more accurately what
the return value means.  It is true when a matching function msymbol for
which no debug info exists is found.

Change-Id: I7c26c065b31575066ed2398563cdc1dac8869f8b
---
 gdb/symtab.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

Comments

Stephan Rohr Sept. 30, 2026, 1:08 p.m. UTC | #1
> -----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 6/9] gdb: update comment of
> global_symbol_searcher::expand_symtabs
> 
> Update the comment of expand_symtabs to describe more accurately what
> the return value means.  It is true when a matching function msymbol for
> which no debug info exists is found.
> 
> Change-Id: I7c26c065b31575066ed2398563cdc1dac8869f8b
> ---
>  gdb/symtab.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index 50d7000681df..acbe41d131b7 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -2676,8 +2676,9 @@ class global_symbol_searcher
>    std::optional<compiled_regex> compile_type_regex () const;
> 
>    /* Expand symtabs in OBJFILE that match NAME_REGEX, are of type
> M_KIND.
> -     Return true if any msymbols were seen that we should later consider
> -     adding to the results list.  */
> +     Return true if a matching function msymbol without debug info was
> +     seen, meaning that msymbols should later be considered for the results
> +     list.  */

If we update comment, what about updating the first sentence to

"Expand symtabs in OBJFILE that match NAME_REGEX and are of type M_KIND."

This reads better from my point of view.

I was also wondering if it's worth updating the comments inside the "expand_symtabs"
function:

  - [..] When no file names were given the caller unconditionally rescans [..]

     The "unconditionally" can be dropped.  It is gated in "search ()".

- The comment this contains "SEARCH_VAR_DOMAIN".  Should this be changed to
   "symbol_search_kind:: VARIABLE" instead?  Not sure if this is the right patch for this
   change.

>    bool expand_symtabs (objfile *objfile,
>  		       const std::optional<compiled_regex> &name_regex) const;
> 
> --
> 2.55.0

________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
  
Simon Marchi Sept. 30, 2026, 8:01 p.m. UTC | #2
On 2026-09-30 09:08, Rohr, Stephan wrote:
>> diff --git a/gdb/symtab.h b/gdb/symtab.h
>> index 50d7000681df..acbe41d131b7 100644
>> --- a/gdb/symtab.h
>> +++ b/gdb/symtab.h
>> @@ -2676,8 +2676,9 @@ class global_symbol_searcher
>>    std::optional<compiled_regex> compile_type_regex () const;
>>
>>    /* Expand symtabs in OBJFILE that match NAME_REGEX, are of type
>> M_KIND.
>> -     Return true if any msymbols were seen that we should later consider
>> -     adding to the results list.  */
>> +     Return true if a matching function msymbol without debug info was
>> +     seen, meaning that msymbols should later be considered for the results
>> +     list.  */
> 
> If we update comment, what about updating the first sentence to
> 
> "Expand symtabs in OBJFILE that match NAME_REGEX and are of type M_KIND."
> 
> This reads better from my point of view.

Agreed, fixed.

> I was also wondering if it's worth updating the comments inside the "expand_symtabs"
> function:
> 
>   - [..] When no file names were given the caller unconditionally rescans [..]
> 
>      The "unconditionally" can be dropped.  It is gated in "search ()".

I'll do that.  Honestly, I'm not even sure what this part of the comment
brings.


> - The comment this contains "SEARCH_VAR_DOMAIN".  Should this be changed to
>    "symbol_search_kind:: VARIABLE" instead?  Not sure if this is the right patch for this
>    change.

I'll just say "for variables".

Simon
  

Patch

diff --git a/gdb/symtab.h b/gdb/symtab.h
index 50d7000681df..acbe41d131b7 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -2676,8 +2676,9 @@  class global_symbol_searcher
   std::optional<compiled_regex> compile_type_regex () const;
 
   /* Expand symtabs in OBJFILE that match NAME_REGEX, are of type M_KIND.
-     Return true if any msymbols were seen that we should later consider
-     adding to the results list.  */
+     Return true if a matching function msymbol without debug info was
+     seen, meaning that msymbols should later be considered for the results
+     list.  */
   bool expand_symtabs (objfile *objfile,
 		       const std::optional<compiled_regex> &name_regex) const;