make iterator_adaptor easier to use (#364)

.appveyor.yml

@@ -34,4 +34,5 @@ install:
 
 test_script:
   # cxxstd=latest (aka 20) breaks VS 2022 in algorithm_project_test so we use cxxstd=17
-  - ..\..\b2 %B2_OPTS% cxxstd=17 test//minimal test//serialization
+  # msvc-14.3 breaks test//serialization, more precisely axis_variant_serialization_test
+  - ..\..\b2 %B2_OPTS% cxxstd=17 test//minimal

.gitignore

@@ -11,3 +11,4 @@ __pycache__
 tools/lcov-*
 tools/codecov
 coverage-report
+.cache

benchmark/CMakeLists.txt

@@ -7,7 +7,7 @@ include(BoostFetch)
 # setup for google benchmark
 set(CMAKE_BUILD_TYPE Release) # ok, only set in local scope
 option(BENCHMARK_ENABLE_TESTING "" OFF)
-boost_fetch(google/benchmark)
+boost_fetch(google/benchmark TAG main)
 
 function(add_benchmark NAME)
 

include/boost/histogram/axis/iterator.hpp

@@ -19,12 +19,14 @@ template <class Axis>
 class iterator : public detail::iterator_adaptor<iterator<Axis>, index_type,
                                                  decltype(std::declval<Axis>().bin(0))> {
 public:
+  using reference = typename iterator::iterator_adaptor_::reference;
+
   /// Make iterator from axis and index.
   iterator(const Axis& axis, index_type idx)
       : iterator::iterator_adaptor_(idx), axis_(axis) {}
 
   /// Return current bin object.
-  decltype(auto) operator*() const { return axis_.bin(this->base()); }
+  reference operator*() const { return axis_.bin(this->base()); }
 
 private:
   const Axis& axis_;

include/boost/histogram/detail/iterator_adaptor.hpp

@@ -10,6 +10,7 @@
 #ifndef BOOST_HISTOGRAM_DETAIL_ITERATOR_ADAPTOR_HPP
 #define BOOST_HISTOGRAM_DETAIL_ITERATOR_ADAPTOR_HPP
 
+#include <cstddef>
 #include <iterator>
 #include <memory>
 #include <type_traits>
@@ -24,26 +25,43 @@ namespace detail {
 // produce an lvalue to which a pointer can be formed.  We do that by
 // returning a proxy object containing an instance of the reference object.
 template <class Reference>
-struct operator_arrow_dispatch_t // proxy references
-{
-  struct proxy {
-    explicit proxy(Reference const& x) noexcept : m_ref(x) {}
+struct operator_arrow_dispatch_t {
+  struct pointer {
+    explicit pointer(Reference const& x) noexcept : m_ref(x) {}
     Reference* operator->() noexcept { return std::addressof(m_ref); }
     Reference m_ref;
   };
 
-  using result_type = proxy;
-  static result_type apply(Reference const& x) noexcept { return proxy(x); }
+  using result_type = pointer;
+  static result_type apply(Reference const& x) noexcept { return pointer(x); }
 };
 
+// specialization for "real" references
 template <class T>
-struct operator_arrow_dispatch_t<T&> // "real" references
-{
+struct operator_arrow_dispatch_t<T&> {
   using result_type = T*;
   static result_type apply(T& x) noexcept { return std::addressof(x); }
 };
 
-// only for random access Base
+// it is ok if void_t is already defined in another header
+template <class...>
+using void_t = void;
+
+template <class T, class = void>
+struct get_difference_type_impl : std::make_signed<T> {};
+
+template <class T>
+struct get_difference_type_impl<
+    T, void_t<typename std::iterator_traits<T>::difference_type>> {
+  using type = typename std::iterator_traits<T>::difference_type;
+};
+
+template <class T>
+using get_difference_type = typename get_difference_type_impl<T>::type;
+
+// adaptor supports only random access Base
+// Base: underlying base type of the iterator; can be iterator, pointer, integer
+// Reference: type returned when pointer is dereferenced
 template <class Derived, class Base, class Reference = std::remove_pointer_t<Base>&,
           class Value = std::decay_t<Reference>>
 class iterator_adaptor {
@@ -53,103 +71,98 @@ public:
   using base_type = Base;
 
   using reference = Reference;
-  using value_type = std::remove_const_t<Value>;
+  using value_type = Value;
   using pointer = typename operator_arrow_dispatch::result_type;
-  using difference_type = std::ptrdiff_t;
+  using difference_type = get_difference_type<base_type>;
   using iterator_category = std::random_access_iterator_tag;
 
   iterator_adaptor() = default;
 
   explicit iterator_adaptor(base_type const& iter) : iter_(iter) {}
 
-  pointer operator->() const noexcept {
-    return operator_arrow_dispatch::apply(this->derived().operator*());
-  }
-  reference operator[](difference_type n) const { return *(this->derived() + n); }
-
-  Derived& operator++() {
-    ++iter_;
-    return this->derived();
-  }
-
-  Derived& operator--() {
-    --iter_;
-    return this->derived();
-  }
-
-  Derived operator++(int) {
-    Derived tmp(this->derived());
-    ++iter_;
-    return tmp;
-  }
-
-  Derived operator--(int) {
-    Derived tmp(this->derived());
-    --iter_;
-    return tmp;
-  }
+  // you can override this in derived
+  decltype(auto) operator*() const noexcept { return *iter_; }
 
+  // you can override this in derived
   Derived& operator+=(difference_type n) {
     iter_ += n;
     return this->derived();
   }
 
-  Derived& operator-=(difference_type n) {
-    iter_ -= n;
-    return this->derived();
-  }
-
-  Derived operator+(difference_type n) const {
-    Derived tmp(this->derived());
-    tmp += n;
-    return tmp;
-  }
-
-  Derived operator-(difference_type n) const { return operator+(-n); }
-
+  // you should override this in derived if there is an override for operator+=
   template <class... Ts>
   difference_type operator-(const iterator_adaptor<Ts...>& x) const noexcept {
     return iter_ - x.iter_;
   }
 
+  // you can override this in derived
   template <class... Ts>
   bool operator==(const iterator_adaptor<Ts...>& x) const noexcept {
     return iter_ == x.iter_;
   }
+
+  reference operator[](difference_type n) const { return *(this->derived() + n); }
+  pointer operator->() const noexcept {
+    return operator_arrow_dispatch::apply(this->derived().operator*());
+  }
+
+  Derived& operator-=(difference_type n) { return this->derived().operator+=(-n); }
+  Derived& operator++() { return this->derived().operator+=(1); }
+  Derived& operator--() { return this->derived().operator+=(-1); }
+
+  Derived operator++(int) {
+    Derived tmp(this->derived());
+    operator++();
+    return tmp;
+  }
+
+  Derived operator--(int) {
+    Derived tmp(this->derived());
+    operator--();
+    return tmp;
+  }
+
+  Derived operator+(difference_type n) const { return Derived(this->derived()) += n; }
+  Derived operator-(difference_type n) const { return Derived(this->derived()) -= n; }
+
   template <class... Ts>
   bool operator!=(const iterator_adaptor<Ts...>& x) const noexcept {
-    return !this->derived().operator==(x); // equal operator may be overridden in derived
+    return !this->derived().operator==(x);
   }
+
   template <class... Ts>
   bool operator<(const iterator_adaptor<Ts...>& x) const noexcept {
     return iter_ < x.iter_;
   }
+
+  template <class... Ts>
+  bool operator>=(const iterator_adaptor<Ts...>& x) const noexcept {
+    return iter_ >= x.iter_;
+  }
+
   template <class... Ts>
   bool operator>(const iterator_adaptor<Ts...>& x) const noexcept {
     return iter_ > x.iter_;
   }
+
   template <class... Ts>
   bool operator<=(const iterator_adaptor<Ts...>& x) const noexcept {
     return iter_ <= x.iter_;
   }
-  template <class... Ts>
-  bool operator>=(const iterator_adaptor<Ts...>& x) const noexcept {
-    return iter_ >= x.iter_;
-  }
 
   friend Derived operator+(difference_type n, const Derived& x) { return x + n; }
 
   Base const& base() const noexcept { return iter_; }
 
 protected:
-  // for convenience in derived classes
+  // for convenience: refer to base class in derived class
   using iterator_adaptor_ = iterator_adaptor;
 
 private:
   Derived& derived() noexcept { return *static_cast<Derived*>(this); }
   const Derived& derived() const noexcept { return *static_cast<Derived const*>(this); }
 
-  Base iter_;
+  base_type iter_;
 
   template <class, class, class, class>
   friend class iterator_adaptor;
@@ -159,4 +172,4 @@ private:
 } // namespace histogram
 } // namespace boost
 
-#endif
+#endif

test/detail_iterator_adaptor_test.cpp

@@ -17,7 +17,6 @@
 #include "std_ostream.hpp"
 #include "utility_iterator.hpp"
 
-using namespace boost::histogram;
 using boost::histogram::detail::iterator_adaptor;
 
 typedef std::deque<int> storage;
@@ -186,5 +185,34 @@ int main() {
     BOOST_TEST(i - k == 0);
   }
 
+  {
+    using C = std::vector<int>;
+    C a = {{1, 2, 3, 4}};
+
+    struct skip_iterator : iterator_adaptor<skip_iterator, C::iterator> {
+      using difference_type = typename iterator_adaptor_::difference_type;
+      using iterator_adaptor_::iterator_adaptor_;
+
+      skip_iterator& operator+=(difference_type n) {
+        iterator_adaptor_::operator+=(n * 2);
+        return *this;
+      }
+    };
+
+    {
+      skip_iterator it(a.begin());
+      BOOST_TEST_EQ(it.base() - a.begin(), 0);
+      BOOST_TEST_EQ(*it++, 1);
+      BOOST_TEST_EQ(*it++, 3);
+      BOOST_TEST_EQ(a.end() - it.base(), 0);
+    }
+
+    {
+      int i = 0;
+      for (auto it = skip_iterator(a.begin()); it != skip_iterator(a.end()); ++it) ++i;
+      BOOST_TEST_EQ(i, 2);
+    }
+  }
+
   return boost::report_errors();
 }

test/utility_iterator.hpp

@@ -17,9 +17,6 @@
 #include <iterator>
 #include <type_traits>
 
-namespace boost {
-namespace histogram {
-
 namespace detail {
 template <class T>
 struct icast_identity {
@@ -53,14 +50,14 @@ void trivial_iterator_test(const Iterator i, const Iterator j, T val) {
   BOOST_TEST(i != j);
   typename std::iterator_traits<Iterator>::value_type v = *i;
   BOOST_TEST(v == val);
-  ignore_unused(v);
+  boost::ignore_unused(v);
   BOOST_TEST(v == i->foo());
   k = i;
   BOOST_TEST(k == k);
   BOOST_TEST(k == i);
   BOOST_TEST(k != j);
   BOOST_TEST(*k == val);
-  ignore_unused(k);
+  boost::ignore_unused(k);
 }
 
 // Preconditions: i != j
@@ -94,7 +91,7 @@ void input_iterator_test(Iterator i, T v1, T v2) {
   // we cannot test for equivalence of (void)++i & (void)i++
   // as i is only guaranteed to be single pass.
   BOOST_TEST(*i++ == v1);
-  ignore_unused(i1);
+  boost::ignore_unused(i1);
 
   i1 = i;
 
@@ -103,7 +100,7 @@ void input_iterator_test(Iterator i, T v1, T v2) {
 
   BOOST_TEST(*i1 == v2);
   BOOST_TEST(*i == v2);
-  ignore_unused(i1);
+  boost::ignore_unused(i1);
 
   // i is dereferencable, so it must be incrementable.
   ++i;
@@ -176,7 +173,7 @@ void random_access_iterator_test(Iterator i, int N, TrueVals vals) {
   int c;
 
   typedef typename std::iterator_traits<Iterator>::value_type value_type;
-  ignore_unused<value_type>();
+  boost::ignore_unused<value_type>();
 
   for (c = 0; c < N - 1; ++c) {
     BOOST_TEST(i == j + c);
@@ -197,7 +194,7 @@ void random_access_iterator_test(Iterator i, int N, TrueVals vals) {
     BOOST_TEST(*i == vals[N - 1 - c]);
     BOOST_TEST(*i == implicit_cast<value_type>(j[N - 1 - c]));
     Iterator q = k - c;
-    ignore_unused(q);
+    boost::ignore_unused(q);
     BOOST_TEST(*i == *q);
     BOOST_TEST(i > j);
     BOOST_TEST(i >= j);
@@ -220,10 +217,7 @@ void const_nonconst_iterator_test(Iterator i, ConstIterator j) {
   k = i;
   BOOST_TEST(k == i);
   BOOST_TEST(i == k);
-  ignore_unused(k);
+  boost::ignore_unused(k);
 }
 
-} // namespace histogram
-} // namespace boost
-
 #endif // BOOST_HISTOGRAM_TEST_ITERATOR_TESTS_HPP