[libcxx][span] Remove const_iterator from std::span

[miscco]

Summary

This implements the resolution to LWG-3320
"span::cbegin/cend methods produce different results than std::[ranges::]cbegin/cend"

[miscco]

Fix whitespaces

[miscco]

clang-format all the files

[ldionne]

Can you please avoid clang-formatting the changes? I can't tell what changed and what didn't. We should agree on a clang-format style for libc++ and clang-format the whole code base at once instead.

[miscco]

My hope was that it would fix the complaints from the Harbormaster. But that doesnt seem to be the case

[miscco]

Do not clang-format the code

[miscco]

span-ing more pings

[miscco]

Ping, this is all red so...

[miscco]

ping, this is now without whitespace changes so I would guess it is ready to go?

[zoecarver]

Looks like there are still white-space changes in the tests. And there are warning from the CI? Are those formatting warnings or "real" warnings?

[miscco]

In D75642#1995120, @zoecarver wrote:

Looks like there are still white-space changes in the tests. And there are warning from the CI? Are those formatting warnings or "real" warnings?

There are some manual whitespace changes, where I removed double spaces that were previously used to align the end and cend versions. @ldionne referred to an attempt to clang-format the file

[miscco]

Thanks for the review, could somebody commit it as I do not have the rights

[ldionne]

libcxx/test/std/containers/views/span.iterators/end.pass.cpp

51

Please watch out for added tabs in the future.

libcxx/include/span

public:
     using value_type = remove_cv_t<ElementType>;
     using size_type = size_t;
     using difference_type = ptrdiff_t;
     using pointer = element_type*;
     using const_pointer = const element_type*;
     using reference = element_type&;
     using const_reference = const element_type&;
     using iterator = implementation-defined;
-    using const_iterator = implementation-defined;
     using reverse_iterator = std::reverse_iterator<iterator>;
-    using const_reverse_iterator = std::reverse_iterator<const_iterator>;
     static constexpr size_type extent = Extent;
 
     // [span.cons], span constructors, copy, assignment, and destructor
     constexpr span() noexcept;
     constexpr span(pointer ptr, size_type count);
     constexpr span(pointer firstElem, pointer lastElem);
     template <size_t N>
         constexpr span(element_type (&arr)[N]) noexcept;
public:
     constexpr reference operator[](size_type idx) const;
     constexpr reference front() const;
     constexpr reference back() const;
     constexpr pointer data() const noexcept;
 
     // [span.iterators], span iterator support
     constexpr iterator begin() const noexcept;
     constexpr iterator end() const noexcept;
-    constexpr const_iterator cbegin() const noexcept;
-    constexpr const_iterator cend() const noexcept;
     constexpr reverse_iterator rbegin() const noexcept;
     constexpr reverse_iterator rend() const noexcept;
-    constexpr const_reverse_iterator crbegin() const noexcept;
-    constexpr const_reverse_iterator crend() const noexcept;
 
 private:
     pointer data_;    // exposition only
     size_type size_;  // exposition only
 };
 
 template<class T, size_t N>
     span(T (&)[N]) -> span<T, N>;
//  constants and types
     using value_type             = remove_cv_t<_Tp>;
     using size_type              = size_t;
     using difference_type        = ptrdiff_t;
     using pointer                = _Tp *;
     using const_pointer          = const _Tp *;
     using reference              = _Tp &;
     using const_reference        = const _Tp &;
     using iterator               =  __wrap_iter<pointer>;
-    using const_iterator         =  __wrap_iter<const_pointer>;
     using reverse_iterator       = _VSTD::reverse_iterator<iterator>;
-    using const_reverse_iterator = _VSTD::reverse_iterator<const_iterator>;
 
     static constexpr size_type extent = _Extent;
 
 // [span.cons], span constructors, copy, assignment, and destructor
     _LIBCPP_INLINE_VISIBILITY constexpr span() noexcept : __data{nullptr}
     { static_assert(_Extent == 0, "Can't default construct a statically sized span with size > 0"); }
 
     constexpr span           (const span&) noexcept = default;
_LIBCPP_INLINE_VISIBILITY constexpr reference back() const noexcept
         return __data[size()-1];
     }
 
     _LIBCPP_INLINE_VISIBILITY constexpr pointer data()                         const noexcept { return __data; }
 
 // [span.iter], span iterator support
     _LIBCPP_INLINE_VISIBILITY constexpr iterator                 begin() const noexcept { return iterator(data()); }
     _LIBCPP_INLINE_VISIBILITY constexpr iterator                   end() const noexcept { return iterator(data() + size()); }
-    _LIBCPP_INLINE_VISIBILITY constexpr const_iterator          cbegin() const noexcept { return const_iterator(data()); }
-    _LIBCPP_INLINE_VISIBILITY constexpr const_iterator            cend() const noexcept { return const_iterator(data() + size()); }
     _LIBCPP_INLINE_VISIBILITY constexpr reverse_iterator        rbegin() const noexcept { return reverse_iterator(end()); }
     _LIBCPP_INLINE_VISIBILITY constexpr reverse_iterator          rend() const noexcept { return reverse_iterator(begin()); }
-    _LIBCPP_INLINE_VISIBILITY constexpr const_reverse_iterator crbegin() const noexcept { return const_reverse_iterator(cend()); }
-    _LIBCPP_INLINE_VISIBILITY constexpr const_reverse_iterator   crend() const noexcept { return const_reverse_iterator(cbegin()); }
 
     _LIBCPP_INLINE_VISIBILITY constexpr void swap(span &__other) noexcept
     {
         pointer __p = __data;
         __data = __other.__data;
         __other.__data = __p;
     }
 
//  constants and types
     using value_type             = remove_cv_t<_Tp>;
     using size_type              = size_t;
     using difference_type        = ptrdiff_t;
     using pointer                = _Tp *;
     using const_pointer          = const _Tp *;
     using reference              = _Tp &;
     using const_reference        = const _Tp &;
     using iterator               =  __wrap_iter<pointer>;
-    using const_iterator         =  __wrap_iter<const_pointer>;
     using reverse_iterator       = _VSTD::reverse_iterator<iterator>;
-    using const_reverse_iterator = _VSTD::reverse_iterator<const_iterator>;
 
     static constexpr size_type extent = dynamic_extent;
 
 // [span.cons], span constructors, copy, assignment, and destructor
     _LIBCPP_INLINE_VISIBILITY constexpr span() noexcept : __data{nullptr}, __size{0} {}
 
     constexpr span           (const span&) noexcept = default;
     constexpr span& operator=(const span&) noexcept = default;
//    ~span() noexcept = default;
     }
 
 
     _LIBCPP_INLINE_VISIBILITY constexpr pointer data()                         const noexcept { return __data; }
 
 // [span.iter], span iterator support
     _LIBCPP_INLINE_VISIBILITY constexpr iterator                 begin() const noexcept { return iterator(data()); }
     _LIBCPP_INLINE_VISIBILITY constexpr iterator                   end() const noexcept { return iterator(data() + size()); }
-    _LIBCPP_INLINE_VISIBILITY constexpr const_iterator          cbegin() const noexcept { return const_iterator(data()); }
-    _LIBCPP_INLINE_VISIBILITY constexpr const_iterator            cend() const noexcept { return const_iterator(data() + size()); }
     _LIBCPP_INLINE_VISIBILITY constexpr reverse_iterator        rbegin() const noexcept { return reverse_iterator(end()); }
     _LIBCPP_INLINE_VISIBILITY constexpr reverse_iterator          rend() const noexcept { return reverse_iterator(begin()); }
-    _LIBCPP_INLINE_VISIBILITY constexpr const_reverse_iterator crbegin() const noexcept { return const_reverse_iterator(cend()); }
-    _LIBCPP_INLINE_VISIBILITY constexpr const_reverse_iterator   crend() const noexcept { return const_reverse_iterator(cbegin()); }
 
     _LIBCPP_INLINE_VISIBILITY constexpr void swap(span &__other) noexcept
     {
         pointer __p = __data;
         __data = __other.__data;
         __other.__data = __p;
 
         size_type __sz = __size;

libcxx/test/std/containers/views/span.iterators/begin.pass.cpp

 #include <string>
 
 #include "test_macros.h"
 
 template <class Span>
 constexpr bool testConstexprSpan(Span s)
 {
     bool ret = true;
-    typename Span::iterator b        = s. begin();
-    typename Span::const_iterator cb = s.cbegin();
+    typename Span::iterator b = s.begin();
 
     if (s.empty())
     {
-        ret = ret &&  ( b ==  s.end());
-        ret = ret &&  (cb == s.cend());
+        ret = ret && (b == s.end());
     }
     else
     {
-        ret = ret &&  (  *b ==  s[0]);
-        ret = ret &&  ( &*b == &s[0]);
-        ret = ret &&  ( *cb ==  s[0]);
-        ret = ret &&  (&*cb == &s[0]);
+        ret = ret && ( *b ==  s[0]);
+        ret = ret && (&*b == &s[0]);
     }
-    ret = ret && (b == cb);
     return ret;
 }
 
 
 template <class Span>
 void testRuntimeSpan(Span s)
 {
-    typename Span::iterator b        = s. begin();
-    typename Span::const_iterator cb = s.cbegin();
+    typename Span::iterator b = s.begin();
 
     if (s.empty())
     {
-        assert( b ==  s.end());
-        assert(cb == s.cend());
+        assert(b == s.end());
     }
     else
     {
-        assert(  *b ==  s[0]);
-        assert( &*b == &s[0]);
-        assert( *cb ==  s[0]);
-        assert(&*cb == &s[0]);
+        assert( *b ==  s[0]);
+        assert(&*b == &s[0]);
     }
-    assert(b == cb);
 }
 
 struct A{};
 bool operator==(A, A) {return true;}
 
 constexpr int iArr1[] = { 0,  1,  2,  3,  4,  5,  6,  7,  8,  9};
           int iArr2[] = {10, 11, 12, 13, 14, 15, 16, 17, 18, 19};
 

libcxx/test/std/containers/views/span.iterators/end.pass.cpp

 #include <string>
 
 #include "test_macros.h"
 
 template <class Span>
 constexpr bool testConstexprSpan(Span s)
 {
     bool ret = true;
-    typename Span::iterator e        = s. end();
-    typename Span::const_iterator ce = s.cend();
+    typename Span::iterator e = s.end();
     if (s.empty())
     {
-        ret = ret &&  ( e ==  s.begin());
-        ret = ret &&  (ce == s.cbegin());
+        ret = ret &&  (e == s.begin());
     }
     else
     {
-      typename Span::const_pointer last = &*(s.cbegin() + s.size() - 1);
-        ret = ret &&  ( e !=  s.begin());
-        ret = ret &&  (ce != s.cbegin());
+      typename Span::const_pointer last = &*(s.begin() + s.size() - 1);
+        ret = ret &&  (e != s.begin());
         ret = ret &&  (&*( e-1) == last);
-        ret = ret &&  (&*(ce-1) == last);
     }
 
-    ret = ret &&  (static_cast<size_t>( e -  s.begin()) == s.size());
-    ret = ret &&  (static_cast<size_t>(ce - s.cbegin()) == s.size());
-
-    ret = ret &&  (e == ce);
+    ret = ret &&  (static_cast<size_t>(e - s.begin()) == s.size());
     return ret;
 }
 
 template <class Span>
 void testRuntimeSpan(Span s)
 {
-    typename Span::iterator e        = s. end();
-    typename Span::const_iterator ce = s.cend();
+    typename Span::iterator e = s.end();
     if (s.empty())
     {
-        assert( e ==  s.begin());
-        assert(ce == s.cbegin());
+        assert(e == s.begin());
     }
     else
     {
-      typename Span::const_pointer last = &*(s.cbegin() + s.size() - 1);
-        assert( e !=  s.begin());
-        assert(ce != s.cbegin());
-        assert( &*( e-1) == last);
-        assert( &*(ce-1) == last);
+      typename Span::const_pointer last = &*(s.begin() + s.size() - 1);
+        assert(e != s.begin());
+        assert(&*( e-1) == last);
     }
 
-    assert(static_cast<size_t>( e -  s.begin()) == s.size());
-    assert(static_cast<size_t>(ce - s.cbegin()) == s.size());
-
-    assert(e == ce);
+    assert(static_cast<size_t>(e - s.begin()) == s.size());
 }
 
 
 struct A{};
 bool operator==(A, A) {return true;}
 
 constexpr int iArr1[] = { 0,  1,  2,  3,  4,  5,  6,  7,  8,  9};
           int iArr2[] = {10, 11, 12, 13, 14, 15, 16, 17, 18, 19};

libcxx/test/std/containers/views/span.iterators/rbegin.pass.cpp

 #include <string>
 
 #include "test_macros.h"
 
 template <class Span>
 constexpr bool testConstexprSpan(Span s)
 {
     bool ret = true;
-    typename Span::reverse_iterator b        = s. rbegin();
-    typename Span::const_reverse_iterator cb = s.crbegin();
+    typename Span::reverse_iterator b = s.rbegin();
     if (s.empty())
     {
         ret = ret &&  ( b ==  s.rend());
-        ret = ret &&  (cb == s.crend());
     }
     else
     {
         const typename Span::size_type last = s.size() - 1;
-        ret = ret &&  (  *b ==  s[last]);
-        ret = ret &&  ( &*b == &s[last]);
-        ret = ret &&  ( *cb ==  s[last]);
-        ret = ret &&  (&*cb == &s[last]);
+        ret = ret && ( *b ==  s[last]);
+        ret = ret && (&*b == &s[last]);
     }
-    ret = ret &&  (b == cb);
     return ret;
 }
 
 
 template <class Span>
 void testRuntimeSpan(Span s)
 {
-    typename Span::reverse_iterator b        = s. rbegin();
-    typename Span::const_reverse_iterator cb = s.crbegin();
+    typename Span::reverse_iterator b = s.rbegin();
     if (s.empty())
     {
-        assert( b ==  s.rend());
-        assert(cb == s.crend());
+        assert(b == s.rend());
     }
     else
     {
         const typename Span::size_type last = s.size() - 1;
-        assert(  *b ==  s[last]);
-        assert( &*b == &s[last]);
-        assert( *cb ==  s[last]);
-        assert(&*cb == &s[last]);
+        assert( *b ==  s[last]);
+        assert(&*b == &s[last]);
     }
-    assert(b == cb);
 }
 
 
 struct A{};
 bool operator==(A, A) {return true;}
 
 constexpr int iArr1[] = { 0,  1,  2,  3,  4,  5,  6,  7,  8,  9};
           int iArr2[] = {10, 11, 12, 13, 14, 15, 16, 17, 18, 19};

libcxx/test/std/containers/views/span.iterators/rend.pass.cpp

 #include <string>
 
 #include "test_macros.h"
 
 template <class Span>
 constexpr bool testConstexprSpan(Span s)
 {
     bool ret = true;
-    typename Span::reverse_iterator e        = s. rend();
-    typename Span::const_reverse_iterator ce = s.crend();
+    typename Span::reverse_iterator e = s.rend();
     if (s.empty())
     {
-        ret = ret &&  ( e ==  s.rbegin());
-        ret = ret &&  (ce == s.crbegin());
+        ret = ret &&  (e == s.rbegin());
     }
     else
     {
-        ret = ret &&  ( e !=  s.rbegin());
-        ret = ret &&  (ce != s.crbegin());
+        ret = ret &&  (e != s.rbegin());
     }
 
-    ret = ret &&  (static_cast<size_t>( e -  s.rbegin()) == s.size());
-    ret = ret &&  (static_cast<size_t>(ce - s.crbegin()) == s.size());
-
-    ret = ret &&  (e == ce);
+    ret = ret &&  (static_cast<size_t>(e - s.rbegin()) == s.size());
     return ret;
 }
 
 template <class Span>
 void testRuntimeSpan(Span s)
 {
-    typename Span::reverse_iterator e        = s. rend();
-    typename Span::const_reverse_iterator ce = s.crend();
+    typename Span::reverse_iterator e = s.rend();
     if (s.empty())
     {
-        assert( e ==  s.rbegin());
-        assert(ce == s.crbegin());
+        assert(e == s.rbegin());
     }
     else
     {
-        assert( e !=  s.rbegin());
-        assert(ce != s.crbegin());
+        assert(e != s.rbegin());
     }
 
-    assert(static_cast<size_t>( e -  s.rbegin()) == s.size());
-    assert(static_cast<size_t>(ce - s.crbegin()) == s.size());
-
-    assert(e == ce);
+    assert(static_cast<size_t>(e - s.rbegin()) == s.size());
 }
 
 
 struct A{};
 bool operator==(A, A) {return true;}
 
 constexpr int iArr1[] = { 0,  1,  2,  3,  4,  5,  6,  7,  8,  9};
           int iArr2[] = {10, 11, 12, 13, 14, 15, 16, 17, 18, 19};

libcxx/test/std/containers/views/types.pass.cpp

void testIterator()
 
     ASSERT_SAME_TYPE(typename ItT::iterator_category, std::random_access_iterator_tag);
     ASSERT_SAME_TYPE(typename ItT::value_type,        typename S::value_type);
     ASSERT_SAME_TYPE(typename ItT::reference,         typename S::reference);
     ASSERT_SAME_TYPE(typename ItT::pointer,           typename S::pointer);
     ASSERT_SAME_TYPE(typename ItT::difference_type,   typename S::difference_type);
 }
 
-template <typename S, typename Iter>
-void testConstIterator()
-{
-    typedef std::iterator_traits<Iter> ItT;
-
-    ASSERT_SAME_TYPE(typename ItT::iterator_category, std::random_access_iterator_tag);
-    ASSERT_SAME_TYPE(typename ItT::value_type,        typename S::value_type);
-//  I'd like to say 'const typename S::pointer' here, but that gives me
-//      a const pointer to a non-const value, which is not what I want.
-    ASSERT_SAME_TYPE(typename ItT::reference,         typename S::element_type const &);
-    ASSERT_SAME_TYPE(typename ItT::pointer,           typename S::element_type const *);
-    ASSERT_SAME_TYPE(typename ItT::difference_type,   typename S::difference_type);
-}
-
 template <typename S, typename ElementType, std::size_t Size>
 void testSpan()
 {
     ASSERT_SAME_TYPE(typename S::element_type,    ElementType);
     ASSERT_SAME_TYPE(typename S::value_type,      std::remove_cv_t<ElementType>);
     ASSERT_SAME_TYPE(typename S::size_type,       std::size_t);
     ASSERT_SAME_TYPE(typename S::difference_type, std::ptrdiff_t);
     ASSERT_SAME_TYPE(typename S::pointer,         ElementType *);
     ASSERT_SAME_TYPE(typename S::const_pointer,   const ElementType *);
     ASSERT_SAME_TYPE(typename S::reference,       ElementType &);
     ASSERT_SAME_TYPE(typename S::const_reference, const ElementType &);
 
     static_assert(S::extent == Size); // check that it exists
 
     testIterator<S, typename S::iterator>();
     testIterator<S, typename S::reverse_iterator>();
-    testConstIterator<S, typename S::const_iterator>();
-    testConstIterator<S, typename S::const_reverse_iterator>();
 }
 
 
 template <typename T>
 void test()
 {
     testSpan<std::span<               T>,                T, std::dynamic_extent>();
     testSpan<std::span<const          T>, const          T, std::dynamic_extent>();

libcxx/www/cxx2a_status.html

-->
   <tr><td><a href="https://wg21.link/LWG3310">3310</a></td><td>Replace <tt>SIZE_MAX</tt> with <tt>numeric_limits&lt;size_t&gt;::max()</tt></td><td>Prague</td><td></td></tr>
   <tr><td><a href="https://wg21.link/LWG3313">3313</a></td><td><tt>join_view::iterator::operator--</tt> is incorrectly constrained</td><td>Prague</td><td></td></tr>
   <tr><td><a href="https://wg21.link/LWG3314">3314</a></td><td>Is stream insertion behavior locale dependent when <tt>Period::type</tt> is <tt>micro</tt>?</td><td>Prague</td><td></td></tr>
   <tr><td><a href="https://wg21.link/LWG3315">3315</a></td><td>Correct Allocator Default Behavior</td><td>Prague</td><td></td></tr>
   <tr><td><a href="https://wg21.link/LWG3316">3316</a></td><td>Correctly define epoch for <tt>utc_clock</tt> / <tt>utc_timepoint</tt></td><td>Prague</td><td></td></tr>
   <tr><td><a href="https://wg21.link/LWG3317">3317</a></td><td>Incorrect <tt>operator&lt;&lt;</tt> for floating-point durations</td><td>Prague</td><td></td></tr>
   <tr><td><a href="https://wg21.link/LWG3318">3318</a></td><td>Clarify whether clocks can represent time before their epoch</td><td>Prague</td><td></td></tr>
   <tr><td><a href="https://wg21.link/LWG3319">3319</a></td><td>Properly reference specification of IANA time zone database</td><td>Prague</td><td></td></tr>
-  <tr><td><a href="https://wg21.link/LWG3320">3320</a></td><td><tt>span::cbegin/cend</tt> methods produce different results than <tt>std::[ranges::]cbegin/cend</tt></td><td>Prague</td><td></td></tr>
+  <tr><td><a href="https://wg21.link/LWG3320">3320</a></td><td><tt>span::cbegin/cend</tt> methods produce different results than <tt>std::[ranges::]cbegin/cend</tt></td><td>Prague</td><td>Complete</td></tr>
   <tr><td><a href="https://wg21.link/LWG3321">3321</a></td><td><tt>uninitialized_construct_using_allocator</tt> should use <tt>construct_at</tt></td><td>Prague</td><td></td></tr>
   <tr><td><a href="https://wg21.link/LWG3323">3323</a></td><td><tt><i>has-tuple-element</i></tt> helper concept needs <tt>convertible_to</tt></td><td>Prague</td><td></td></tr>
   <tr><td><a href="https://wg21.link/LWG3324">3324</a></td><td>Special-case <tt>std::strong/weak/partial_order</tt> for pointers</td><td>Prague</td><td></td></tr>
   <tr><td><a href="https://wg21.link/LWG3325">3325</a></td><td>Constrain return type of transformation function for <tt>transform_view</tt></td><td>Prague</td><td></td></tr>
   <tr><td><a href="https://wg21.link/LWG3326">3326</a></td><td><tt>enable_view</tt> has false positives</td><td>Prague</td><td></td></tr>
   <tr><td><a href="https://wg21.link/LWG3327">3327</a></td><td>Format alignment specifiers vs. text direction</td><td>Prague</td><td></td></tr>
   <tr><td><a href="https://wg21.link/LWG3328">3328</a></td><td>Clarify that <tt>std::string</tt> is not good for UTF-8</td><td>Prague</td><td></td></tr>
   <tr><td><a href="https://wg21.link/LWG3329">3329</a></td><td><tt>totally_ordered_with</tt> both directly and indirectly requires <tt>common_reference_with</tt></td><td>Prague</td><td></td></tr>