From 82389aa630a28ec520b115b9bcb770a5ae495860 Mon Sep 17 00:00:00 2001 From: Neil MacIntosh Date: Mon, 8 Aug 2016 12:06:47 -0700 Subject: [PATCH 1/3] Fixed up iterator implementation to allow conversion from iterator to const_iterator. --- include/span.h | 222 ++++++++----------------------------------- tests/span_tests.cpp | 98 +++++++++++++++---- 2 files changed, 122 insertions(+), 198 deletions(-) diff --git a/include/span.h b/include/span.h index a5d2823..ca35d8c 100644 --- a/include/span.h +++ b/include/span.h @@ -166,159 +166,29 @@ namespace details { }; - template - class const_span_iterator - { - public: - using iterator_category = std::random_access_iterator_tag; - using value_type = typename Span::element_type; - using difference_type = std::ptrdiff_t; - - using const_pointer = std::add_const_t; - using pointer = const_pointer; - - using const_reference = std::add_const_t; - using reference = const_reference; - - constexpr const_span_iterator() : const_span_iterator(nullptr, 0) {} - constexpr const_span_iterator(const Span* span, typename Span::index_type index) - : span_(span), index_(index) - { - Expects(span == nullptr || (index_ >= 0 && index <= span_->length())); - } - - constexpr reference operator*() const - { - Expects(span_); - return (*span_)[index_]; - } - - constexpr pointer operator->() const - { - Expects(span_); - return &((*span_)[index_]); - } - - constexpr const_span_iterator& operator++() noexcept - { - Expects(span_ && index_ >= 0 && index_ < span_->length()); - ++index_; - return *this; - } - - constexpr const_span_iterator operator++(int) noexcept - { - auto ret = *this; - ++(*this); - return ret; - } - - constexpr const_span_iterator& operator--() noexcept - { - Expects(span_ && index_ > 0 && index_ <= span_->length()); - --index_; - return *this; - } - - constexpr const_span_iterator operator--(int) noexcept - { - auto ret = *this; - --(*this); - return ret; - } - - constexpr const_span_iterator operator+(difference_type n) const noexcept - { - auto ret = *this; - return ret += n; - } - - constexpr const_span_iterator& operator+=(difference_type n) noexcept - { - Expects(span_ && (index_ + n) >= 0 && (index_ + n) <= span_->length()); - index_ += n; - return *this; - } - - constexpr const_span_iterator operator-(difference_type n) const noexcept - { - auto ret = *this; - return ret -= n; - } - - constexpr const_span_iterator& operator-=(difference_type n) noexcept - { - return *this += -n; - } - - constexpr difference_type operator-(const const_span_iterator& rhs) const noexcept - { - Expects(span_ == rhs.span_); - return index_ - rhs.index_; - } - - constexpr reference operator[](difference_type n) const noexcept { return *(*this + n); } - - constexpr bool operator==(const const_span_iterator& rhs) const noexcept - { - return span_ == rhs.span_ && index_ == rhs.index_; - } - - constexpr bool operator!=(const const_span_iterator& rhs) const noexcept - { - return !(*this == rhs); - } - - constexpr bool operator<(const const_span_iterator& rhs) const noexcept - { - Expects(span_ == rhs.span_); - return index_ < rhs.index_; - } - - constexpr bool operator<=(const const_span_iterator& rhs) const noexcept - { - return !(rhs < *this); - } - - constexpr bool operator>(const const_span_iterator& rhs) const noexcept - { - return rhs < *this; - } - - constexpr bool operator>=(const const_span_iterator& rhs) const noexcept - { - return !(rhs > *this); - } - - void swap(const_span_iterator& rhs) noexcept - { - std::swap(index_, rhs.index_); - std::swap(span_, rhs.span_); - } - - private: - const Span* span_; - std::ptrdiff_t index_; - }; - - template + template class span_iterator { public: using iterator_category = std::random_access_iterator_tag; - using value_type = typename Span::element_type; - using difference_type = std::ptrdiff_t; + using value_type = std::conditional_t, typename Span::element_type>; + using difference_type = typename Span::index_type; - using pointer = value_type*; - using reference = value_type&; + using pointer = std::add_pointer_t; + using reference = std::add_lvalue_reference_t; - constexpr span_iterator() : span_iterator(nullptr, 0) {} + constexpr span_iterator() : span_iterator(nullptr, 0) noexcept {} + constexpr span_iterator(const Span* span, typename Span::index_type index) : span_(span), index_(index) { Expects(span == nullptr || (index_ >= 0 && index <= span_->length())); } + friend class span_iterator; + constexpr span_iterator(const span_iterator& other) noexcept + : span_iterator(other.span_, other.index_) {} + constexpr reference operator*() const { Expects(span_); @@ -378,7 +248,10 @@ namespace details return ret -= n; } - constexpr span_iterator& operator-=(difference_type n) noexcept { return *this += -n; } + constexpr span_iterator& operator-=(difference_type n) noexcept + { + return *this += -n; + } constexpr difference_type operator-(const span_iterator& rhs) const noexcept { @@ -388,32 +261,35 @@ namespace details constexpr reference operator[](difference_type n) const noexcept { return *(*this + n); } - constexpr bool operator==(const span_iterator& rhs) const noexcept + constexpr friend bool operator==(const span_iterator& lhs, const span_iterator& rhs) noexcept { - return span_ == rhs.span_ && index_ == rhs.index_; + return lhs.span_ == rhs.span_ && lhs.index_ == rhs.index_; } - constexpr bool operator!=(const span_iterator& rhs) const noexcept + constexpr friend bool operator!=(const span_iterator& lhs, const span_iterator& rhs) noexcept { - return !(*this == rhs); + return !(lhs == rhs); } - constexpr bool operator<(const span_iterator& rhs) const noexcept + constexpr friend bool operator<(const span_iterator& lhs, const span_iterator& rhs) noexcept { - Expects(span_ == rhs.span_); - return index_ < rhs.index_; + Expects(lhs.span_ == rhs.span_); + return lhs.index_ < rhs.index_; } - constexpr bool operator<=(const span_iterator& rhs) const noexcept + constexpr friend bool operator<=(const span_iterator& lhs, const span_iterator& rhs) noexcept { - return !(rhs < *this); + return !(rhs < lhs); } - constexpr bool operator>(const span_iterator& rhs) const noexcept { return rhs < *this; } - - constexpr bool operator>=(const span_iterator& rhs) const noexcept + constexpr friend bool operator>(const span_iterator& lhs, const span_iterator& rhs) noexcept { - return !(rhs > *this); + return rhs < lhs; + } + + constexpr friend bool operator>=(const span_iterator& lhs, const span_iterator& rhs) noexcept + { + return !(rhs > lhs); } void swap(span_iterator& rhs) noexcept @@ -422,37 +298,23 @@ namespace details std::swap(span_, rhs.span_); } - private: + protected: const Span* span_; std::ptrdiff_t index_; }; - template - constexpr const_span_iterator - operator+(typename const_span_iterator::difference_type n, - const const_span_iterator& rhs) noexcept + template + constexpr span_iterator + operator+(typename span_iterator::difference_type n, + const span_iterator& rhs) noexcept { return rhs + n; } - template - constexpr const_span_iterator - operator-(typename const_span_iterator::difference_type n, - const const_span_iterator& rhs) noexcept - { - return rhs - n; - } - - template - constexpr span_iterator operator+(typename span_iterator::difference_type n, - const span_iterator& rhs) noexcept - { - return rhs + n; - } - - template - constexpr span_iterator operator-(typename span_iterator::difference_type n, - const span_iterator& rhs) noexcept + template + constexpr span_iterator + operator-(typename span_iterator::difference_type n, + const span_iterator& rhs) noexcept { return rhs - n; } @@ -511,8 +373,8 @@ public: using pointer = element_type*; using reference = element_type&; - using iterator = details::span_iterator>; - using const_iterator = details::const_span_iterator>; + using iterator = details::span_iterator, false>; + using const_iterator = details::span_iterator, true>; using reverse_iterator = std::reverse_iterator; using const_reverse_iterator = std::reverse_iterator; diff --git a/tests/span_tests.cpp b/tests/span_tests.cpp index 8c9829d..4058463 100644 --- a/tests/span_tests.cpp +++ b/tests/span_tests.cpp @@ -787,20 +787,90 @@ SUITE(span_tests) } } - TEST(iterator) + TEST(iterator_default_init) { span::iterator it1; span::iterator it2; CHECK(it1 == it2); } - TEST(const_iterator) + TEST(const_iterator_default_init) { span::const_iterator it1; span::const_iterator it2; CHECK(it1 == it2); } + TEST(iterator_conversions) + { + span::iterator badIt; + span::const_iterator badConstIt; + CHECK(badIt == badConstIt); + + int a[] = { 1, 2, 3, 4 }; + span s = a; + + auto it = s.begin(); + auto cit = s.cbegin(); + + CHECK(it == cit); + CHECK(cit == it); + + span::const_iterator cit2 = it; + CHECK(cit2 == cit); + + span::const_iterator cit3 = it + 4; + CHECK(cit3 == s.cend()); + } + + TEST(iterator_comparisons) + { + int a[] = { 1, 2, 3, 4 }; + { + span s = a; + span::iterator it = s.begin(); + auto it2 = it + 1; + span::const_iterator cit = s.cbegin(); + auto cit2 = s.cbegin(); + + CHECK(it == cit); + CHECK(cit == it); + CHECK(it == it); + CHECK(cit == cit); + CHECK(cit == s.begin()); + CHECK(s.begin() == cit); + CHECK(s.cbegin() == cit); + CHECK(it == s.begin()); + CHECK(s.begin() == it); + + CHECK(it != it2); + CHECK(it2 != it); + CHECK(it != s.end()); + CHECK(it2 != s.end()); + CHECK(s.end() != it); + CHECK(it2 != cit); + CHECK(cit != it2); + + CHECK(it < it2); + CHECK(it <= it2); + CHECK(it2 <= s.end()); + CHECK(it < s.end()); + CHECK(it <= cit); + CHECK(cit <= it); + CHECK(cit < it2); + CHECK(cit <= it2); + CHECK(cit < s.end()); + CHECK(cit <= s.end()); + + CHECK(it2 > it); + CHECK(it2 >= it); + CHECK(s.end() > it2); + CHECK(s.end() >= it2); + CHECK(it2 > cit); + CHECK(it2 >= cit); + } + } + TEST(begin_end) { { @@ -867,25 +937,21 @@ SUITE(span_tests) ++it; CHECK(it - first == 1); CHECK(*it == 2); - *it = 22; - CHECK(*it == 22); CHECK(beyond - it == 3); + int last = 0; it = first; CHECK(it == first); while (it != s.cend()) { - *it = 5; + CHECK(*it == last + 1); + + last = *it; ++it; } CHECK(it == beyond); CHECK(it - beyond == 0); - - for (auto& n : s) - { - CHECK(n == 5); - } } } @@ -955,25 +1021,21 @@ SUITE(span_tests) ++it; CHECK(it - first == 1); CHECK(*it == 3); - *it = 22; - CHECK(*it == 22); CHECK(beyond - it == 3); it = first; CHECK(it == first); + int last = 5; while (it != s.crend()) { - *it = 5; + CHECK(*it == last - 1); + last = *it; + ++it; } CHECK(it == beyond); CHECK(it - beyond == 0); - - for (auto& n : s) - { - CHECK(n == 5); - } } } From 0dd5f56bed0362c84819d84ed1a0d77b025ad422 Mon Sep 17 00:00:00 2001 From: Neil MacIntosh Date: Mon, 8 Aug 2016 13:33:02 -0700 Subject: [PATCH 2/3] Fixed unused variable and ran clang-format. Tested on gcc/clang. --- include/multi_span.h | 6 ++++-- include/span.h | 27 ++++++++++++++++----------- tests/span_tests.cpp | 1 - 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/include/multi_span.h b/include/multi_span.h index c883fb0..b31909d 100644 --- a/include/multi_span.h +++ b/include/multi_span.h @@ -1066,12 +1066,14 @@ struct dim_t }; template -constexpr std::enable_if_t<(N >= 0),dim_t> dim() noexcept { +constexpr std::enable_if_t<(N >= 0), dim_t> dim() noexcept +{ return dim_t(); } template -constexpr std::enable_if_t> dim(std::ptrdiff_t n) noexcept { +constexpr std::enable_if_t> dim(std::ptrdiff_t n) noexcept +{ return dim_t<>(n); } diff --git a/include/span.h b/include/span.h index ca35d8c..5b2e427 100644 --- a/include/span.h +++ b/include/span.h @@ -171,14 +171,16 @@ namespace details { public: using iterator_category = std::random_access_iterator_tag; - using value_type = std::conditional_t, typename Span::element_type>; + using value_type = + std::conditional_t, + typename Span::element_type>; using difference_type = typename Span::index_type; using pointer = std::add_pointer_t; using reference = std::add_lvalue_reference_t; constexpr span_iterator() : span_iterator(nullptr, 0) noexcept {} - + constexpr span_iterator(const Span* span, typename Span::index_type index) : span_(span), index_(index) { @@ -187,7 +189,9 @@ namespace details friend class span_iterator; constexpr span_iterator(const span_iterator& other) noexcept - : span_iterator(other.span_, other.index_) {} + : span_iterator(other.span_, other.index_) + { + } constexpr reference operator*() const { @@ -248,10 +252,7 @@ namespace details return ret -= n; } - constexpr span_iterator& operator-=(difference_type n) noexcept - { - return *this += -n; - } + constexpr span_iterator& operator-=(difference_type n) noexcept { return *this += -n; } constexpr difference_type operator-(const span_iterator& rhs) const noexcept { @@ -261,12 +262,14 @@ namespace details constexpr reference operator[](difference_type n) const noexcept { return *(*this + n); } - constexpr friend bool operator==(const span_iterator& lhs, const span_iterator& rhs) noexcept + constexpr friend bool operator==(const span_iterator& lhs, + const span_iterator& rhs) noexcept { return lhs.span_ == rhs.span_ && lhs.index_ == rhs.index_; } - constexpr friend bool operator!=(const span_iterator& lhs, const span_iterator& rhs) noexcept + constexpr friend bool operator!=(const span_iterator& lhs, + const span_iterator& rhs) noexcept { return !(lhs == rhs); } @@ -277,7 +280,8 @@ namespace details return lhs.index_ < rhs.index_; } - constexpr friend bool operator<=(const span_iterator& lhs, const span_iterator& rhs) noexcept + constexpr friend bool operator<=(const span_iterator& lhs, + const span_iterator& rhs) noexcept { return !(rhs < lhs); } @@ -287,7 +291,8 @@ namespace details return rhs < lhs; } - constexpr friend bool operator>=(const span_iterator& lhs, const span_iterator& rhs) noexcept + constexpr friend bool operator>=(const span_iterator& lhs, + const span_iterator& rhs) noexcept { return !(rhs > lhs); } diff --git a/tests/span_tests.cpp b/tests/span_tests.cpp index 4058463..55610c7 100644 --- a/tests/span_tests.cpp +++ b/tests/span_tests.cpp @@ -831,7 +831,6 @@ SUITE(span_tests) span::iterator it = s.begin(); auto it2 = it + 1; span::const_iterator cit = s.cbegin(); - auto cit2 = s.cbegin(); CHECK(it == cit); CHECK(cit == it); From 32ee66d3201179b261cf498ab41b11966aab6c35 Mon Sep 17 00:00:00 2001 From: Neil MacIntosh Date: Mon, 8 Aug 2016 13:48:12 -0700 Subject: [PATCH 3/3] Added basic tests for std::begin/end and friends (Issue #252). --- tests/span_tests.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/span_tests.cpp b/tests/span_tests.cpp index 55610c7..5e71410 100644 --- a/tests/span_tests.cpp +++ b/tests/span_tests.cpp @@ -872,6 +872,19 @@ SUITE(span_tests) TEST(begin_end) { + { + int a[] = { 1, 2, 3, 4 }; + span s = a; + + span::iterator it = s.begin(); + span::iterator it2 = std::begin(s); + CHECK(it == it2); + + it = s.end(); + it2 = std::end(s); + CHECK(it == it2); + } + { int a[] = { 1, 2, 3, 4 }; span s = a; @@ -916,6 +929,19 @@ SUITE(span_tests) TEST(cbegin_cend) { + { + int a[] = { 1, 2, 3, 4 }; + span s = a; + + span::const_iterator cit = s.cbegin(); + span::const_iterator cit2 = std::cbegin(s); + CHECK(cit == cit2); + + cit = s.cend(); + cit2 = std::cend(s); + CHECK(cit == cit2); + } + { int a[] = {1, 2, 3, 4}; span s = a;