From 99b6de03fe4ac1518e68ac42c124dd5fa1704397 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 3 Sep 2025 10:50:02 -0400 Subject: [PATCH] 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 --- gdb/block.c | 9 +++++---- gdb/block.h | 17 ++++++++++++++--- gdb/dwarf2/die.h | 4 +++- gdb/gdb_bfd.h | 9 ++++++--- gdb/gdbthread.h | 4 +++- gdb/inferior.h | 30 ++++++++++++++++++++++++------ gdb/linux-nat.c | 6 ++++-- gdb/objfiles.h | 4 +++- gdb/progspace.h | 7 ++++--- gdb/psymtab.h | 4 +++- gdb/symtab.h | 4 +++- gdb/ui.h | 4 +++- gdbsupport/iterator-range.h | 5 ++--- 13 files changed, 77 insertions(+), 30 deletions(-) diff --git a/gdb/block.c b/gdb/block.c index 2450ebd9be9..e09bccdcf86 100644 --- a/gdb/block.c +++ b/gdb/block.c @@ -325,10 +325,11 @@ block::set_scope (const char *scope, struct obstack *obstack) next_range block::get_using () const { - if (m_namespace_info == nullptr) - return {}; - else - return next_range (m_namespace_info->using_decl); + next_iterator begin (m_namespace_info != nullptr + ? m_namespace_info->using_decl + : nullptr); + + return next_range (std::move (begin)); } /* See block.h. */ diff --git a/gdb/block.h b/gdb/block.h index 4ea5294877c..8a3ea822d04 100644 --- a/gdb/block.h +++ b/gdb/block.h @@ -146,7 +146,11 @@ struct block : public allocate_on_obstack /* Return an iterator range for this block's multidict. */ iterator_range multidict_symbols () const - { return iterator_range (m_multidict); } + { + mdict_iterator_wrapper begin (m_multidict); + + return iterator_range (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_range; +inline iterator_range +block_iterator_range (const block *block, + const lookup_name_info *name = nullptr) +{ + block_iterator_wrapper begin (block, name); + + return iterator_range (std::move (begin)); +} /* Return true if symbol A is the best match possible for DOMAIN. */ diff --git a/gdb/dwarf2/die.h b/gdb/dwarf2/die.h index 6ec575757d3..6f269caa6e9 100644 --- a/gdb/dwarf2/die.h +++ b/gdb/dwarf2/die.h @@ -108,7 +108,9 @@ struct die_info DIE. */ next_range children () const { - return next_range (child); + next_iterator begin (child); + + return next_range (std::move (begin)); } /* DWARF-2 tag for this DIE. */ diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h index 41ce5ce6d82..eb4428e5bdb 100644 --- a/gdb/gdb_bfd.h +++ b/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; static inline gdb_bfd_section_range gdb_bfd_sections (bfd *abfd) { - return gdb_bfd_section_range (abfd->sections); + next_iterator 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 begin (abfd->sections); + + return gdb_bfd_section_range (std::move (begin)); }; /* A wrapper for bfd_stat that acquires the per-BFD lock on ABFD. */ diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h index a22210951c4..44f95c6fff4 100644 --- a/gdb/gdbthread.h +++ b/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); diff --git a/gdb/inferior.h b/gdb/inferior.h index 96dd54329e3..b02ec57b5de 100644 --- a/gdb/inferior.h +++ b/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_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 diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 8ca6d78bb7e..af081fca7f7 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -707,7 +707,9 @@ static intrusive_list 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. */ diff --git a/gdb/objfiles.h b/gdb/objfiles.h index 118f1cd9828..9dbb46ab610 100644 --- a/gdb/objfiles.h +++ b/gdb/objfiles.h @@ -476,7 +476,9 @@ public: compunit_symtab_range compunits () { - return compunit_symtab_range (compunit_symtabs); + next_iterator begin (compunit_symtabs); + + return compunit_symtab_range (std::move (begin)); } /* A range adapter that makes it possible to iterate over all diff --git a/gdb/progspace.h b/gdb/progspace.h index 300cfae95a9..06fd33ed62d 100644 --- a/gdb/progspace.h +++ b/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; @@ -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 diff --git a/gdb/psymtab.h b/gdb/psymtab.h index b1c3ad7dda6..ab4173ddc3f 100644 --- a/gdb/psymtab.h +++ b/gdb/psymtab.h @@ -111,7 +111,9 @@ public: partial_symtab_range range () { - return partial_symtab_range (psymtabs); + next_iterator begin (psymtabs); + + return partial_symtab_range (std::move (begin)); } diff --git a/gdb/symtab.h b/gdb/symtab.h index 09a361dda40..254b425a8e2 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -1844,7 +1844,9 @@ struct compunit_symtab symtab_range filetabs () const { - return symtab_range (m_filetabs); + next_iterator begin (m_filetabs); + + return symtab_range (std::move (begin)); } void add_filetab (symtab *filetab) diff --git a/gdb/ui.h b/gdb/ui.h index 41927570617..61bfd2dc2c8 100644 --- a/gdb/ui.h +++ b/gdb/ui.h @@ -227,7 +227,9 @@ using ui_range = next_range; static inline ui_range all_uis () { - return ui_range (ui_list); + next_iterator begin (ui_list); + + return ui_range (std::move (begin)); } #endif /* GDB_UI_H */ diff --git a/gdbsupport/iterator-range.h b/gdbsupport/iterator-range.h index 772c82480ac..d18939c88b0 100644 --- a/gdbsupport/iterator-range.h +++ b/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 - iterator_range (Args &&...args) - : m_begin (std::forward (args)...) + explicit iterator_range (IteratorType begin) + : iterator_range (std::move (begin), IteratorType {}) {} /* Create an iterator range using explicit BEGIN and END iterators. */