From a4654a46b535100f53143b6c616425cf293b6a3b Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Fri, 16 Oct 2015 12:15:22 -0700 Subject: [PATCH] Removed arrow_proxy class, fixes bugs in reverse bounds_iterator --- include/array_view.h | 269 +++++++++++++++++++------------------ tests/array_view_tests.cpp | 63 ++++++++- 2 files changed, 199 insertions(+), 133 deletions(-) diff --git a/include/array_view.h b/include/array_view.h index cc7ab23..a31efd8 100644 --- a/include/array_view.h +++ b/include/array_view.h @@ -74,25 +74,6 @@ namespace details { static const SizeType max_value = std::is_signed::value ? static_cast::type>(-1) / 2 : static_cast(-1); }; - - template - class arrow_proxy - { - public: - explicit arrow_proxy(T t) - : val(t) - {} - const T operator*() const noexcept - { - return val; - } - const T* operator->() const noexcept - { - return &val; - } - private: - T val; - }; } template @@ -730,8 +711,9 @@ public: using size_type = SizeType; using index_type = index; - using iterator = bounds_iterator; - using const_iterator = bounds_iterator; + using const_index_type = std::add_const_t; + using iterator = bounds_iterator; + using const_iterator = bounds_iterator; using difference_type = ptrdiff_t; using sliced_type = static_bounds; using mapping_type = contiguous_mapping_tag; @@ -822,7 +804,7 @@ public: constexpr const_iterator begin() const noexcept { - return const_iterator(*this); + return const_iterator(*this, index_type{}); } constexpr const_iterator end() const noexcept @@ -845,8 +827,9 @@ public: using difference_type = SizeType; using value_type = SizeType; using index_type = index; - using iterator = bounds_iterator; - using const_iterator = bounds_iterator; + using const_index_type = std::add_const_t; + using iterator = bounds_iterator; + using const_iterator = bounds_iterator; static const int dynamic_rank = rank; static const size_t static_size = dynamic_range; using sliced_type = std::conditional_t, void>; @@ -920,11 +903,11 @@ public: { return m_extents; } - const_iterator begin() const noexcept + constexpr const_iterator begin() const noexcept { - return const_iterator{ *this }; + return const_iterator{ *this, index_type{} }; } - const_iterator end() const noexcept + constexpr const_iterator end() const noexcept { return const_iterator{ *this, index_bounds() }; } @@ -941,15 +924,11 @@ template struct is_bounds> : std::integral_constant {}; template -class bounds_iterator - : public std::iterator, - const IndexType> +class bounds_iterator: public std::iterator { private: - using Base = std::iterator , const IndexType>; + using Base = std::iterator ; + public: static const size_t rank = IndexType::rank; using typename Base::reference; @@ -959,79 +938,88 @@ public: using index_type = value_type; using index_size_type = typename IndexType::value_type; template - explicit bounds_iterator(const Bounds& bnd, value_type curr = value_type{}) noexcept - : boundary(bnd.index_bounds()) - , curr(std::move(curr)) + explicit bounds_iterator(const Bounds& bnd, value_type curr) noexcept + : boundary(bnd.index_bounds()), curr(std::move(curr)) { static_assert(is_bounds::value, "Bounds type must be provided"); } - reference operator*() const noexcept + + constexpr reference operator*() const noexcept { return curr; } - pointer operator->() const noexcept + + constexpr pointer operator->() const noexcept { - return details::arrow_proxy{ curr }; + return &curr; } - bounds_iterator& operator++() noexcept + + constexpr bounds_iterator& operator++() noexcept { for (size_t i = rank; i-- > 0;) { - if (++curr[i] < boundary[i]) + if (curr[i] < boundary[i] - 1) { + curr[i]++; return *this; } - else - { - curr[i] = 0; - } + curr[i] = 0; } // If we're here we've wrapped over - set to past-the-end. - for (size_t i = 0; i < rank; ++i) - { - curr[i] = boundary[i]; - } + curr = boundary; return *this; } - bounds_iterator operator++(int) noexcept + + constexpr bounds_iterator operator++(int) noexcept { auto ret = *this; ++(*this); return ret; } - bounds_iterator& operator--() noexcept + + constexpr bounds_iterator& operator--() noexcept { - for (size_t i = rank; i-- > 0;) + if (!less(curr, boundary)) { - if (curr[i]-- > 0) - { - return *this; - } - else + // if at the past-the-end, set to last element + for (size_t i = 0; i < rank; ++i) { curr[i] = boundary[i] - 1; } + return *this; + } + for (size_t i = rank; i-- > 0;) + { + if (curr[i] >= 1) + { + curr[i]--; + return *this; + } + curr[i] = boundary[i] - 1; } // If we're here the preconditions were violated // "pre: there exists s such that r == ++s" fail_fast_assert(false); return *this; } - bounds_iterator operator--(int) noexcept + + constexpr bounds_iterator operator--(int) noexcept { auto ret = *this; --(*this); return ret; } - bounds_iterator operator+(difference_type n) const noexcept + + constexpr bounds_iterator operator+(difference_type n) const noexcept { bounds_iterator ret{ *this }; return ret += n; } - bounds_iterator& operator+=(difference_type n) noexcept + + constexpr bounds_iterator& operator+=(difference_type n) noexcept { auto linear_idx = linearize(curr) + n; - value_type stride; + std::remove_const_t stride; stride[rank - 1] = 1; for (size_t i = rank - 1; i-- > 0;) { @@ -1042,76 +1030,84 @@ public: curr[i] = linear_idx / stride[i]; linear_idx = linear_idx % stride[i]; } + fail_fast_assert(!less(curr, index_type{}) && !less(boundary, curr), "index is out of bounds of the array"); return *this; } - bounds_iterator operator-(difference_type n) const noexcept + + constexpr bounds_iterator operator-(difference_type n) const noexcept { bounds_iterator ret{ *this }; return ret -= n; } - bounds_iterator& operator-=(difference_type n) noexcept + + constexpr bounds_iterator& operator-=(difference_type n) noexcept { return *this += -n; } - difference_type operator-(const bounds_iterator& rhs) const noexcept + + constexpr difference_type operator-(const bounds_iterator& rhs) const noexcept { return linearize(curr) - linearize(rhs.curr); } - reference operator[](difference_type n) const noexcept + + constexpr reference operator[](difference_type n) const noexcept { return *(*this + n); } - bool operator==(const bounds_iterator& rhs) const noexcept + + constexpr bool operator==(const bounds_iterator& rhs) const noexcept { return curr == rhs.curr; } - bool operator!=(const bounds_iterator& rhs) const noexcept + + constexpr bool operator!=(const bounds_iterator& rhs) const noexcept { return !(*this == rhs); } - bool operator<(const bounds_iterator& rhs) const noexcept + + constexpr bool operator<(const bounds_iterator& rhs) const noexcept { - for (size_t i = 0; i < rank; ++i) - { - if (curr[i] < rhs.curr[i]) - return true; - } - return false; + return less(curr, rhs.curr); } - bool operator<=(const bounds_iterator& rhs) const noexcept + + constexpr bool operator<=(const bounds_iterator& rhs) const noexcept { return !(rhs < *this); } - bool operator>(const bounds_iterator& rhs) const noexcept + + constexpr bool operator>(const bounds_iterator& rhs) const noexcept { return rhs < *this; } - bool operator>=(const bounds_iterator& rhs) const noexcept + + constexpr bool operator>=(const bounds_iterator& rhs) const noexcept { return !(rhs > *this); } + void swap(bounds_iterator& rhs) noexcept { std::swap(boundary, rhs.boundary); std::swap(curr, rhs.curr); } private: - index_size_type linearize(const value_type& idx) const noexcept + constexpr bool less(index_type& one, index_type& other) const noexcept + { + for (size_t i = 0; i < rank; ++i) + { + if (one[i] < other[i]) + return true; + } + return false; + } + + constexpr index_size_type linearize(const value_type& idx) const noexcept { // TODO: Smarter impl. // Check if past-the-end - bool pte = true; - for (size_t i = 0; i < rank; ++i) - { - if (idx[i] != boundary[i]) - { - pte = false; - break; - } - } index_size_type multiplier = 1; index_size_type res = 0; - if (pte) + if (!less(idx, boundary)) { res = 1; for (size_t i = rank; i-- > 0;) @@ -1130,19 +1126,15 @@ private: } return res; } + value_type boundary; - value_type curr; + std::remove_const_t curr; }; template -class bounds_iterator> - : public std::iterator, - ptrdiff_t, - const details::arrow_proxy>, - const index<1, SizeType>> +class bounds_iterator> : public std::iterator> { - using Base = std::iterator, ptrdiff_t, const details::arrow_proxy>, const index<1, SizeType>>; + using Base = std::iterator>; public: using typename Base::reference; @@ -1153,96 +1145,116 @@ public: using index_size_type = typename index_type::value_type; template - explicit bounds_iterator(const Bounds &, value_type curr = value_type{}) noexcept - : curr( std::move(curr) ) + constexpr explicit bounds_iterator(const Bounds&, value_type curr) noexcept + : curr(std::move(curr)) {} - reference operator*() const noexcept + + constexpr reference operator*() const noexcept { return curr; } - pointer operator->() const noexcept + + constexpr pointer operator->() const noexcept { - return details::arrow_proxy{ curr }; + &curr; } - bounds_iterator& operator++() noexcept + + constexpr bounds_iterator& operator++() noexcept { ++curr; return *this; } - bounds_iterator operator++(int) noexcept + + constexpr bounds_iterator operator++(int) noexcept { auto ret = *this; ++(*this); return ret; } - bounds_iterator& operator--() noexcept + + constexpr bounds_iterator& operator--() noexcept { curr--; return *this; } - bounds_iterator operator--(int) noexcept + + constexpr bounds_iterator operator--(int) noexcept { auto ret = *this; --(*this); return ret; } - bounds_iterator operator+(difference_type n) const noexcept + + constexpr bounds_iterator operator+(difference_type n) const noexcept { bounds_iterator ret{ *this }; return ret += n; } - bounds_iterator& operator+=(difference_type n) noexcept + + constexpr bounds_iterator& operator+=(difference_type n) noexcept { curr += n; return *this; } - bounds_iterator operator-(difference_type n) const noexcept + + constexpr bounds_iterator operator-(difference_type n) const noexcept { bounds_iterator ret{ *this }; return ret -= n; } - bounds_iterator& operator-=(difference_type n) noexcept + + constexpr bounds_iterator& operator-=(difference_type n) noexcept { return *this += -n; } - difference_type operator-(const bounds_iterator& rhs) const noexcept + + constexpr difference_type operator-(const bounds_iterator& rhs) const noexcept { return curr[0] - rhs.curr[0]; } - reference operator[](difference_type n) const noexcept + + constexpr reference operator[](difference_type n) const noexcept { return curr + n; } - bool operator==(const bounds_iterator& rhs) const noexcept + + constexpr bool operator==(const bounds_iterator& rhs) const noexcept { return curr == rhs.curr; } - bool operator!=(const bounds_iterator& rhs) const noexcept + + constexpr bool operator!=(const bounds_iterator& rhs) const noexcept { return !(*this == rhs); } - bool operator<(const bounds_iterator& rhs) const noexcept + + constexpr bool operator<(const bounds_iterator& rhs) const noexcept { return curr[0] < rhs.curr[0]; } - bool operator<=(const bounds_iterator& rhs) const noexcept + + constexpr bool operator<=(const bounds_iterator& rhs) const noexcept { return !(rhs < *this); } - bool operator>(const bounds_iterator& rhs) const noexcept + + constexpr bool operator>(const bounds_iterator& rhs) const noexcept { return rhs < *this; } - bool operator>=(const bounds_iterator& rhs) const noexcept + + constexpr bool operator>=(const bounds_iterator& rhs) const noexcept { return !(rhs > *this); } - void swap(bounds_iterator& rhs) noexcept + + constexpr void swap(bounds_iterator& rhs) noexcept { std::swap(curr, rhs.curr); } + private: - value_type curr; + std::remove_const_t curr; }; template @@ -1304,10 +1316,11 @@ public: using size_type = typename bounds_type::size_type; using index_type = typename bounds_type::index_type; using value_type = ValueType; + using const_value_type = std::add_const_t; using pointer = ValueType*; using reference = ValueType&; using iterator = std::conditional_t::value, contiguous_array_view_iterator, general_array_view_iterator>; - using const_iterator = std::conditional_t::value, contiguous_array_view_iterator>, general_array_view_iterator>>; + using const_iterator = std::conditional_t::value, contiguous_array_view_iterator>, general_array_view_iterator>>; using reverse_iterator = std::reverse_iterator; using const_reverse_iterator = std::reverse_iterator; using sliced_type = std::conditional_t>; @@ -1360,7 +1373,7 @@ public: } constexpr iterator end() const { - return iterator {this}; + return iterator {this, false}; } constexpr const_iterator cbegin() const { @@ -1368,7 +1381,7 @@ public: } constexpr const_iterator cend() const { - return const_iterator {reinterpret_cast *>(this)}; + return const_iterator {reinterpret_cast *>(this), false}; } constexpr reverse_iterator rbegin() const @@ -1999,8 +2012,8 @@ private: { fail_fast_assert(m_pdata >= m_validator->m_pdata && m_pdata < m_validator->m_pdata + m_validator->size(), "iterator is out of range of the array"); } - contiguous_array_view_iterator (const ArrayView *container, bool isbegin = false) : - m_pdata(isbegin ? container->m_pdata : container->m_pdata + container->size()), m_validator(container) { } + contiguous_array_view_iterator (const ArrayView *container, bool isbegin) : + m_pdata(isbegin ? container->m_pdata : container->m_pdata + container->size()), m_validator(container) {} public: reference operator*() const noexcept { @@ -2115,16 +2128,16 @@ private: friend class basic_array_view; const ArrayView * m_container; typename ArrayView::bounds_type::iterator m_itr; - general_array_view_iterator(const ArrayView *container, bool isbegin = false) : + general_array_view_iterator(const ArrayView *container, bool isbegin) : m_container(container), m_itr(isbegin ? m_container->bounds().begin() : m_container->bounds().end()) { } public: - reference operator*() const noexcept + reference operator*() noexcept { return (*m_container)[*m_itr]; } - pointer operator->() const noexcept + pointer operator->() noexcept { return &(*m_container)[*m_itr]; } diff --git a/tests/array_view_tests.cpp b/tests/array_view_tests.cpp index f84e908..cf83fd5 100644 --- a/tests/array_view_tests.cpp +++ b/tests/array_view_tests.cpp @@ -925,11 +925,35 @@ SUITE(array_view_tests) } } - size_t idx = 0; - for (auto num : section) + size_t check_sum = 0; + for (size_t i = 0; i < length; ++i) { - CHECK(num == av[idx][1]); - idx++; + check_sum += av[i][1]; + } + + { + size_t idx = 0; + size_t sum = 0; + for (auto num : section) + { + CHECK(num == av[idx][1]); + sum += num; + idx++; + } + + CHECK(sum == check_sum); + } + { + size_t idx = length - 1; + size_t sum = 0; + for (auto iter = section.rbegin(); iter != section.rend(); ++iter) + { + CHECK(*iter == av[idx][1]); + sum += *iter; + idx--; + } + + CHECK(sum == check_sum); } } @@ -1688,7 +1712,7 @@ SUITE(array_view_tests) CHECK_THROW(f(), fail_fast); } - TEST(AsWriteableBytes) + TEST(AsWriteableBytes) { int a[] = { 1, 2, 3, 4 }; @@ -1714,7 +1738,36 @@ SUITE(array_view_tests) CHECK(wav.data() == (byte*)&a[0]); CHECK(wav.length() == sizeof(a)); } + } + TEST(NonConstIterator) + { + int a[] = { 1, 2, 3, 4 }; + + { + array_view av = a; + auto wav = av.as_writeable_bytes(); + for (auto& b : wav) + { + b = byte(0); + } + for (size_t i = 0; i < 4; ++i) + { + CHECK(a[i] == 0); + } + } + + { + array_view av = a; + for (auto& n : av) + { + n = 1; + } + for (size_t i = 0; i < 4; ++i) + { + CHECK(a[i] == 1); + } + } } TEST(ArrayViewComparison)