[4/9] gdb: use a switch on m_kind in add_matching_symbols

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

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 fail Patch failed to apply

Commit Message

Simon Marchi Sept. 29, 2026, 7:39 p.m. UTC
  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

Simon Marchi Sept. 30, 2026, 7:41 p.m. UTC | #1
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
  

Patch

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;
 		}
-	      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)