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 03/11] gdb: introduce iteration_status enum, use it for search callbacks
Date: Thu, 16 Apr 2026 16:16:13 -0400	[thread overview]
Message-ID: <20260416202408.422441-4-simon.marchi@efficios.com> (raw)
In-Reply-To: <20260416202408.422441-1-simon.marchi@efficios.com>

There are a bunch of iteration functions that take a callback returning
true or false to indicate whether to continue or stop iterating.  These
functions then return the same value, indicate whether the iteration was
done until the end of interrupted.  I think this is confusing and
error-prone, as I never know which value means what.  It is especially
confusing when two opposite conventions collide, such as in
objfile::map_symtabs_matching_filename.

I propose to make that more obvious by introducing a new
iteration_status enum with self-documenting values.

I started to change the callback type
compunit_symtab_iteration_callback, taken by
quick_symbol_functions::search, and then followed that path to update a
bunch of other functions.

I chose the name to be kind of generic, so that it can be used for other
similar iteration patterns.  I also put it in gdbsupport, in case we
want to use it in gdbserver too.

Change-Id: I55d84d0c1af8ac0b82cc9f49ccf0d6b60e1769e0
---
 gdb/ada-lang.c                  |  6 +--
 gdb/cp-support.c                |  2 +-
 gdb/dwarf2/cooked-index-entry.c |  2 +-
 gdb/dwarf2/cooked-index-entry.h | 10 ++--
 gdb/dwarf2/cooked-index.h       |  2 +-
 gdb/dwarf2/index-write.c        |  4 +-
 gdb/dwarf2/read.c               | 66 +++++++++++++----------
 gdb/dwarf2/read.h               | 20 ++++---
 gdb/expanded-symbol.c           | 10 ++--
 gdb/expanded-symbol.h           | 15 +++---
 gdb/linespec.c                  |  4 +-
 gdb/objfiles.h                  | 19 ++++---
 gdb/python/py-symbol.c          | 18 ++++---
 gdb/quick-symbol.h              | 18 +++----
 gdb/symfile-debug.c             | 95 ++++++++++++++++++---------------
 gdb/symtab.c                    | 19 ++++---
 gdb/symtab.h                    |  6 ++-
 gdbsupport/iteration-status.h   | 48 +++++++++++++++++
 18 files changed, 224 insertions(+), 140 deletions(-)
 create mode 100644 gdbsupport/iteration-status.h

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 805999e18eea..f8ed4cc5e102 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -5568,7 +5568,7 @@ map_matching_symbols (struct objfile *objfile,
       bool result = iterate_over_symbols (block, lookup_name, domain, data);
       gdb_assert (result);
       data.finish (block);
-      return true;
+      return iteration_status::keep_going;
     };
 
   objfile->search (nullptr, &lookup_name, nullptr, callback,
@@ -13207,7 +13207,7 @@ ada_add_global_exceptions (compiled_regex *preg,
 		  }
 	    }
 
-	  return true;
+	  return iteration_status::keep_going;
 	};
 
       /* In Ada, the symbol "search name" is a linkage name, whereas
@@ -13883,7 +13883,7 @@ class ada_language : public language_defn
 		  }
 	      }
 
-	    return true;
+	    return iteration_status::keep_going;
 	  };
 
 	objfile.search
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index d321986f72da..d040ad79469a 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -1490,7 +1490,7 @@ add_symbol_overload_list_qualified (const char *func_name,
 	     /* Don't do this block twice.  */
 	     if (b != surrounding_static_block)
 	       add_symbol_overload_list_block (func_name, b, overload_list);
-	     return true;
+	     return iteration_status::keep_going;
 	   };
 
 	 /* Look through the partial symtabs for all symbols which
diff --git a/gdb/dwarf2/cooked-index-entry.c b/gdb/dwarf2/cooked-index-entry.c
index 2f61e5c4c57b..21e25e946158 100644
--- a/gdb/dwarf2/cooked-index-entry.c
+++ b/gdb/dwarf2/cooked-index-entry.c
@@ -253,7 +253,7 @@ cooked_index_entry::force_set_language () const
 
 /* See cooked-index-entry.h.  */
 
-bool
+iteration_status
 cooked_index_entry::visit_defining_cus (per_cu_callback callback) const
 {
   if ((flags & IS_INLINED) == 0)
diff --git a/gdb/dwarf2/cooked-index-entry.h b/gdb/dwarf2/cooked-index-entry.h
index 9eed2ca9c46c..9f7a7c42de4f 100644
--- a/gdb/dwarf2/cooked-index-entry.h
+++ b/gdb/dwarf2/cooked-index-entry.h
@@ -232,12 +232,14 @@ struct cooked_index_entry : public allocate_on_obstack<cooked_index_entry>
   void force_set_language () const;
 
   /* Type of callback used when visiting defining CUs.  */
-  using per_cu_callback = gdb::function_view<bool (dwarf2_per_cu *)>;
+  using per_cu_callback
+    = gdb::function_view<iteration_status (dwarf2_per_cu *)>;
 
   /* Calls CALLBACK for each CU that "defines" this entry.
 
-     If any call to CALLBACK returns false, this immediately returns
-     false (skipping the remaining calls); otherwise returns true.
+     If any call to CALLBACK returns iteration_status::stop, this
+     immediately returns iteration_status::stop (skipping the remaining
+     calls); otherwise returns iteration_status::keep_going.
 
      For most entries, there is a single defining CU, which is the
      canonical outermost includer of the CU holding the entry.  In the
@@ -248,7 +250,7 @@ struct cooked_index_entry : public allocate_on_obstack<cooked_index_entry>
      such outermost includer -- if a subroutine is inlined in many
      places, and then "dwz" is run, the IS_INLINED subroutine may be
      defined in some CU that is included by many other CUs.  */
-  bool visit_defining_cus (per_cu_callback callback) const;
+  iteration_status visit_defining_cus (per_cu_callback callback) const;
 
   /* The name as it appears in DWARF.  This always points into one of
      the mapped DWARF sections.  Note that this may be the name or the
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 00a15661677b..2e177cb62dd2 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -237,7 +237,7 @@ struct cooked_index_functions : public dwarf2_base_index_functions
     dwarf2_base_index_functions::expand_all_symtabs (objfile);
   }
 
-  bool search
+  iteration_status search
     (struct objfile *objfile,
      search_symtabs_file_matcher file_matcher,
      const lookup_name_info *lookup_name,
diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index 0f940920862f..6f8e41ff96fa 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -721,7 +721,7 @@ class debug_names
 	    entry->visit_defining_cus ([&] (dwarf2_per_cu *one_cu)
 	      {
 		per_cus.push_back (one_cu);
-		return true;
+		return iteration_status::keep_going;
 	      });
 	    /* Make sure the output is stable.  */
 	    std::sort (per_cus.begin (), per_cus.end (),
@@ -1317,7 +1317,7 @@ write_cooked_index (cooked_index *table,
 	  gdb_assert (it != cu_index_htab.cend ());
 	  symtab->add_index_entry (name, (entry->flags & IS_STATIC) != 0,
 				   kind, it->second);
-	  return true;
+	  return iteration_status::keep_going;
 	});
     }
 }
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 6146ff78b678..48e89d10eac7 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1453,14 +1453,14 @@ struct readnow_functions : public dwarf2_base_index_functions
   {
   }
 
-  bool search (struct objfile *objfile,
-	       search_symtabs_file_matcher file_matcher,
-	       const lookup_name_info *lookup_name,
-	       search_symtabs_symbol_matcher symbol_matcher,
-	       compunit_symtab_iteration_callback compunit_callback,
-	       block_search_flags search_flags,
-	       domain_search_flags domain,
-	       search_symtabs_lang_matcher lang_matcher) override
+  iteration_status search (struct objfile *objfile,
+			   search_symtabs_file_matcher file_matcher,
+			   const lookup_name_info *lookup_name,
+			   search_symtabs_symbol_matcher symbol_matcher,
+			   compunit_symtab_iteration_callback compunit_callback,
+			   block_search_flags search_flags,
+			   domain_search_flags domain,
+			   search_symtabs_lang_matcher lang_matcher) override
   {
     dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
     auto_bool_vector cus_to_skip;
@@ -1481,11 +1481,13 @@ struct readnow_functions : public dwarf2_base_index_functions
 	    || per_objfile->get_compunit_symtab (per_cu.get ()) == nullptr)
 	  continue;
 
-	if (!search_one (per_cu.get (), per_objfile, cus_to_skip,
-			 compunit_callback, lang_matcher))
-	  return false;
+	if (search_one (per_cu.get (), per_objfile, cus_to_skip,
+			compunit_callback, lang_matcher)
+	    == iteration_status::stop)
+	  return iteration_status::stop;
       }
-    return true;
+
+    return iteration_status::keep_going;
   }
 
   struct symbol *find_symbol_by_address (struct objfile *objfile,
@@ -1919,7 +1921,7 @@ dwarf2_base_index_functions::expand_all_symtabs (struct objfile *objfile)
 
 /* See read.h.  */
 
-bool
+iteration_status
 dwarf2_base_index_functions::search_one
   (dwarf2_per_cu *per_cu,
    dwarf2_per_objfile *per_objfile,
@@ -1929,7 +1931,7 @@ dwarf2_base_index_functions::search_one
 {
   /* Already visited, or intentionally skipped.  */
   if (cus_to_skip.is_set (per_cu->index))
-    return true;
+    return iteration_status::keep_going;
 
   if (lang_matcher != nullptr)
     {
@@ -1937,7 +1939,7 @@ dwarf2_base_index_functions::search_one
       per_cu->ensure_lang (per_objfile);
       if (!per_cu->maybe_multi_language ()
 	  && !lang_matcher (per_cu->lang ()))
-	return true;
+	return iteration_status::keep_going;
     }
 
   compunit_symtab *symtab
@@ -1950,7 +1952,7 @@ dwarf2_base_index_functions::search_one
       return compunit_callback (symtab);
     }
 
-  return true;
+  return iteration_status::keep_going;
 }
 
 /* If FILE_MATCHER is non-NULL, update CUS_TO_SKIP as appropriate
@@ -14030,7 +14032,7 @@ cooked_index_functions::find_symbol_by_address
   return cu->symbol_at_address (address);
 }
 
-bool
+iteration_status
 cooked_index_functions::search
   (objfile *objfile,
    search_symtabs_file_matcher file_matcher,
@@ -14056,11 +14058,13 @@ cooked_index_functions::search
 	{
 	  QUIT;
 
-	  if (!search_one (per_cu, per_objfile, cus_to_skip, compunit_callback,
-			   lang_matcher))
-	    return false;
+	  if (search_one (per_cu, per_objfile, cus_to_skip, compunit_callback,
+			  lang_matcher)
+	      == iteration_status::stop)
+	    return iteration_status::stop;
 	}
-      return true;
+
+      return iteration_status::keep_going;
     }
 
   lookup_name_info lookup_name_without_params
@@ -14222,17 +14226,19 @@ cooked_index_functions::search
 	  else if (!symbol_matcher (full_name))
 	    continue;
 
-	  bool check = entry->visit_defining_cus ([&] (dwarf2_per_cu *per_cu)
+	  iteration_status status
+	    = entry->visit_defining_cus ([&] (dwarf2_per_cu *per_cu)
 	    {
 	      return search_one (per_cu, per_objfile, cus_to_skip,
 				 compunit_callback, nullptr);
 	    });
-	  if (!check)
-	    return false;
+
+	  if (status == iteration_status::stop)
+	    return iteration_status::stop;
 	}
     }
 
-  return true;
+  return iteration_status::keep_going;
 }
 
 /* Start reading .debug_info using the indexer.  */
@@ -17989,15 +17995,17 @@ dwarf2_per_cu::ensure_lang (dwarf2_per_objfile *per_objfile)
 
 /* See read.h.  */
 
-bool
+iteration_status
 dwarf2_per_cu::recursively_visit_cus (per_cu_callback callback)
 {
   if (including_cus.empty ())
     return callback (this);
+
   for (dwarf2_per_cu *iter : including_cus)
-    if (!iter->recursively_visit_cus (callback))
-      return false;
-  return true;
+    if (iter->recursively_visit_cus (callback) == iteration_status::stop)
+      return iteration_status::stop;
+
+  return iteration_status::keep_going;
 }
 
 /* See read.h.  */
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 40b55a33bc8d..c65ee7f86990 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -32,6 +32,7 @@
 #include "gdbsupport/cxx-thread.h"
 #include "gdbsupport/gdb_obstack.h"
 #include "gdbsupport/function-view.h"
+#include "gdbsupport/iteration-status.h"
 #include "gdbsupport/packed.h"
 
 /* Hold 'maintenance (set|show) dwarf' commands.  */
@@ -429,13 +430,15 @@ struct dwarf2_per_cu
   }
 
   /* Type of callback used when visiting defining CUs.  */
-  using per_cu_callback = gdb::function_view<bool (dwarf2_per_cu *)>;
+  using per_cu_callback
+    = gdb::function_view<iteration_status (dwarf2_per_cu *)>;
 
   /* Calls CALLBACK for each CU that is the outermost includer of
      ONE_CU.  If ONE_CU has no includers, it calls CALLBACK on ONE_CU.
-     If any call to CALLBACK returns false, this immediately returns
-     false (skipping the remaining calls); otherwise returns true.  */
-  bool recursively_visit_cus (per_cu_callback callback);
+     If any call to CALLBACK returns iteration_status::stop, this
+     immediately returns iteration_status::stop (skipping the remaining
+     calls); otherwise returns iteration_status::keep_going.  */
+  iteration_status recursively_visit_cus (per_cu_callback callback);
 
   /* Return a canonical outermost CU corresponding to this CU.  If
      this CU is standalone (not included by other CUs), then this
@@ -1321,10 +1324,11 @@ struct dwarf2_base_index_functions : public quick_symbol_functions
   /* If CUS_TO_SKIP does not include the CU's index and the CU's language
      matches LANG_MATCHER, expand the CU and call COMPUNIT_CALLBACK (if
      provided) on it.  */
-  bool search_one (dwarf2_per_cu *per_cu, dwarf2_per_objfile *per_objfile,
-		   auto_bool_vector &cus_to_skip,
-		   compunit_symtab_iteration_callback compunit_callback,
-		   search_symtabs_lang_matcher lang_matcher);
+  iteration_status search_one
+    (dwarf2_per_cu *per_cu, dwarf2_per_objfile *per_objfile,
+     auto_bool_vector &cus_to_skip,
+     compunit_symtab_iteration_callback compunit_callback,
+     search_symtabs_lang_matcher lang_matcher);
 };
 
 /* Return pointer to string at .debug_str offset STR_OFFSET.  */
diff --git a/gdb/expanded-symbol.c b/gdb/expanded-symbol.c
index fee55cd23db3..6dcb4047e3fe 100644
--- a/gdb/expanded-symbol.c
+++ b/gdb/expanded-symbol.c
@@ -47,7 +47,7 @@ expanded_symbols_functions::lookup_global_symbol_language
 
 /* See expanded-symbol.h.  */
 
-bool
+iteration_status
 expanded_symbols_functions::search
      (objfile *objfile,
       search_symtabs_file_matcher file_matcher,
@@ -92,10 +92,12 @@ expanded_symbols_functions::search
 	 consult lookup_name and symbol_matcher (if any).  This should be
 	 okay since i) all symtabs are already expanded and ii) callbacks
 	 iterate over matching symbols themselves.  */
-      if (compunit_callback != nullptr && !compunit_callback (cu))
-	return false;
+      if (compunit_callback != nullptr
+	  && compunit_callback (cu) == iteration_status::stop)
+	return iteration_status::stop;
     }
-  return true;
+
+  return iteration_status::keep_going;
 }
 
 /* See expanded-symbol.h.  */
diff --git a/gdb/expanded-symbol.h b/gdb/expanded-symbol.h
index fc42938d9d3d..68f5532e6b8c 100644
--- a/gdb/expanded-symbol.h
+++ b/gdb/expanded-symbol.h
@@ -66,13 +66,14 @@ struct expanded_symbols_functions : public quick_symbol_functions
   {
   }
 
-  bool search (objfile *objfile,
-	       search_symtabs_file_matcher file_matcher,
-	       const lookup_name_info *lookup_name,
-	       search_symtabs_symbol_matcher symbol_matcher,
-	       compunit_symtab_iteration_callback compunit_callback,
-	       block_search_flags search_flags, domain_search_flags domain,
-	       search_symtabs_lang_matcher lang_matcher) override;
+  iteration_status search (objfile *objfile,
+			   search_symtabs_file_matcher file_matcher,
+			   const lookup_name_info *lookup_name,
+			   search_symtabs_symbol_matcher symbol_matcher,
+			   compunit_symtab_iteration_callback compunit_callback,
+			   block_search_flags search_flags,
+			   domain_search_flags domain,
+			   search_symtabs_lang_matcher lang_matcher) override;
 
   compunit_symtab *find_pc_sect_compunit_symtab
     (objfile *objfile, bound_minimal_symbol msymbol, CORE_ADDR pc,
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 981bd5faa4fc..4ea6d597a093 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1169,7 +1169,7 @@ iterate_over_all_matching_symtabs
 		    }
 		}
 
-	      return true;
+	      return iteration_status::keep_going;
 	    };
 
 	  objfile.search (nullptr, &lookup_name, nullptr, expand_callback,
@@ -3613,7 +3613,7 @@ collect_symtabs_from_filename (const char *file,
     {
       if (symtab_table.insert (symtab).second)
 	symtabs.push_back (symtab);
-      return false;
+      return iteration_status::keep_going;
     };
 
   /* Find that file's data.  */
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index aafac6ac93c6..8ae54b37bb79 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -583,9 +583,9 @@ struct objfile : intrusive_list_node<objfile>
      Then, this calls iterate_over_some_symtabs (or equivalent) over
      all newly-created symbol tables, passing CALLBACK to it.
      The result of this call is returned.  */
-  bool map_symtabs_matching_filename
+  iteration_status map_symtabs_matching_filename
     (const char *name, const char *real_path,
-     gdb::function_view<bool (symtab *)> callback);
+     gdb::function_view<iteration_status (symtab *)> callback);
 
   /* Check to see if the symbol is defined in a "partial" symbol table
      of this objfile.  BLOCK_INDEX should be either GLOBAL_BLOCK or
@@ -618,14 +618,13 @@ struct objfile : intrusive_list_node<objfile>
   void expand_symtabs_with_fullname (const char *fullname);
 
   /* See quick_symbol_functions.  */
-  bool search
-    (search_symtabs_file_matcher file_matcher,
-     const lookup_name_info *lookup_name,
-     search_symtabs_symbol_matcher symbol_matcher,
-     compunit_symtab_iteration_callback compunit_callback,
-     block_search_flags search_flags,
-     domain_search_flags domain,
-     search_symtabs_lang_matcher lang_matcher = nullptr);
+  iteration_status search (search_symtabs_file_matcher file_matcher,
+			   const lookup_name_info *lookup_name,
+			   search_symtabs_symbol_matcher symbol_matcher,
+			   compunit_symtab_iteration_callback compunit_callback,
+			   block_search_flags search_flags,
+			   domain_search_flags domain,
+			   search_symtabs_lang_matcher lang_matcher = nullptr);
 
   /* See quick_symbol_functions.  */
   struct compunit_symtab *
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index e74c8e4b3680..6293b658ea1d 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -599,7 +599,7 @@ gdbpy_lookup_static_symbols (PyObject *self, PyObject *args, PyObject *kw)
 	      /* Skip included compunits to prevent including compunits from
 		 being searched twice.  */
 	      if (cust->user != nullptr)
-		return true;
+		return iteration_status::keep_going;
 
 	      const struct blockvector *bv = cust->blockvector ();
 	      const struct block *block = bv->static_block ();
@@ -615,16 +615,22 @@ gdbpy_lookup_static_symbols (PyObject *self, PyObject *args, PyObject *kw)
 		      if (sym_obj == nullptr
 			  || PyList_Append (return_list.get (),
 					    sym_obj.get ()) == -1)
-			return false;
+			return iteration_status::stop;
 		    }
 		}
 
-	      return true;
+	      return iteration_status::keep_going;
 	    };
 
-	  if (!objfile.search (nullptr, &lookup_name, nullptr, callback,
-			       SEARCH_STATIC_BLOCK, flags))
-	    return nullptr;
+	  /* The only reason why the iteration would stop is if an error was
+	     encountered during the callback execution.  */
+	  if (objfile.search (nullptr, &lookup_name, nullptr, callback,
+			      SEARCH_STATIC_BLOCK, flags)
+	      == iteration_status::stop)
+	    {
+	      gdb_assert (PyErr_Occurred ());
+	      return nullptr;
+	    }
 	}
     }
   catch (const gdb_exception &except)
diff --git a/gdb/quick-symbol.h b/gdb/quick-symbol.h
index 9cecc1bf52c1..2d1ca78293dd 100644
--- a/gdb/quick-symbol.h
+++ b/gdb/quick-symbol.h
@@ -21,6 +21,7 @@
 #define GDB_QUICK_SYMBOL_H
 
 #include "symtab.h"
+#include "gdbsupport/iteration-status.h"
 
 /* Like block_enum, but used as flags to pass to lookup functions.  */
 
@@ -56,12 +57,10 @@ using search_symtabs_lang_matcher
   = gdb::function_view<bool (enum language lang)>;
 
 /* Callback for quick_symbol_functions::search to be called when a
-   compunit_symtab matches (perhaps expanding it first).  If this
-   returns true, more compunit_symtabs are checked; if it returns false,
-   iteration stops.  */
+   compunit_symtab matches (perhaps expanding it first).  */
 
 using compunit_symtab_iteration_callback
-  = gdb::function_view<bool (compunit_symtab *symtab)>;
+  = gdb::function_view<iteration_status (compunit_symtab *symtab)>;
 
 /* The "quick" symbol functions exist so that symbol readers can
    avoiding an initial read of all the symbols.  For example, symbol
@@ -153,11 +152,12 @@ struct quick_symbol_functions
 
      Then (regardless of whether the symbol table was already
      expanded, or just expanded in response to this search),
-     COMPUNIT_CALLBACK is called.  If COMPUNIT_CALLBACK returns false,
-     execution stops and this method returns false.  Otherwise, more
-     files are considered.  This method returns true if all calls to
-     COMPUNIT_CALLBACK return true.  */
-  virtual bool search
+     COMPUNIT_CALLBACK is called.  If COMPUNIT_CALLBACK returns
+     iteration_status::stop, execution stops and this method returns
+     iteration_status::stop.  Otherwise, more files are considered.  This
+     method returns iteration_status::keep_going if all calls to
+     COMPUNIT_CALLBACK return iteration_status::keep_going.  */
+  virtual iteration_status search
     (struct objfile *objfile,
      search_symtabs_file_matcher file_matcher,
      const lookup_name_info *lookup_name,
diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
index d26662e20656..8e4892649508 100644
--- a/gdb/symfile-debug.c
+++ b/gdb/symfile-debug.c
@@ -170,19 +170,18 @@ objfile::forget_cached_source_info ()
    CUST indicates which compunit symtab to search.  Each symtab within
    the specified compunit symtab is also searched.  */
 
-static bool
-iterate_over_one_compunit_symtab (const char *base_name,
-				  const char *name,
-				  const char *real_path,
-				  compunit_symtab *cust,
-				  gdb::function_view<bool (symtab *)> callback)
+static iteration_status
+iterate_over_one_compunit_symtab
+  (const char *base_name, const char *name, const char *real_path,
+   compunit_symtab *cust,
+   gdb::function_view<iteration_status (symtab *)> callback)
 {
   for (symtab *s : cust->filetabs ())
     {
       if (compare_filenames_for_search (s->filename (), name))
 	{
-	  if (callback (s))
-	    return true;
+	  if (callback (s) == iteration_status::stop)
+	    return iteration_status::stop;
 	  continue;
 	}
 
@@ -194,8 +193,8 @@ iterate_over_one_compunit_symtab (const char *base_name,
 
       if (compare_filenames_for_search (symtab_to_fullname (s), name))
 	{
-	  if (callback (s))
-	    return true;
+	  if (callback (s) == iteration_status::stop)
+	    return iteration_status::stop;
 	  continue;
 	}
 
@@ -212,25 +211,26 @@ iterate_over_one_compunit_symtab (const char *base_name,
 	  fullname = fullname_real_path.get ();
 	  if (FILENAME_CMP (real_path, fullname) == 0)
 	    {
-	      if (callback (s))
-		return true;
+	      if (callback (s) == iteration_status::stop)
+		return iteration_status::stop;
 	      continue;
 	    }
 	}
     }
 
   for (compunit_symtab *iter : cust->includes)
-    if (iterate_over_one_compunit_symtab (base_name, name, real_path,
-					  iter, callback))
-      return true;
+    if (iterate_over_one_compunit_symtab (base_name, name, real_path, iter,
+					  callback)
+	== iteration_status::stop)
+      return iteration_status::stop;
 
-  return false;
+  return iteration_status::keep_going;
 }
 
-bool
+iteration_status
 objfile::map_symtabs_matching_filename
   (const char *name, const char *real_path,
-   gdb::function_view<bool (symtab *)> callback)
+   gdb::function_view<iteration_status (symtab *)> callback)
 {
   if (debug_symfile)
     gdb_printf (gdb_stdlog,
@@ -240,7 +240,6 @@ objfile::map_symtabs_matching_filename
 		real_path ? real_path : NULL,
 		host_address_to_string (&callback));
 
-  bool retval = false;
   const char *name_basename = lbasename (name);
 
   auto match_one_filename = [&] (const char *filename, bool basenames)
@@ -260,30 +259,37 @@ objfile::map_symtabs_matching_filename
     /* Skip included compunits, as they are searched by
        iterate_over_one_compunit_symtab.  */
     if (symtab->user != nullptr)
-      return true;
+      return iteration_status::keep_going;
 
-    /* CALLBACK returns false to keep going and true to continue, so
-       we have to invert the result here, for search.  */
-    return !iterate_over_one_compunit_symtab (name_basename, name, real_path,
-					      symtab, callback);
+    /* iterate_over_one_compunit_symtab returns true to stop,
+       convert to iteration_status.  */
+    if (iterate_over_one_compunit_symtab (name_basename, name, real_path,
+					  symtab, callback)
+	== iteration_status::stop)
+      return iteration_status::stop;
+
+    return iteration_status::keep_going;
   };
 
+  iteration_status retval = iteration_status::keep_going;
+
   for (const auto &iter : qf)
     {
-      if (!iter->search (this, match_one_filename, nullptr, nullptr,
-			 compunit_callback,
-			 SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
-			 SEARCH_ALL_DOMAINS))
+      if (iter->search (this, match_one_filename, nullptr, nullptr,
+			compunit_callback,
+			SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
+			SEARCH_ALL_DOMAINS)
+	  == iteration_status::stop)
 	{
-	  retval = true;
+	  retval = iteration_status::stop;
 	  break;
 	}
     }
 
   if (debug_symfile)
     gdb_printf (gdb_stdlog,
-		"qf->map_symtabs_matching_filename (...) = %d\n",
-		retval);
+		"qf->map_symtabs_matching_filename (...) = %s\n",
+		iteration_status_str (retval));
 
   return retval;
 }
@@ -316,22 +322,23 @@ objfile::lookup_symbol (block_enum kind, const lookup_name_info &name,
       {
 	retval = stab;
 	/* Found it.  */
-	return false;
+	return iteration_status::stop;
       }
     if (with_opaque != nullptr)
       retval = stab;
 
     /* Keep looking through other psymtabs.  */
-    return true;
+    return iteration_status::keep_going;
   };
 
   for (const auto &iter : qf)
     {
-      if (!iter->search (this, nullptr, &name, nullptr, search_one_symtab,
-			 kind == GLOBAL_BLOCK
-			 ? SEARCH_GLOBAL_BLOCK
-			 : SEARCH_STATIC_BLOCK,
-			 domain))
+      if (iter->search (this, nullptr, &name, nullptr, search_one_symtab,
+			kind == GLOBAL_BLOCK
+			? SEARCH_GLOBAL_BLOCK
+			: SEARCH_STATIC_BLOCK,
+			domain)
+	  == iteration_status::stop)
 	break;
     }
 
@@ -397,7 +404,7 @@ objfile::expand_symtabs_with_fullname (const char *fullname)
 		  SEARCH_ALL_DOMAINS);
 }
 
-bool
+iteration_status
 objfile::search (search_symtabs_file_matcher file_matcher,
 		 const lookup_name_info *lookup_name,
 		 search_symtabs_symbol_matcher symbol_matcher,
@@ -419,10 +426,12 @@ objfile::search (search_symtabs_file_matcher file_matcher,
 		domain_name (domain).c_str ());
 
   for (const auto &iter : qf)
-    if (!iter->search (this, file_matcher, lookup_name, symbol_matcher,
-		       compunit_callback, search_flags, domain, lang_matcher))
-      return false;
-  return true;
+    if (iter->search (this, file_matcher, lookup_name, symbol_matcher,
+		      compunit_callback, search_flags, domain, lang_matcher)
+	== iteration_status::stop)
+      return iteration_status::stop;
+
+  return iteration_status::keep_going;
 }
 
 struct compunit_symtab *
diff --git a/gdb/symtab.c b/gdb/symtab.c
index a6b145e5331b..d22dd81a246a 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -633,7 +633,7 @@ compare_filenames_for_search (const char *filename, const char *search_name)
 
 void
 iterate_over_symtabs (program_space *pspace, const char *name,
-		      gdb::function_view<bool (symtab *)> callback)
+		      gdb::function_view<iteration_status (symtab *)> callback)
 {
   gdb::unique_xmalloc_ptr<char> real_path;
 
@@ -647,7 +647,8 @@ iterate_over_symtabs (program_space *pspace, const char *name,
 
   for (objfile &objfile : pspace->objfiles ())
     if (objfile.map_symtabs_matching_filename (name, real_path.get (),
-					       callback))
+					       callback)
+	== iteration_status::stop)
       return;
 }
 
@@ -661,7 +662,7 @@ lookup_symtab (program_space *pspace, const char *name)
   iterate_over_symtabs (pspace, name, [&] (symtab *symtab)
     {
       result = symtab;
-      return true;
+      return iteration_status::stop;
     });
 
   return result;
@@ -2381,9 +2382,11 @@ lookup_symbol_via_quick_fns (struct objfile *objfile,
     {
       const struct blockvector *bv = symtab->blockvector ();
       const struct block *block = bv->block (block_index);
-      /* If the accumulator finds a best symbol, end the search by
-	 returning false; otherwise keep going by returning true.  */
-      return !accum.search (symtab, block, lookup_name, domain);
+      /* End the search if the accumulator finds a best symbol.  */
+      if (accum.search (symtab, block, lookup_name, domain))
+	return iteration_status::stop;
+
+      return iteration_status::keep_going;
     };
 
   objfile->search (nullptr, &lookup_name, nullptr, searcher,
@@ -5952,7 +5955,7 @@ default_collect_symbol_completion_matches_break_on
 	     add_symtab_completions (symtab,
 				     tracker, mode, lookup_name,
 				     sym_text, word, code);
-	     return true;
+	     return iteration_status::keep_going;
 	   },
 	 SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
 	 SEARCH_ALL_DOMAINS);
@@ -6142,7 +6145,7 @@ collect_file_symbol_completion_matches (completion_tracker &tracker,
       add_symtab_completions (s->compunit (),
 			      tracker, mode, lookup_name,
 			      sym_text, word, TYPE_CODE_UNDEF);
-      return false;
+      return iteration_status::keep_going;
     });
 }
 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 0db85bcf5baa..9e45f087adda 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -30,6 +30,7 @@
 #include "gdbsupport/gdb_regex.h"
 #include "gdbsupport/enum-flags.h"
 #include "gdbsupport/function-view.h"
+#include "gdbsupport/iteration-status.h"
 #include <optional>
 #include <string_view>
 #include "gdbsupport/next-iterator.h"
@@ -2795,8 +2796,9 @@ bool compare_filenames_for_search (const char *filename,
    Call CALLBACK with each symtab that is found.  If CALLBACK returns
    true, the search stops.  */
 
-void iterate_over_symtabs (program_space *pspace, const char *name,
-			   gdb::function_view<bool (symtab *)> callback);
+void iterate_over_symtabs
+  (program_space *pspace, const char *name,
+   gdb::function_view<iteration_status (symtab *)> callback);
 
 std::vector<const linetable_entry *> find_linetable_entries_for_symtab_line
     (struct symtab *symtab, int line, const linetable_entry **best_entry);
diff --git a/gdbsupport/iteration-status.h b/gdbsupport/iteration-status.h
new file mode 100644
index 000000000000..c4c17fea4fb5
--- /dev/null
+++ b/gdbsupport/iteration-status.h
@@ -0,0 +1,48 @@
+/* Copyright (C) 2026 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_ITERATION_STATUS_H
+#define GDBSUPPORT_ITERATION_STATUS_H
+
+/* Return type for iteration callbacks.
+
+   Using an enum makes it clear at each call site whether the callback wants to
+   stop or continue, unlike a plain bool where the convention can change between
+   APIs.  */
+
+enum class iteration_status
+{
+  keep_going,
+  stop,
+};
+
+constexpr const char *
+iteration_status_str (iteration_status status)
+{
+  switch (status)
+    {
+    case iteration_status::keep_going:
+      return "keep_going";
+
+    case iteration_status::stop:
+      return "stop";
+    }
+
+  gdb_assert_not_reached ("invalid iteration_status value");
+}
+
+#endif /* GDBSUPPORT_ITERATION_STATUS_H */
-- 
2.53.0


  parent reply	other threads:[~2026-04-16 20:25 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-16 20:16 [PATCH 00/11] Readability improvements of some iteration functions Simon Marchi
2026-04-16 20:16 ` [PATCH 01/11] gdb/dwarf: remove unused file_match parameter from dwarf2_base_index_functions::search_one Simon Marchi
2026-04-17 11:11   ` Andrew Burgess
2026-04-16 20:16 ` [PATCH 02/11] gdb: rename search_symtabs_expansion_listener -> compunit_symtab_iteration_callback Simon Marchi
2026-04-17 15:08   ` Simon Marchi
2026-04-17 17:17   ` Tom Tromey
2026-04-17 19:34     ` Simon Marchi
2026-04-16 20:16 ` Simon Marchi [this message]
2026-04-17 11:01   ` [PATCH 03/11] gdb: introduce iteration_status enum, use it for search callbacks Andrew Burgess
2026-04-17 14:33     ` Simon Marchi
2026-04-17 14:34   ` Tom Tromey
2026-04-16 20:16 ` [PATCH 04/11] gdb, gdbserver: make iterate_over_lwps_ftype a function_view Simon Marchi
2026-04-17 11:09   ` Andrew Burgess
2026-04-17 14:37     ` Simon Marchi
2026-04-16 20:16 ` [PATCH 05/11] gdb, gdbserver: split iterate_over_lwps in for_each_lwp and find_lwp Simon Marchi
2026-04-17 11:25   ` Andrew Burgess
2026-04-17 14:53     ` Simon Marchi
2026-04-16 20:16 ` [PATCH 06/11] gdb: split iterate_over_threads in for_each_thread and find_thread Simon Marchi
2026-04-17 11:33   ` Andrew Burgess
2026-04-16 20:16 ` [PATCH 07/11] gdb: split iterate_over_minimal_symbols in for_each_minimal_symbol and find_minimal_symbol Simon Marchi
2026-04-17 12:13   ` Andrew Burgess
2026-04-16 20:16 ` [PATCH 08/11] gdb: split iterate_over_symtabs in for_each_symtab and find_symtab Simon Marchi
2026-04-17 13:31   ` Andrew Burgess
2026-04-17 14:54     ` Simon Marchi
2026-04-16 20:16 ` [PATCH 09/11] gdb: change objfile::map_symtabs_matching_filename to find_symtab_matching_filename Simon Marchi
2026-04-17 13:50   ` Andrew Burgess
2026-04-17 15:03     ` Simon Marchi
2026-04-16 20:16 ` [PATCH 10/11] gdb: make symbol_found_callback_ftype a function_view Simon Marchi
2026-04-17 13:55   ` Andrew Burgess
2026-04-17 15:04     ` Simon Marchi
2026-04-16 20:16 ` [PATCH 11/11] gdb: make iterate_over_symbols return void, rename to for_each_symbol Simon Marchi
2026-04-17 14:05   ` Andrew Burgess
2026-04-17 15:06     ` 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=20260416202408.422441-4-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