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<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
Approved-By: Tom Tromey <tom@tromey.com>

diff --git a/gdb/unittests/filtered_iterator-selftests.c b/gdb/unittests/filtered_iterator-selftests.c
index 49c95cb2bf9414ade5eaa53d6d7d30176ed7ccf1..c04cae4963e0554b03ee093a1aa0398db13cf4ba 100644 (file)
--- 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 e824d6115a8a5c900ebe0e6372ef0aae0ba1ad14..4952582358b864e8a25cff55ba9df8a49e8bfbf9 100644 (file)
--- 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,14 @@ 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 +44,10 @@ public:
     : m_it (std::forward<Args> (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<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++ ()