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 2/3] gdbsupport: make filtered_iterator work with pointers
Date: Thu, 28 Aug 2025 11:10:50 -0400	[thread overview]
Message-ID: <20250828151100.84594-2-simon.marchi@efficios.com> (raw)
In-Reply-To: <20250828151100.84594-1-simon.marchi@efficios.com>

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<Foo *, FooFilter> (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
---
 gdb/unittests/filtered_iterator-selftests.c | 52 +++++++++++++++++++++
 gdbsupport/filtered-iterator.h              | 26 ++++++-----
 2 files changed, 67 insertions(+), 11 deletions(-)

diff --git a/gdb/unittests/filtered_iterator-selftests.c b/gdb/unittests/filtered_iterator-selftests.c
index 49c95cb2bf94..c04cae4963e0 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<int> even_ints;
+  const std::vector<int> expected_even_ints { 4, 4, 6, 8 };
+
+  filtered_iterator<int *, even_numbers_only> iter
+    (array, array + ARRAY_SIZE (array));
+  filtered_iterator<int *, even_numbers_only> 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<int *, even_numbers_only> iter1
+    (array, array + ARRAY_SIZE(array));
+  filtered_iterator<int *, even_numbers_only> 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 e824d6115a8a..38b4bb1236be 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 <type_traits>
-
 /* 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,16 @@ template<typename BaseIterator, typename FilterFunc>
 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<BaseIterator>::value_type;
+  using reference = typename std::iterator_traits<BaseIterator>::reference;
+  using pointer = typename std::iterator_traits<BaseIterator>::pointer;
+  using iterator_category =
+      typename std::iterator_traits<BaseIterator>::iterator_category;
+  ;
+  using difference_type =
+      typename std::iterator_traits<BaseIterator>::difference_type;
+  ;
 
   /* Construct by forwarding all arguments to the underlying
      iterator.  */
@@ -44,6 +46,10 @@ class filtered_iterator
     : m_it (std::forward<Args> (args)...)
   { skip_filtered (); }
 
+  filtered_iterator (BaseIterator begin, BaseIterator end)
+    : m_it (begin), m_end (end)
+  { skip_filtered (); }
+
   /* Create a one-past-end iterator.  */
   filtered_iterator () = default;
 
@@ -56,9 +62,7 @@ class filtered_iterator
     : filtered_iterator (static_cast<const filtered_iterator &> (other))
   {}
 
-  typename std::invoke_result<decltype(&BaseIterator::operator*),
-			      BaseIterator>::type
-    operator* () const
+  decltype(auto) operator* () const
   { return *m_it; }
 
   self_type &operator++ ()
-- 
2.51.0


  reply	other threads:[~2025-08-28 15:11 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-28 15:10 [PATCH 1/3] gdb/objfiles: make objfile::sections yield references Simon Marchi
2025-08-28 15:10 ` Simon Marchi [this message]
2025-08-29 13:40   ` [PATCH 2/3] gdbsupport: make filtered_iterator work with pointers Tom Tromey
2025-08-29 16:00     ` Simon Marchi
2025-08-29 17:39       ` Tom Tromey
2025-08-29 20:11         ` Simon Marchi
2025-09-03  2:01       ` Tom Tromey
2025-08-29 17:59     ` Simon Marchi
2025-08-29 18:19       ` Tom Tromey
2025-08-28 15:10 ` [PATCH 3/3] gdb/objfiles: use filtered_iterator as objfile::section_iterator Simon Marchi
2025-08-29 13:42   ` Tom Tromey
2025-08-29 16:06     ` Simon Marchi
2025-08-29 20:22       ` Simon Marchi
2025-08-29 13:34 ` [PATCH 1/3] gdb/objfiles: make objfile::sections yield references Tom Tromey
2025-08-29 15:47   ` 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=20250828151100.84594-2-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