]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
gdbsupport: remove variadicity from iterator_range constructor
authorSimon Marchi <simon.marchi@polymtl.ca>
Wed, 3 Sep 2025 14:50:02 +0000 (10:50 -0400)
committerSimon Marchi <simon.marchi@efficios.com>
Tue, 7 Oct 2025 20:22:15 +0000 (16:22 -0400)
There are two ways to build an iterator_range:

 - Using the variadic constructor, where the arguments you pass are used
   to construct the "begin" underlying iterator.  The "end" iterator is
   obtained by default-constructing the underlying iterator.

 - Using the other constructor, by explicitly providing the "begin" and
   "end" iterators.

My experience is that using the variadic constructor is very confusing,
especially when you have multiple layers of iterator wrappers.  It's not
obvious where the arguments you provide end up.  When you have a
compilation error, it is hard to decipher.

I propose to remove the variadicity of the first constructor of
iterator_range, and subsequently of the other iterator wrappers.  This
requires callers to be more verbose, explicitly instantiate all the
layers.  But since we only instantiate these iterator wrappers in
factory functions, I think it's fine.  If there is a compilation error,
it will be much easier to find and fix the problem.

Using the new one-argument constructor, it is still assumed that the end
iterator can be obtained by default-constructing the underlying iterator
type, which I think is fine and not too confusing.

Change-Id: I54d6fdef18bcd7e308825064e0fc18fadd7ca717
Approved-By: Tom Tromey <tom@tromey.com>
13 files changed:
gdb/block.c
gdb/block.h
gdb/dwarf2/die.h
gdb/gdb_bfd.h
gdb/gdbthread.h
gdb/inferior.h
gdb/linux-nat.c
gdb/objfiles.h
gdb/progspace.h
gdb/psymtab.h
gdb/symtab.h
gdb/ui.h
gdbsupport/iterator-range.h

index 2450ebd9be951a970b80acb2b17786e490894a11..e09bccdcf8620a2467fbf5dabdd9507b067d886b 100644 (file)
@@ -325,10 +325,11 @@ block::set_scope (const char *scope, struct obstack *obstack)
 next_range<using_direct>
 block::get_using () const
 {
-  if (m_namespace_info == nullptr)
-    return {};
-  else
-    return next_range<using_direct> (m_namespace_info->using_decl);
+  next_iterator<using_direct> begin (m_namespace_info != nullptr
+                                    ? m_namespace_info->using_decl
+                                    : nullptr);
+
+  return next_range<using_direct> (std::move (begin));
 }
 
 /* See block.h.  */
index 4ea5294877c5690d61923a5d01e9b2c523e1c885..8a3ea822d04c61d7e2b875ae7ecc96b9c06598f0 100644 (file)
@@ -146,7 +146,11 @@ struct block : public allocate_on_obstack<block>
 
   /* Return an iterator range for this block's multidict.  */
   iterator_range<mdict_iterator_wrapper> multidict_symbols () const
-  { return iterator_range<mdict_iterator_wrapper> (m_multidict); }
+  {
+    mdict_iterator_wrapper begin (m_multidict);
+
+    return iterator_range<mdict_iterator_wrapper> (std::move (begin));
+  }
 
   /* Set this block's multidict.  */
   void set_multidict (multidictionary *multidict)
@@ -610,9 +614,16 @@ private:
   struct block_iterator m_iter;
 };
 
-/* An iterator range for block_iterator_wrapper.  */
+/* Return an iterator range for block_iterator_wrapper.  */
+
+inline iterator_range<block_iterator_wrapper>
+block_iterator_range (const block *block,
+                     const lookup_name_info *name = nullptr)
+{
+  block_iterator_wrapper begin (block, name);
 
-typedef iterator_range<block_iterator_wrapper> block_iterator_range;
+  return iterator_range<block_iterator_wrapper> (std::move (begin));
+}
 
 /* Return true if symbol A is the best match possible for DOMAIN.  */
 
index 6ec575757d395bf476d0aeea7aad843299365d1b..6f269caa6e988d98feedd2f2d8b58a53b3c192e6 100644 (file)
@@ -108,7 +108,9 @@ struct die_info
      DIE.  */
   next_range<die_info> children () const
   {
-    return next_range<die_info> (child);
+    next_iterator<die_info> begin (child);
+
+    return next_range<die_info> (std::move (begin));
   }
 
   /* DWARF-2 tag for this DIE.  */
index 41ce5ce6d82e92021f450cefa5f6780808fc7d7e..eb4428e5bdb683f03fda84271931838dee9225c4 100644 (file)
@@ -24,7 +24,6 @@
 #include "gdbsupport/byte-vector.h"
 #include "gdbsupport/function-view.h"
 #include "gdbsupport/gdb_ref_ptr.h"
-#include "gdbsupport/iterator-range.h"
 #include "gdbsupport/next-iterator.h"
 
 /* A registry adaptor for BFD.  This arranges to store the registry in
@@ -242,13 +241,17 @@ using gdb_bfd_section_range = next_range<asection>;
 static inline gdb_bfd_section_range
 gdb_bfd_sections (bfd *abfd)
 {
-  return gdb_bfd_section_range (abfd->sections);
+  next_iterator<asection> begin (abfd->sections);
+
+  return gdb_bfd_section_range (std::move (begin));
 }
 
 static inline gdb_bfd_section_range
 gdb_bfd_sections (const gdb_bfd_ref_ptr &abfd)
 {
-  return gdb_bfd_section_range (abfd->sections);
+  next_iterator<asection> begin (abfd->sections);
+
+  return gdb_bfd_section_range (std::move (begin));
 };
 
 /* A wrapper for bfd_stat that acquires the per-BFD lock on ABFD.  */
index a22210951c49619be738be156080803d3148e96d..44f95c6fff4d333242d874b73292752a019bd6e7 100644 (file)
@@ -794,7 +794,9 @@ all_non_exited_threads (process_stratum_target *proc_target = nullptr,
 inline all_threads_safe_range
 all_threads_safe ()
 {
-  return all_threads_safe_range (all_threads_iterator::begin_t {});
+  all_threads_safe_iterator begin (all_threads_iterator::begin_t {});
+
+  return all_threads_safe_range (std::move (begin));
 }
 
 extern int thread_count (process_stratum_target *proc_target);
index 96dd54329e35cb7dd9b384691e390989e11ea9a9..b02ec57b5de119e0c58d1ee2a216bdbb9a127f05 100644 (file)
@@ -473,7 +473,11 @@ public:
         { .... }
   */
   inf_threads_range threads ()
-  { return inf_threads_range (this->thread_list.begin ()); }
+  {
+    inf_threads_iterator begin (this->thread_list.begin ());
+
+    return inf_threads_range (std::move (begin));
+  }
 
   /* Returns a range adapter covering the inferior's non-exited
      threads.  Used like this:
@@ -482,7 +486,11 @@ public:
         { .... }
   */
   inf_non_exited_threads_range non_exited_threads ()
-  { return inf_non_exited_threads_range (this->thread_list.begin ()); }
+  {
+    inf_non_exited_threads_iterator begin (this->thread_list.begin ());
+
+    return inf_non_exited_threads_range (std::move (begin));
+  }
 
   /* Like inferior::threads(), but returns a range adapter that can be
      used with range-for, safely.  I.e., it is safe to delete the
@@ -493,7 +501,11 @@ public:
         delete &f;
   */
   inline safe_inf_threads_range threads_safe ()
-  { return safe_inf_threads_range (this->thread_list.begin ()); }
+  {
+    safe_inf_threads_iterator begin (this->thread_list.begin ());
+
+    return safe_inf_threads_range (std::move (begin));
+  }
 
   /* Find (non-exited) thread PTID of this inferior.  */
   thread_info *find_thread (ptid_t ptid);
@@ -820,7 +832,9 @@ extern intrusive_list<inferior> inferior_list;
 inline all_inferiors_safe_range
 all_inferiors_safe ()
 {
-  return all_inferiors_safe_range (nullptr, inferior_list);
+  all_inferiors_safe_iterator begin (nullptr, inferior_list);
+
+  return all_inferiors_safe_range (std::move (begin));
 }
 
 /* Returns a range representing all inferiors, suitable to use with
@@ -833,7 +847,9 @@ all_inferiors_safe ()
 inline all_inferiors_range
 all_inferiors (process_stratum_target *proc_target = nullptr)
 {
-  return all_inferiors_range (proc_target, inferior_list);
+  all_inferiors_iterator begin (proc_target, inferior_list);
+
+  return all_inferiors_range (std::move (begin));
 }
 
 /* Return a range that can be used to walk over all inferiors with PID
@@ -842,7 +858,9 @@ all_inferiors (process_stratum_target *proc_target = nullptr)
 inline all_non_exited_inferiors_range
 all_non_exited_inferiors (process_stratum_target *proc_target = nullptr)
 {
-  return all_non_exited_inferiors_range (proc_target, inferior_list);
+  all_non_exited_inferiors_iterator begin (proc_target, inferior_list);
+
+  return all_non_exited_inferiors_range (std::move (begin));
 }
 
 /* Prune away automatically added inferiors that aren't required
index 8ca6d78bb7ea4dc3c5f91032a7cae4f692306d02..af081fca7f7e3402f319b5b76810db186d5c04b5 100644 (file)
@@ -707,7 +707,9 @@ static intrusive_list<lwp_info> lwp_list;
 lwp_info_range
 all_lwps ()
 {
-  return lwp_info_range (lwp_list.begin ());
+  lwp_info_iterator begin (lwp_list.begin ());
+
+  return lwp_info_range (std::move (begin));
 }
 
 /* See linux-nat.h.  */
@@ -715,7 +717,7 @@ all_lwps ()
 lwp_info_safe_range
 all_lwps_safe ()
 {
-  return lwp_info_safe_range (lwp_list.begin ());
+  return lwp_info_safe_range (all_lwps ());
 }
 
 /* Add LP to sorted-by-reverse-creation-order doubly-linked list.  */
index 118f1cd98289f0bf2f88e436073fec676f34378a..9dbb46ab6104a027618db93c0c059f97755ded79 100644 (file)
@@ -476,7 +476,9 @@ public:
 
   compunit_symtab_range compunits ()
   {
-    return compunit_symtab_range (compunit_symtabs);
+    next_iterator<compunit_symtab> begin (compunit_symtabs);
+
+    return compunit_symtab_range (std::move (begin));
   }
 
   /* A range adapter that makes it possible to iterate over all
index 300cfae95a915031646b60ceca7fef89322cbe4d..06fd33ed62d199c26efcf44830764a5e3be79a1f 100644 (file)
@@ -193,7 +193,9 @@ struct program_space
      for (objfile &objf : pspace->objfiles ()) { ... }  */
   objfiles_range objfiles ()
   {
-    return objfiles_range (objfiles_iterator (m_objfiles_list.begin ()));
+    objfiles_iterator begin (m_objfiles_list.begin ());
+
+    return objfiles_range (std::move (begin));
   }
 
   using objfiles_safe_range = basic_safe_range<objfiles_range>;
@@ -207,8 +209,7 @@ struct program_space
      deleted during iteration.  */
   objfiles_safe_range objfiles_safe ()
   {
-    return objfiles_safe_range
-      (objfiles_range (objfiles_iterator (m_objfiles_list.begin ())));
+    return objfiles_safe_range (this->objfiles ());
   }
 
   /* Iterate over all objfiles of the program space in the order that makes the
index b1c3ad7dda670431771a95e7e1bee25aa9be88c4..ab4173ddc3fedf97cd2537b9fb9c44ea33437e44 100644 (file)
@@ -111,7 +111,9 @@ public:
 
   partial_symtab_range range ()
   {
-    return partial_symtab_range (psymtabs);
+    next_iterator<partial_symtab> begin (psymtabs);
+
+    return partial_symtab_range (std::move (begin));
   }
 
 
index 09a361dda400e7374b116ad7c082ee41ffdde2a7..254b425a8e2068b212606ecd89688f75d1d70d17 100644 (file)
@@ -1844,7 +1844,9 @@ struct compunit_symtab
 
   symtab_range filetabs () const
   {
-    return symtab_range (m_filetabs);
+    next_iterator<symtab> begin (m_filetabs);
+
+    return symtab_range (std::move (begin));
   }
 
   void add_filetab (symtab *filetab)
index 4192757061763f8f2d5f8cc71ef6f8c0cbf17f92..61bfd2dc2c8d91e399d87258885cc803114d6f88 100644 (file)
--- a/gdb/ui.h
+++ b/gdb/ui.h
@@ -227,7 +227,9 @@ using ui_range = next_range<ui>;
 static inline
 ui_range all_uis ()
 {
-  return ui_range (ui_list);
+  next_iterator<ui> begin (ui_list);
+
+  return ui_range (std::move (begin));
 }
 
 #endif /* GDB_UI_H */
index 772c82480acc4ad03e0a2047671224e0a346d5d0..d18939c88b0b4b43510619501901c5a60ecd3ec9 100644 (file)
@@ -30,9 +30,8 @@ struct iterator_range
   /* Create an iterator_range using BEGIN as the begin iterator.
 
      Assume that the end iterator can be default-constructed.  */
-  template <typename... Args>
-  iterator_range (Args &&...args)
-    : m_begin (std::forward<Args> (args)...)
+  explicit iterator_range (IteratorType begin)
+    : iterator_range (std::move (begin), IteratorType {})
   {}
 
   /* Create an iterator range using explicit BEGIN and END iterators.  */