From 925a0f748af6e0cd857bc6c9aac61798f45b3829 Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Mon, 2 Jul 2018 10:54:38 -0600 Subject: [PATCH] Make NestedIterator an InputIterator. The previous declaration that it was a RandomAccessIterator was incorrect, as it did not make the multi-pass algorithm guarantee. --- CHANGELOG.md | 7 ++ include/ndarray/detail/NestedIterator.h | 139 ++++++++++++++++++------ 2 files changed, 110 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e350d59f..2a8d8e06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # ndarray change log +## 1.5.2 + +### Bug fixes + +Correctly declared higher-dimensional (>1) iterators to be InputIterators, +not RandomAccessIterators. + ## 1.5.1 ### Bug fixes diff --git a/include/ndarray/detail/NestedIterator.h b/include/ndarray/detail/NestedIterator.h index d13605c1..5543fb48 100644 --- a/include/ndarray/detail/NestedIterator.h +++ b/include/ndarray/detail/NestedIterator.h @@ -17,7 +17,6 @@ * @brief Definition of NestedIterator. */ -#include #include "ndarray_fwd.h" namespace ndarray { @@ -26,37 +25,25 @@ namespace detail { /** * @internal @brief Nested-array iterator class for Array with ND > 1. * - * While this iterator makes use of boost::iterator_facade, it - * reimplements the actual dereferencing operations (operator*, - * operator->) to return Reference const & and - * Reference const *, even though these are - * only convertible to the reference and - * pointer types associated with the iterator, - * not the types themselves. + * NestedIterator does not support multi-pass algorithms, so it is formally + * just an InputIterator. It nevertheless supports efficient indexing, + * arithmetic, and comparison operations of the sort provided by + * RandomAccessIterator. * * @ingroup ndarrayInternalGroup */ template -class NestedIterator : public boost::iterator_facade< - NestedIterator, - typename ArrayTraits::Value, - boost::random_access_traversal_tag, - typename ArrayTraits::Reference - > -{ +class NestedIterator { public: - typedef typename ArrayTraits::Value Value; - typedef typename ArrayTraits::Reference Reference; - - Reference operator[](Size n) const { - Reference r(_ref); - r._data += n * _stride; - return r; - } - Reference const & operator*() const { return _ref; } + typedef typename ArrayTraits::Value value_type; + typedef typename ArrayTraits::Reference const & reference; + typedef typename ArrayTraits::Reference const * pointer; + typedef Offset difference_type; + typedef std::input_iterator_tag iterator_category; - Reference const * operator->() { return &_ref; } + typedef typename ArrayTraits::Value Value; + typedef typename ArrayTraits::Reference Reference; NestedIterator() : _ref(Value()), _stride(0) {} @@ -67,6 +54,19 @@ class NestedIterator : public boost::iterator_facade< template NestedIterator(NestedIterator const & other) : _ref(other._ref), _stride(other._stride) {} + // Return Reference (ArrayRef) instead of Value (Array) so + // `iter[n] = ` copies values, not pointers. Can't + // return (lowercase) reference because it's a temporary. + Reference operator[](Size n) const { + Reference r(_ref); + r._data += n * _stride; + return r; + } + + reference operator*() const { return _ref; } + + pointer operator->() { return &_ref; } + NestedIterator & operator=(NestedIterator const & other) { if (&other != this) { _ref._data = other._ref._data; @@ -84,28 +84,95 @@ class NestedIterator : public boost::iterator_facade< return *this; } -private: + template + bool operator==(NestedIterator const & other) const { + return _ref._data == other._ref._data; + } - friend class boost::iterator_core_access; + template + bool operator!=(NestedIterator const & other) const { + return !(*this == other); + } - template friend class NestedIterator; + template + bool operator<(NestedIterator const & other) const { + return _ref._data < other._ref._data; + } - Reference const & dereference() const { return _ref; } + template + bool operator>(NestedIterator const & other) const { + return _ref._data > other._ref._data; + } - void increment() { _ref._data += _stride; } - void decrement() { _ref._data -= _stride; } - void advance(Offset n) { _ref._data += _stride * n; } + template + bool operator<=(NestedIterator const & other) const { + return _ref._data <= other._ref._data; + } template - Offset distance_to(NestedIterator const & other) const { - return std::distance(_ref._data, other._ref._data) / _stride; + bool operator>=(NestedIterator const & other) const { + return _ref._data >= other._ref._data; + } + + NestedIterator & operator++() { + _ref._data += _stride; + return *this; + } + + NestedIterator & operator--() { + _ref._data -= _stride; + return *this; + } + + NestedIterator operator++(int) { + NestedIterator copy(*this); + ++(*this); + return copy; + } + + NestedIterator operator--(int) { + NestedIterator copy(*this); + --(*this); + return copy; + } + + NestedIterator & operator+=(difference_type n) { + _ref._data += _stride*n; + return *this; + } + + NestedIterator & operator-=(difference_type n) { + _ref._data -= _stride*n; + return *this; + } + + NestedIterator operator+(difference_type n) { + NestedIterator copy(*this); + copy += n; + return copy; + } + + NestedIterator operator-(difference_type n) { + NestedIterator copy(*this); + copy -= n; + return copy; + } + + friend NestedIterator operator+(difference_type n, NestedIterator const & self) { + NestedIterator copy(self); + copy += n; + return copy; } template - bool equal(NestedIterator const & other) const { - return _ref._data == other._ref._data; + difference_type operator-(NestedIterator const & other) { + return (_ref.data - other._ref._data) / _stride; } +private: + + template friend class NestedIterator; + Reference _ref; Offset _stride; };