gdbsupport: remove variadicity from iterator_range constructor

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>

gdb/block.c

@@ -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.  */

gdb/block.h

@@ -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.  */
 
-typedef iterator_range<block_iterator_wrapper> block_iterator_range;
+inline iterator_range<block_iterator_wrapper>
+block_iterator_range (const block *block,
+		      const lookup_name_info *name = nullptr)
+{
+  block_iterator_wrapper begin (block, name);
+
+  return iterator_range<block_iterator_wrapper> (std::move (begin));
+}
 
 /* Return true if symbol A is the best match possible for DOMAIN.  */
 

gdb/dwarf2/die.h

@@ -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.  */

gdb/gdb_bfd.h

@@ -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.  */

gdb/gdbthread.h

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

gdb/inferior.h

@@ -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

gdb/linux-nat.c

@@ -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.  */

gdb/objfiles.h

@@ -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

gdb/progspace.h

@@ -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

gdb/psymtab.h

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

gdb/symtab.h

@@ -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)

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 */

gdbsupport/iterator-range.h

@@ -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.  */