Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@efficios.com>
To: gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@efficios.com>
Subject: [PATCH 2/9] gdb: factor out regexp compilation from global_symbol_searcher::search
Date: Tue, 29 Sep 2026 15:39:23 -0400	[thread overview]
Message-ID: <20260929194119.155169-3-simon.marchi@efficios.com> (raw)
In-Reply-To: <20260929194119.155169-1-simon.marchi@efficios.com>

Factor out the scopes that compile the regexps, in order to make
global_symbol_searcher::search itself simpler to read.

Also, I found the names "preg" and "treg" very unclear, so replace them
with "name_regex" and "type_regex".

Change-Id: Ie4485b2eb593967069ac635c933df817bc5304e6
---
 gdb/symtab.c | 171 +++++++++++++++++++++++++++++----------------------
 gdb/symtab.h |  43 +++++++------
 2 files changed, 122 insertions(+), 92 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 708df078e2ea..c4f76a494abd 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4745,7 +4745,8 @@ global_symbol_searcher::is_suitable_msymbol
 
 bool
 global_symbol_searcher::expand_symtabs
-	(objfile *objfile, const std::optional<compiled_regex> &preg) const
+	(objfile *objfile,
+	 const std::optional<compiled_regex> &name_regex) const
 {
   bool found_func_msymbol_without_debug_info = false;
 
@@ -4762,8 +4763,8 @@ global_symbol_searcher::expand_symtabs
      &lookup_name_info::match_any (),
      [&] (const char *symname)
      {
-       return (!preg.has_value ()
-	       || preg->exec (symname, 0, NULL, 0) == 0);
+       return (!name_regex.has_value ()
+	       || name_regex->exec (symname, 0, NULL, 0) == 0);
      },
      NULL,
      SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
@@ -4794,9 +4795,9 @@ global_symbol_searcher::expand_symtabs
 
 	  if (is_suitable_msymbol (m_kind, msymbol))
 	    {
-	      if (!preg.has_value ()
-		  || preg->exec (msymbol->natural_name (), 0,
-				 NULL, 0) == 0)
+	      if (!name_regex.has_value ()
+		  || name_regex->exec (msymbol->natural_name (), 0,
+				       NULL, 0) == 0)
 		{
 		  /* An important side-effect of this lookup function is
 		     to expand the symbol table if msymbol is found, later
@@ -4819,8 +4820,8 @@ global_symbol_searcher::expand_symtabs
 bool
 global_symbol_searcher::add_matching_symbols
 	(objfile *objfile,
-	 const std::optional<compiled_regex> &preg,
-	 const std::optional<compiled_regex> &treg,
+	 const std::optional<compiled_regex> &name_regex,
+	 const std::optional<compiled_regex> &type_regex,
 	 std::set<symbol_search> *result_set) const
 {
   domain_search_flags domain = to_search_flags (m_kind);
@@ -4854,14 +4855,15 @@ global_symbol_searcher::add_matching_symbols
 	      if (!sym->matches (domain))
 		continue;
 
-	      if (preg.has_value () && preg->exec (sym->natural_name (), 0,
-						   nullptr, 0) != 0)
+	      if (name_regex.has_value ()
+		  && name_regex->exec (sym->natural_name (), 0,
+				       nullptr, 0) != 0)
 		continue;
 
 	      if (((sym->domain () == VAR_DOMAIN
 		    || sym->domain () == FUNCTION_DOMAIN)
-		   && treg.has_value ()
-		   && !treg_matches_sym_type_name (*treg, sym)))
+		   && type_regex.has_value ()
+		   && !treg_matches_sym_type_name (*type_regex, sym)))
 		continue;
 
 	      if (m_kind == symbol_search_kind::VARIABLE)
@@ -4898,7 +4900,7 @@ global_symbol_searcher::add_matching_symbols
 
 bool
 global_symbol_searcher::add_matching_msymbols
-	(objfile *objfile, const std::optional<compiled_regex> &preg,
+	(objfile *objfile, const std::optional<compiled_regex> &name_regex,
 	 std::vector<symbol_search> *results) const
 {
   for (minimal_symbol *msymbol : objfile->msymbols ())
@@ -4910,9 +4912,9 @@ global_symbol_searcher::add_matching_msymbols
 
       if (is_suitable_msymbol (m_kind, msymbol))
 	{
-	  if (!preg.has_value ()
-	      || preg->exec (msymbol->natural_name (), 0,
-			     NULL, 0) == 0)
+	  if (!name_regex.has_value ()
+	      || name_regex->exec (msymbol->natural_name (), 0,
+				   NULL, 0) == 0)
 	    {
 	      /* For functions we can do a quick check of whether the
 		 symbol might be found via find_pc_symtab.  */
@@ -4938,65 +4940,84 @@ global_symbol_searcher::add_matching_msymbols
   return true;
 }
 
+/* Return the regcomp flags to use for the symbol name and symbol type
+   regexps.  */
+
+static int
+search_regex_cflags ()
+{
+  return REG_NOSUB | (case_sensitivity == case_sensitive_off ? REG_ICASE : 0);
+}
+
+/* See symtab.h.  */
+
+std::optional<compiled_regex>
+global_symbol_searcher::compile_name_regex () const
+{
+  if (m_symbol_name_regexp == nullptr)
+    return {};
+
+  const char *symbol_name_regexp = m_symbol_name_regexp;
+  std::string symbol_name_regexp_holder;
+
+  /* Make sure spacing is right for C++ operators.
+     This is just a courtesy to make the matching less sensitive
+     to how many spaces the user leaves between 'operator'
+     and <TYPENAME> or <OPERATOR>.  */
+  const char *op_end;
+  const char *opname = operator_chars (symbol_name_regexp, &op_end);
+
+  if (*opname)
+    {
+      /* -1 means ok; otherwise number of spaces needed.  */
+      int fix = -1;
+
+      if (c_isalpha (*opname) || *opname == '_' || *opname == '$')
+	{
+	  /* There should 1 space between 'operator' and 'TYPENAME'.  */
+	  if (opname[-1] != ' ' || opname[-2] == ' ')
+	    fix = 1;
+	}
+      else
+	{
+	  /* There should 0 spaces between 'operator' and 'OPERATOR'.  */
+	  if (opname[-1] == ' ')
+	    fix = 0;
+	}
+      /* If wrong number of spaces, fix it.  */
+      if (fix >= 0)
+	{
+	  symbol_name_regexp_holder
+	    = string_printf ("operator%.*s%s", fix, " ", opname);
+	  symbol_name_regexp = symbol_name_regexp_holder.c_str ();
+	}
+    }
+
+  return std::optional<compiled_regex> (std::in_place, symbol_name_regexp,
+					search_regex_cflags (),
+					_("Invalid regexp"));
+}
+
+/* See symtab.h.  */
+
+std::optional<compiled_regex>
+global_symbol_searcher::compile_type_regex () const
+{
+  if (m_symbol_type_regexp == nullptr)
+    return {};
+
+  return std::optional<compiled_regex> (std::in_place, m_symbol_type_regexp,
+					search_regex_cflags (),
+					_("Invalid regexp"));
+}
+
 /* See symtab.h.  */
 
 std::vector<symbol_search>
 global_symbol_searcher::search () const
 {
-  std::optional<compiled_regex> preg;
-  std::optional<compiled_regex> treg;
-
-  if (m_symbol_name_regexp != NULL)
-    {
-      const char *symbol_name_regexp = m_symbol_name_regexp;
-      std::string symbol_name_regexp_holder;
-
-      /* Make sure spacing is right for C++ operators.
-	 This is just a courtesy to make the matching less sensitive
-	 to how many spaces the user leaves between 'operator'
-	 and <TYPENAME> or <OPERATOR>.  */
-      const char *op_end;
-      const char *opname = operator_chars (symbol_name_regexp, &op_end);
-
-      if (*opname)
-	{
-	  int fix = -1;		/* -1 means ok; otherwise number of
-				    spaces needed.  */
-
-	  if (c_isalpha (*opname) || *opname == '_' || *opname == '$')
-	    {
-	      /* There should 1 space between 'operator' and 'TYPENAME'.  */
-	      if (opname[-1] != ' ' || opname[-2] == ' ')
-		fix = 1;
-	    }
-	  else
-	    {
-	      /* There should 0 spaces between 'operator' and 'OPERATOR'.  */
-	      if (opname[-1] == ' ')
-		fix = 0;
-	    }
-	  /* If wrong number of spaces, fix it.  */
-	  if (fix >= 0)
-	    {
-	      symbol_name_regexp_holder
-		= string_printf ("operator%.*s%s", fix, " ", opname);
-	      symbol_name_regexp = symbol_name_regexp_holder.c_str ();
-	    }
-	}
-
-      int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
-				? REG_ICASE : 0);
-      preg.emplace (symbol_name_regexp, cflags,
-		    _("Invalid regexp"));
-    }
-
-  if (m_symbol_type_regexp != NULL)
-    {
-      int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
-				? REG_ICASE : 0);
-      treg.emplace (m_symbol_type_regexp, cflags,
-		    _("Invalid regexp"));
-    }
+  std::optional<compiled_regex> name_regex = compile_name_regex ();
+  std::optional<compiled_regex> type_regex = compile_type_regex ();
 
   bool found_func_msymbol_without_debug_info = false;
   std::set<symbol_search> result_set;
@@ -5004,13 +5025,15 @@ global_symbol_searcher::search () const
     {
       /* Expand symtabs within objfile that possibly contain matching
 	 symbols.  */
-      found_func_msymbol_without_debug_info |= expand_symtabs (&objfile, preg);
+      found_func_msymbol_without_debug_info
+	|= expand_symtabs (&objfile, name_regex);
 
       /* Find matching symbols within OBJFILE and add them in to the
 	 RESULT_SET set.  Use a set here so that we can easily detect
 	 duplicates as we go, and can therefore track how many unique
 	 matches we have found so far.  */
-      if (!add_matching_symbols (&objfile, preg, treg, &result_set))
+      if (!add_matching_symbols (&objfile, name_regex, type_regex,
+				 &result_set))
 	break;
     }
 
@@ -5025,12 +5048,12 @@ global_symbol_searcher::search () const
   if ((found_func_msymbol_without_debug_info
        || (m_filenames.empty () && m_kind == symbol_search_kind::VARIABLE))
       && !m_exclude_minsyms
-      && !treg.has_value ())
+      && !type_regex.has_value ())
     {
       gdb_assert (m_kind == symbol_search_kind::VARIABLE
 		  || m_kind == symbol_search_kind::FUNCTION);
       for (objfile &objfile : current_program_space->objfiles ())
-	if (!add_matching_msymbols (&objfile, preg, &result))
+	if (!add_matching_msymbols (&objfile, name_regex, &result))
 	  break;
     }
 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index a35449fbeeda..1675b126b19d 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -2669,31 +2669,38 @@ class global_symbol_searcher
      of SIZE_MAX, there is no "unlimited".  */
   size_t m_max_search_results = SIZE_MAX;
 
-  /* Expand symtabs in OBJFILE that match PREG, are of type M_KIND.  Return
-     true if any msymbols were seen that we should later consider adding to
-     the results list.  */
-  bool expand_symtabs (objfile *objfile,
-		       const std::optional<compiled_regex> &preg) const;
+  /* Compile M_SYMBOL_NAME_REGEXP, if set.  */
+  std::optional<compiled_regex> compile_name_regex () const;
 
-  /* Add symbols from symtabs in OBJFILE that match PREG, and TREG, and are
-     of type M_KIND, to the results set RESULTS_SET.  Return false if we
-     stop adding results early due to having already found too many results
-     (based on M_MAX_SEARCH_RESULTS limit), otherwise return true.
+  /* Compile M_SYMBOL_TYPE_REGEXP, if set.  */
+  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.  */
+  bool expand_symtabs (objfile *objfile,
+		       const std::optional<compiled_regex> &name_regex) const;
+
+  /* Add symbols from symtabs in OBJFILE that match NAME_REGEX, and
+     TYPE_REGEX, and are of type M_KIND, to the results set RESULTS_SET.
+     Return false if we stop adding results early due to having already
+     found too many results (based on M_MAX_SEARCH_RESULTS limit),
+     otherwise return true.
      Returning true does not indicate that any results were added, just
      that we didn't _not_ add a result due to reaching MAX_SEARCH_RESULTS.  */
   bool add_matching_symbols (objfile *objfile,
-			     const std::optional<compiled_regex> &preg,
-			     const std::optional<compiled_regex> &treg,
+			     const std::optional<compiled_regex> &name_regex,
+			     const std::optional<compiled_regex> &type_regex,
 			     std::set<symbol_search> *result_set) const;
 
-  /* Add msymbols from OBJFILE that match PREG and M_KIND, to the results
-     vector RESULTS.  Return false if we stop adding results early due to
-     having already found too many results (based on max search results
-     limit M_MAX_SEARCH_RESULTS), otherwise return true.  Returning true
-     does not indicate that any results were added, just that we didn't
-     _not_ add a result due to reaching MAX_SEARCH_RESULTS.  */
+  /* Add msymbols from OBJFILE that match NAME_REGEX and M_KIND, to the
+     results vector RESULTS.  Return false if we stop adding results early
+     due to having already found too many results (based on max search
+     results limit M_MAX_SEARCH_RESULTS), otherwise return true.  Returning
+     true does not indicate that any results were added, just that we
+     didn't _not_ add a result due to reaching MAX_SEARCH_RESULTS.  */
   bool add_matching_msymbols (objfile *objfile,
-			      const std::optional<compiled_regex> &preg,
+			      const std::optional<compiled_regex> &name_regex,
 			      std::vector<symbol_search> *results) const;
 
   /* Return true if MSYMBOL is of type KIND.  */
-- 
2.55.0


  parent reply	other threads:[~2026-09-29 19:41 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-29 19:39 [PATCH 0/9] Cleanups in global_symbol_searcher Simon Marchi
2026-09-29 19:39 ` [PATCH 1/9] gdb: add symbol_search_kind enum for global_symbol_searcher Simon Marchi
2026-09-29 19:39 ` Simon Marchi [this message]
2026-09-29 19:39 ` [PATCH 3/9] gdb: simplify insertion in global_symbol_searcher::add_matching_symbols Simon Marchi
2026-09-29 19:39 ` [PATCH 4/9] gdb: use a switch on m_kind in add_matching_symbols Simon Marchi
2026-09-29 19:39 ` [PATCH 5/9] gdb: make global_symbol_searcher::is_suitable_msymbol non-static Simon Marchi
2026-09-29 19:39 ` [PATCH 6/9] gdb: update comment of global_symbol_searcher::expand_symtabs Simon Marchi
2026-09-29 19:39 ` [PATCH 7/9] gdb: add matching helpers to global_symbol_searcher Simon Marchi
2026-09-29 19:39 ` [PATCH 8/9] gdb: pass result containers by reference in global_symbol_searcher Simon Marchi
2026-09-29 19:39 ` [PATCH 9/9] gdb: fix some comments " Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260929194119.155169-3-simon.marchi@efficios.com \
    --to=simon.marchi@efficios.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox