Support iterator conversions as per Issue #252

3 files changed, 152 insertions, 196 deletions

diff --git a/gsl/multi_span b/gsl/multi_span
index fcc53b8..5ddf6fa 100644
--- a/gsl/multi_span
+++ b/gsl/multi_span
@@ -1066,12 +1066,14 @@ struct dim_t<dynamic_range>
 };
 
 template <std::ptrdiff_t N>
-constexpr std::enable_if_t<(N >= 0),dim_t<N>> dim() noexcept {
+constexpr std::enable_if_t<(N >= 0), dim_t<N>> dim() noexcept
+{
     return dim_t<N>();
 }
 
 template <std::ptrdiff_t N = dynamic_range>
-constexpr std::enable_if_t<N == dynamic_range,dim_t<N>> dim(std::ptrdiff_t n) noexcept {
+constexpr std::enable_if_t<N == dynamic_range, dim_t<N>> dim(std::ptrdiff_t n) noexcept
+{
     return dim_t<>(n);
 }
 
diff --git a/gsl/span b/gsl/span
index b6a2f6d..3b8318d 100644
--- a/gsl/span
+++ b/gsl/span
@@ -166,157 +166,31 @@ namespace details
     {
     };
 
-    template <class Span>
-    class const_span_iterator
+    template <class Span, bool IsConst>
+    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<IsConst, std::add_const_t<typename Span::element_type>,
+                               typename Span::element_type>;
+        using difference_type = typename Span::index_type;
 
-        using const_pointer = std::add_const_t<value_type*>;
-        using pointer = const_pointer;
+        using pointer = std::add_pointer_t<value_type>;
+        using reference = std::add_lvalue_reference_t<value_type>;
 
-        using const_reference = std::add_const_t<value_type&>;
-        using reference = const_reference;
+        constexpr span_iterator() : span_iterator(nullptr, 0) noexcept {}
 
-        constexpr const_span_iterator() : const_span_iterator(nullptr, 0) {}
-        constexpr const_span_iterator(const Span* span, typename Span::index_type index)
+        constexpr 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
+        friend class span_iterator<Span, true>;
+        constexpr span_iterator(const span_iterator<Span, false>& other) noexcept
+            : span_iterator(other.span_, other.index_)
         {
-            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 <class Span>
-    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 pointer = value_type*;
-        using reference = value_type&;
-
-        constexpr span_iterator() : span_iterator(nullptr, 0) {}
-        constexpr 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
@@ -388,32 +262,39 @@ 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 friend bool operator>(const span_iterator& lhs, const span_iterator& rhs) noexcept
+        {
+            return rhs < lhs;
+        }
 
-        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);
         }
 
         void swap(span_iterator& rhs) noexcept
@@ -422,37 +303,23 @@ namespace details
             std::swap(span_, rhs.span_);
         }
 
-    private:
+    protected:
         const Span* span_;
         std::ptrdiff_t index_;
     };
 
-    template <typename Span>
-    constexpr const_span_iterator<Span>
-    operator+(typename const_span_iterator<Span>::difference_type n,
-              const const_span_iterator<Span>& rhs) noexcept
-    {
-        return rhs + n;
-    }
-
-    template <typename Span>
-    constexpr const_span_iterator<Span>
-    operator-(typename const_span_iterator<Span>::difference_type n,
-              const const_span_iterator<Span>& rhs) noexcept
-    {
-        return rhs - n;
-    }
-
-    template <typename Span>
-    constexpr span_iterator<Span> operator+(typename span_iterator<Span>::difference_type n,
-                                            const span_iterator<Span>& rhs) noexcept
+    template <class Span, bool IsConst>
+    constexpr span_iterator<Span, IsConst>
+    operator+(typename span_iterator<Span, IsConst>::difference_type n,
+              const span_iterator<Span, IsConst>& rhs) noexcept
     {
         return rhs + n;
     }
 
-    template <typename Span>
-    constexpr span_iterator<Span> operator-(typename span_iterator<Span>::difference_type n,
-                                            const span_iterator<Span>& rhs) noexcept
+    template <class Span, bool IsConst>
+    constexpr span_iterator<Span, IsConst>
+    operator-(typename span_iterator<Span, IsConst>::difference_type n,
+              const span_iterator<Span, IsConst>& rhs) noexcept
     {
         return rhs - n;
     }
@@ -511,8 +378,8 @@ public:
     using pointer = element_type*;
     using reference = element_type&;
 
-    using iterator = details::span_iterator<span<ElementType, Extent>>;
-    using const_iterator = details::const_span_iterator<span<ElementType, Extent>>;
+    using iterator = details::span_iterator<span<ElementType, Extent>, false>;
+    using const_iterator = details::span_iterator<span<ElementType, Extent>, true>;
     using reverse_iterator = std::reverse_iterator<iterator>;
     using const_reverse_iterator = std::reverse_iterator<const_iterator>;
 
diff --git a/tests/span_tests.cpp b/tests/span_tests.cpp
index 8479aa1..05386b2 100644
--- a/tests/span_tests.cpp
+++ b/tests/span_tests.cpp
@@ -787,26 +787,108 @@ SUITE(span_tests)
         }
     }
 
-    TEST(iterator)
+    TEST(iterator_default_init)
     {
         span<int>::iterator it1;
         span<int>::iterator it2;
         CHECK(it1 == it2);
     }
 
-    TEST(const_iterator)
+    TEST(const_iterator_default_init)
     {
         span<int>::const_iterator it1;
         span<int>::const_iterator it2;
         CHECK(it1 == it2);
     }
 
+    TEST(iterator_conversions)
+    {
+        span<int>::iterator badIt;
+        span<int>::const_iterator badConstIt;
+        CHECK(badIt == badConstIt);
+
+        int a[] = { 1, 2, 3, 4 };
+        span<int> s = a;
+
+        auto it = s.begin();
+        auto cit = s.cbegin();
+
+        CHECK(it == cit);
+        CHECK(cit == it);
+
+        span<int>::const_iterator cit2 = it;
+        CHECK(cit2 == cit);
+
+        span<int>::const_iterator cit3 = it + 4;
+        CHECK(cit3 == s.cend());
+    }
+
+    TEST(iterator_comparisons)
+    {
+        int a[] = { 1, 2, 3, 4 };
+        {
+            span<int> s = a;
+            span<int>::iterator it = s.begin();
+            auto it2 = it + 1;
+            span<int>::const_iterator cit = 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)
     {
         {
             int a[] = { 1, 2, 3, 4 };
             span<int> s = a;
 
+            span<int>::iterator it = s.begin();
+            span<int>::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<int> s = a;
+
             auto it = s.begin();
             auto first = it;
             CHECK(it == first);
@@ -848,6 +930,19 @@ SUITE(span_tests)
     TEST(cbegin_cend)
     {
         {
+            int a[] = { 1, 2, 3, 4 };
+            span<int> s = a;
+
+            span<int>::const_iterator cit = s.cbegin();
+            span<int>::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<int> s = a;
 
@@ -867,25 +962,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 +1046,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);
-            }
         }
     }