From: Simon Marchi Date: Thu, 28 Aug 2025 15:10:50 +0000 (-0400) Subject: gdbsupport: make filtered_iterator work with pointers X-Git-Tag: gdb-17-branchpoint~118 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=1e10e9ea96a4141080a7cd7322babe6bcd7c42ba;p=thirdparty%2Fbinutils-gdb.git gdbsupport: make filtered_iterator work with pointers It's currently not possible to use filtered_iterator with a pointer as the base iterator type. This patch makes it possible. The indended usage is: Foo array[12]; Foo *begin = array; Foo *end = array + ARRAY_SIZE (array); filtered_iterator (begin, end); Here are the things that needed changing: - Give filtered_iterator a constructor where the caller provides already constructed begin and end iterators. filtered_iterator currently assumes that default-constructing a BaseIterator will produce a valid "end" iterator. This is not the case if BaseIterator is a pointer. The caller needs to pass in the end of the array / region to iterate on as the end. - Typedefs of member types like wouldn't work: typedef typename BaseIterator::value_type value_type; The compiler would complain that it's not possible to apply `::` to type `BaseIterator` (aka `Foo *`). Use std::iterator_traits to fix it [1]. - Similarly, the compiler would complain about the use of `BaseIterator::operator*` in the return type of `filtered_iterator::operator*`. Fix this by using `decltype(auto)` as the return type. This lets the compiler deduce the return type from the return statement. Unlike `auto`, `decltype(auto)` perfectly preserves the "cvref-ness" of the deduced return type. If the return expression yields a `Foo &`, then the function will return a `Foo &` (which is what we want), whereas it would return a `Foo` if we used just `auto`. Improve the filtered_iterator unit tests to run the same tests but with pointers as iterators. Because the filtered_iterator objects are initialized differently in the two scenarios, I chose to copy the existing code and adapt it. It would probably be possible to add a layer of abstraction to avoid code duplication, but it would end up more complicated and messy. If we ever add a third scenario, we can revisit that. [1] https://en.cppreference.com/w/cpp/iterator/iterator_traits.html Change-Id: Id962ffbcd960a705a82bc5eb4808b4fe118a2761 Approved-By: Tom Tromey --- diff --git a/gdb/unittests/filtered_iterator-selftests.c b/gdb/unittests/filtered_iterator-selftests.c index 49c95cb2bf9..c04cae4963e 100644 --- a/gdb/unittests/filtered_iterator-selftests.c +++ b/gdb/unittests/filtered_iterator-selftests.c @@ -125,6 +125,26 @@ test_filtered_iterator () SELF_CHECK (even_ints == expected_even_ints); } +/* Same as the above, but using pointers as the iterator base type. */ + +static void +test_filtered_iterator_ptr () +{ + int array[] = { 4, 4, 5, 6, 7, 8, 9 }; + std::vector even_ints; + const std::vector expected_even_ints { 4, 4, 6, 8 }; + + filtered_iterator iter + (array, array + ARRAY_SIZE (array)); + filtered_iterator end + (array + ARRAY_SIZE (array), array + ARRAY_SIZE (array)); + + for (; iter != end; ++iter) + even_ints.push_back (*iter); + + SELF_CHECK (even_ints == expected_even_ints); +} + /* Test operator== and operator!=. */ static void @@ -152,6 +172,34 @@ test_filtered_iterator_eq () SELF_CHECK (!(iter1 != iter2)); } + +/* Same as the above, but using pointers as the iterator base type. */ + +static void +test_filtered_iterator_eq_ptr () +{ + int array[] = { 4, 4, 5, 6, 7, 8, 9 }; + + filtered_iterator iter1 + (array, array + ARRAY_SIZE(array)); + filtered_iterator iter2 + (array, array + ARRAY_SIZE(array)); + + /* They start equal. */ + SELF_CHECK (iter1 == iter2); + SELF_CHECK (!(iter1 != iter2)); + + /* Advance 1, now they aren't equal (despite pointing to equal values). */ + ++iter1; + SELF_CHECK (!(iter1 == iter2)); + SELF_CHECK (iter1 != iter2); + + /* Advance 2, now they are equal again. */ + ++iter2; + SELF_CHECK (iter1 == iter2); + SELF_CHECK (!(iter1 != iter2)); +} + } /* namespace selftests */ INIT_GDB_FILE (filtered_iterator_selftests) @@ -160,4 +208,8 @@ INIT_GDB_FILE (filtered_iterator_selftests) selftests::test_filtered_iterator); selftests::register_test ("filtered_iterator_eq", selftests::test_filtered_iterator_eq); + selftests::register_test ("filtered_iterator_ptr", + selftests::test_filtered_iterator_ptr); + selftests::register_test ("filtered_iterator_eq_ptr", + selftests::test_filtered_iterator_eq_ptr); } diff --git a/gdbsupport/filtered-iterator.h b/gdbsupport/filtered-iterator.h index e824d6115a8..4952582358b 100644 --- a/gdbsupport/filtered-iterator.h +++ b/gdbsupport/filtered-iterator.h @@ -19,8 +19,6 @@ #ifndef GDBSUPPORT_FILTERED_ITERATOR_H #define GDBSUPPORT_FILTERED_ITERATOR_H -#include - /* A filtered iterator. This wraps BaseIterator and automatically skips elements that FilterFunc filters out. Requires that default-constructing a BaseIterator creates a valid one-past-end @@ -30,12 +28,14 @@ template class filtered_iterator { public: - typedef filtered_iterator self_type; - typedef typename BaseIterator::value_type value_type; - typedef typename BaseIterator::reference reference; - typedef typename BaseIterator::pointer pointer; - typedef typename BaseIterator::iterator_category iterator_category; - typedef typename BaseIterator::difference_type difference_type; + using self_type = filtered_iterator; + using value_type = typename std::iterator_traits::value_type; + using reference = typename std::iterator_traits::reference; + using pointer = typename std::iterator_traits::pointer; + using iterator_category + = typename std::iterator_traits::iterator_category; + using difference_type + = typename std::iterator_traits::difference_type; /* Construct by forwarding all arguments to the underlying iterator. */ @@ -44,6 +44,10 @@ public: : m_it (std::forward (args)...) { skip_filtered (); } + filtered_iterator (BaseIterator begin, BaseIterator end) + : m_it (std::move (begin)), m_end (std::move (end)) + { skip_filtered (); } + /* Create a one-past-end iterator. */ filtered_iterator () = default; @@ -56,9 +60,7 @@ public: : filtered_iterator (static_cast (other)) {} - typename std::invoke_result::type - operator* () const + decltype(auto) operator* () const { return *m_it; } self_type &operator++ ()