[libcxx][span] Implement P1976R2

[miscco]

Summary

This resolves the NB comment about the construction of a fixed-size span from a dynamic range

[miscco]

Forgot the explicit for the container constructors

[miscco]

Add failing tests for explicit constructors

[miscco]

gentle ping

[ldionne]

I think you're missing the update of the feature-test macro? eel.is/c++draft says: #define __cpp_lib_span 202002L

libcxx/include/span

67

Could you please adjust the synopsis too?

228

Can you walk me through why we don't have a single _Container&& like in the spec of P1976R2?

[miscco]

Thanks for the review, I am unsure whether one should really update the feature test macro without the other changes in place (tuple interface, relaxed array constructors etc)

If you want to update I wouldnt mind too much though

libcxx/include/span

67

Sure will do

228

Because we do not have library concepts and ranges yet.

The _Container&& constructor you mention is the range constructor that requires ranges::contiguous_range, ranges::sized_range and ranges::borrowed_range.

[miscco]

[libcxx][span] Update synopsis

[miscco]

@ldionne do you require any changes here (especially regarding the feature test macro)

[miscco]

ping

[ldionne]

I approved the other span changes, so the feature test macro can be updated now, right? If so, please update it and go ahead, otherwise please explain :-).

[miscco]

• [libcxx][span] Update synopsis

[miscco]

@ldionne Thanks a lot for the review. I have added the feature test macro as I did not find it anywhere else.

Note that I do not have commit access, so you would need to commit those revisions

[ldionne]

You need to modify the feature test macro in libcxx/utils/generate_feature_test_macro_components.py, and then generate the other files (<version>, the documentation and the tests) from it. More details in libcxx/docs/DesignDocs/FeatureTestMacros.rst.

[miscco]

• [libcxx][span] Update synopsis
• Update feature test macro

[miscco]

@ldionne Thanks a lot for the hint. I updated the generate_feature_test_macro_components.py script and also fixed the incorrect value provided to __cpp_lib_to_array

I do not really know why there are changes to execution. Maybe someone forgot to run the script?

[ldionne]

Can you please rebase this on top of master? I'm failing to test it locally.

[miscco]

Rebase on master

[ldionne]

It looks like the rebase dropped most of your changes.

[miscco]

That is unfortunate, i will restore them later in the evening

[miscco]

Add changes?!

[miscco]

second try

[miscco]

It seems it finally worked. No idea what went wrong as the changes were still in there

[ldionne]

We're almost there! When you run generate_feature_test_macro_components.py, it prints out something like producing new <version> header as /var/folders/10/r6bw68bs5b9gwjtrnl9dz0vm0000gn/T/version.Xe5egJ at the end. You must mv /var/...../version.asdasd libcxx/include/version to replace the current <version> header with that one. Otherwise the __cpp_lib_span macro won't be defined properly. Currently, this causes the tests to fail with this patch applied.

Also, please exclude changes to other tests like execution.version.pass.cpp from this patch -- I'll generate them separately, I think it's an oversight.

[miscco]

Only update spans feature test macro

[miscco]

Thanks for the review. I do not have commit rights though.

[curdeius]

I've updated the status page in commit 182adf120ccffe937d95d5b447f6506f82f615ec. I guess it was an oversight.

[ldionne]

In D74577#2038390, @curdeius wrote:

I've updated the status page in commit 182adf120ccffe937d95d5b447f6506f82f615ec. I guess it was an oversight.

Thank you!

libcxx/include/span

template <size_t N>
         constexpr span(element_type (&arr)[N]) noexcept;
     template <size_t N>
         constexpr span(array<value_type, N>& arr) noexcept;
     template <size_t N>
         constexpr span(const array<value_type, N>& arr) noexcept;
     template <class Container>
         constexpr span(Container& cont);
     template <class Container>
         constexpr span(const Container& cont);
     constexpr span(const span& other) noexcept = default;
     template <class OtherElementType, size_t OtherExtent>
         constexpr span(const span<OtherElementType, OtherExtent>& s) noexcept;
     ~span() noexcept = default;
     constexpr span& operator=(const span& other) noexcept = default;
 
     // [span.sub], span subviews
     template <size_t Count>

 
 // [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;
     constexpr span& operator=(const span&) noexcept = default;
 
-    _LIBCPP_INLINE_VISIBILITY constexpr span(pointer __ptr, size_type __count) : __data{__ptr}
+    _LIBCPP_INLINE_VISIBILITY constexpr explicit span(pointer __ptr, size_type __count) : __data{__ptr}
         { (void)__count; _LIBCPP_ASSERT(_Extent == __count, "size mismatch in span's constructor (ptr, len)"); }
-    _LIBCPP_INLINE_VISIBILITY constexpr span(pointer __f, pointer __l) : __data{__f}
+    _LIBCPP_INLINE_VISIBILITY constexpr explicit span(pointer __f, pointer __l) : __data{__f}
         { (void)__l;     _LIBCPP_ASSERT(_Extent == distance(__f, __l), "size mismatch in span's constructor (ptr, ptr)"); }
 
     _LIBCPP_INLINE_VISIBILITY constexpr span(element_type (&__arr)[_Extent])          noexcept : __data{__arr} {}
     _LIBCPP_INLINE_VISIBILITY constexpr span(      array<value_type, _Extent>& __arr) noexcept : __data{__arr.data()} {}
     _LIBCPP_INLINE_VISIBILITY constexpr span(const array<value_type, _Extent>& __arr) noexcept : __data{__arr.data()} {}
 
+    template <class _Container>
+    _LIBCPP_INLINE_VISIBILITY
+        constexpr explicit span(      _Container& __c,
+            enable_if_t<__is_span_compatible_container<_Container, _Tp>::value, nullptr_t> = nullptr)
+        : __data{_VSTD::data(__c)} {
+            _LIBCPP_ASSERT(_Extent == _VSTD::size(__c), "size mismatch in span's constructor (range)");
+        }
+
+    template <class _Container>
+    _LIBCPP_INLINE_VISIBILITY
+        constexpr explicit span(const _Container& __c,
+            enable_if_t<__is_span_compatible_container<const _Container, _Tp>::value, nullptr_t> = nullptr)
+        : __data{_VSTD::data(__c)} {
+            _LIBCPP_ASSERT(_Extent == _VSTD::size(__c), "size mismatch in span's constructor (range)");
+        }
+
     template <class _OtherElementType>
     _LIBCPP_INLINE_VISIBILITY
         constexpr span(const span<_OtherElementType, _Extent>& __other,
                        enable_if_t<
                           is_convertible_v<_OtherElementType(*)[], element_type (*)[]>,
                           nullptr_t> = nullptr)
         : __data{__other.data()} {}
 
     template <class _OtherElementType>
     _LIBCPP_INLINE_VISIBILITY
-        constexpr span(const span<_OtherElementType, dynamic_extent>& __other,
+        constexpr explicit span(const span<_OtherElementType, dynamic_extent>& __other,
                        enable_if_t<
                           is_convertible_v<_OtherElementType(*)[], element_type (*)[]>,
                           nullptr_t> = nullptr) noexcept
         : __data{__other.data()} { _LIBCPP_ASSERT(_Extent == __other.size(), "size mismatch in span's constructor (other span)"); }
 
 
 //  ~span() noexcept = default;
 
     template <size_t _Count>
     _LIBCPP_INLINE_VISIBILITY
     constexpr span<element_type, _Count> first() const noexcept
     {
         static_assert(_Count <= _Extent, "Count out of range in span::first()");
-        return {data(), _Count};
+        return span<element_type, _Count>{data(), _Count};
     }
 
     template <size_t _Count>
     _LIBCPP_INLINE_VISIBILITY
     constexpr span<element_type, _Count> last() const noexcept
     {
         static_assert(_Count <= _Extent, "Count out of range in span::last()");
-        return {data() + size() - _Count, _Count};
+        return span<element_type, _Count>{data() + size() - _Count, _Count};
     }
 
     _LIBCPP_INLINE_VISIBILITY
     constexpr span<element_type, dynamic_extent> first(size_type __count) const noexcept
     {
         _LIBCPP_ASSERT(__count <= size(), "Count out of range in span::first(count)");
         return {data(), __count};
     }
 
     _LIBCPP_INLINE_VISIBILITY
     constexpr span<element_type, dynamic_extent> last(size_type __count) const noexcept
     {
         _LIBCPP_ASSERT(__count <= size(), "Count out of range in span::last(count)");
         return {data() + size() - __count, __count};
     }
 
     template <size_t _Offset, size_t _Count = dynamic_extent>
     _LIBCPP_INLINE_VISIBILITY
     constexpr auto subspan() const noexcept
         -> span<element_type, _Count != dynamic_extent ? _Count : _Extent - _Offset>
     {
         static_assert(_Offset <= _Extent, "Offset out of range in span::subspan()");
-        return {data() + _Offset, _Count == dynamic_extent ? size() - _Offset : _Count};
+        using _ReturnType = span<element_type, _Count != dynamic_extent ? _Count : _Extent - _Offset>;
+        return _ReturnType{data() + _Offset, _Count == dynamic_extent ? size() - _Offset : _Count};
     }
 
 
     _LIBCPP_INLINE_VISIBILITY
     constexpr span<element_type, dynamic_extent>
        subspan(size_type __offset, size_type __count = dynamic_extent) const noexcept
     {
         _LIBCPP_ASSERT(__offset <= size(), "Offset out of range in span::subspan(offset, count)");
// [span.iter], span iterator support
     _LIBCPP_INLINE_VISIBILITY constexpr void swap(span &__other) noexcept
     {
         pointer __p = __data;
         __data = __other.__data;
         __other.__data = __p;
     }
 
     _LIBCPP_INLINE_VISIBILITY span<const byte, _Extent * sizeof(element_type)> __as_bytes() const noexcept
-    { return {reinterpret_cast<const byte *>(data()), size_bytes()}; }
+    { return span<const byte, _Extent * sizeof(element_type)>{reinterpret_cast<const byte *>(data()), size_bytes()}; }
 
     _LIBCPP_INLINE_VISIBILITY span<byte, _Extent * sizeof(element_type)> __as_writable_bytes() const noexcept
-    { return {reinterpret_cast<byte *>(data()), size_bytes()}; }
+    { return span<byte, _Extent * sizeof(element_type)>{reinterpret_cast<byte *>(data()), size_bytes()}; }
 
 private:
     pointer    __data;
 
 };
 
 
 template <typename _Tp>

 
 //    ~span() noexcept = default;
 
     template <size_t _Count>
     _LIBCPP_INLINE_VISIBILITY
     constexpr span<element_type, _Count> first() const noexcept
     {
         _LIBCPP_ASSERT(_Count <= size(), "Count out of range in span::first()");
-        return {data(), _Count};
+        return span<element_type, _Count>{data(), _Count};
     }
 
     template <size_t _Count>
     _LIBCPP_INLINE_VISIBILITY
     constexpr span<element_type, _Count> last() const noexcept
     {
         _LIBCPP_ASSERT(_Count <= size(), "Count out of range in span::last()");
-        return {data() + size() - _Count, _Count};
+        return span<element_type, _Count>{data() + size() - _Count, _Count};
     }
 
     _LIBCPP_INLINE_VISIBILITY
     constexpr span<element_type, dynamic_extent> first(size_type __count) const noexcept
     {
         _LIBCPP_ASSERT(__count <= size(), "Count out of range in span::first(count)");
         return {data(), __count};
     }
 
     _LIBCPP_INLINE_VISIBILITY
     constexpr span<element_type, dynamic_extent> last (size_type __count) const noexcept
     {
         _LIBCPP_ASSERT(__count <= size(), "Count out of range in span::last(count)");
         return {data() + size() - __count, __count};
     }
 
     template <size_t _Offset, size_t _Count = dynamic_extent>
     _LIBCPP_INLINE_VISIBILITY
     constexpr span<element_type, _Count> subspan() const noexcept
     {
         _LIBCPP_ASSERT(_Offset <= size(), "Offset out of range in span::subspan()");
         _LIBCPP_ASSERT(_Count == dynamic_extent || _Offset + _Count <= size(), "Count out of range in span::subspan()");
-        return {data() + _Offset, _Count == dynamic_extent ? size() - _Offset : _Count};
+        return span<element_type, _Count>{data() + _Offset, _Count == dynamic_extent ? size() - _Offset : _Count};
     }
 
     constexpr span<element_type, dynamic_extent>
     _LIBCPP_INLINE_VISIBILITY
     subspan(size_type __offset, size_type __count = dynamic_extent) const noexcept
     {
         _LIBCPP_ASSERT(__offset <= size(), "Offset out of range in span::subspan(offset, count)");
         _LIBCPP_ASSERT(__count  <= size() || __count == dynamic_extent, "count out of range in span::subspan(offset, count)");

libcxx/test/std/containers/views/span.cons/assign.pass.cpp

 
 //      for (size_t i = 0; i < std::size(spans); ++i)
 //          for (size_t j = i; j < std::size(spans); ++j)
 //              static_assert(doAssign(spans[i], spans[j]), "");
     }
 
 //  constexpr statically sized assignment
     {
-        constexpr std::span<const int,2> spans[] = {
-            {carr1, 2},
-            {carr1 + 1, 2},
-            {carr1 + 2, 2},
-            {carr2, 2},
-            {carr2 + 1, 2},
-            {carr3, 2}
+        using spanType = std::span<const int,2>;
+        constexpr spanType spans[] = {
+            spanType{carr1, 2},
+            spanType{carr1 + 1, 2},
+            spanType{carr1 + 2, 2},
+            spanType{carr2, 2},
+            spanType{carr2 + 1, 2},
+            spanType{carr3, 2}
             };
 
         static_assert(std::size(spans) == 6, "" );
 
 //  No for loops in constexpr land :-(
         static_assert(doAssign(spans[0], spans[0]), "");
         static_assert(doAssign(spans[0], spans[1]), "");
         static_assert(doAssign(spans[0], spans[2]), "");
//  dynamically sized assignment
 
         for (size_t i = 0; i < std::size(spans); ++i)
             for (size_t j = i; j < std::size(spans); ++j)
                 assert((doAssign(spans[i], spans[j])));
     }
 
 //  statically sized assignment
     {
-        std::span<int,2> spans[] = {
-            {arr,     arr + 2},
-            {arr + 1, arr + 3},
-            {arr + 2, arr + 4}
+        using spanType = std::span<int,2>;
+        spanType spans[] = {
+            spanType{arr,     arr + 2},
+            spanType{arr + 1, arr + 3},
+            spanType{arr + 2, arr + 4}
             };
 
         for (size_t i = 0; i < std::size(spans); ++i)
             for (size_t j = i; j < std::size(spans); ++j)
                 assert((doAssign(spans[i], spans[j])));
     }
 
 //  dynamically sized assignment
std::span<std::string> spans[] = {
             };
 
         for (size_t i = 0; i < std::size(spans); ++i)
             for (size_t j = i; j < std::size(spans); ++j)
                 assert((doAssign(spans[i], spans[j])));
     }
 
     {
-    std::span<std::string, 1> spans[] = {
-            {strs,     strs + 1},
-            {strs + 1, strs + 2},
-            {strs + 2, strs + 3}
+    using spanType = std::span<std::string, 1>;
+    spanType spans[] = {
+            spanType{strs,     strs + 1},
+            spanType{strs + 1, strs + 2},
+            spanType{strs + 2, strs + 3}
             };
 
         for (size_t i = 0; i < std::size(spans); ++i)
             for (size_t j = i; j < std::size(spans); ++j)
                 assert((doAssign(spans[i], spans[j])));
     }
 
   return 0;
 }

libcxx/test/std/containers/views/span.cons/container.fail.cpp

//  CV wrong
     std::span<               int> s2{v};    // expected-error {{no matching constructor for initialization of 'std::span<int>'}}
     std::span<               int> s3{cv};   // expected-error {{no matching constructor for initialization of 'std::span<int>'}}
     std::span<const          int> s4{v};    // expected-error {{no matching constructor for initialization of 'std::span<const int>'}}
     std::span<const          int> s5{cv};   // expected-error {{no matching constructor for initialization of 'std::span<const int>'}}
     std::span<      volatile int> s6{c};    // expected-error {{no matching constructor for initialization of 'std::span<volatile int>'}}
     std::span<      volatile int> s7{cv};   // expected-error {{no matching constructor for initialization of 'std::span<volatile int>'}}
     }
 
-//  statically sized
-    {
-  IsAContainer<int> c;
-    std::span<int,1> s1{c}; // expected-error {{no matching constructor for initialization of 'std::span<int, 1>'}}
-    }
-
-
   return 0;
 }

libcxx/test/std/containers/views/span.cons/container.pass.cpp

 template <typename T>
 constexpr bool testConstexprSpan()
 {
     constexpr IsAContainer<const T> val{};
     std::span<const T> s1{val};
     return s1.data() == val.getV() && s1.size() == 1;
 }
 
+template <typename T>
+constexpr bool testConstexprSpanStatic()
+{
+    constexpr IsAContainer<const T> val{};
+    std::span<const T, 1> s1{val};
+    return s1.data() == val.getV() && s1.size() == 1;
+}
 
 template <typename T>
 void testRuntimeSpan()
 {
     IsAContainer<T> val{};
     const IsAContainer<T> cVal;
-    std::span<T>          s1{val};
-    std::span<const T>    s2{cVal};
+    std::span<T>       s1{val};
+    std::span<const T> s2{cVal};
     assert(s1.data() == val.getV()  && s1.size() == 1);
     assert(s2.data() == cVal.getV() && s2.size() == 1);
 }
 
+template <typename T>
+void testRuntimeSpanStatic()
+{
+    IsAContainer<T> val{};
+    const IsAContainer<T> cVal;
+    std::span<T, 1>       s1{val};
+    std::span<const T, 1> s2{cVal};
+    assert(s1.data() == val.getV()  && s1.size() == 1);
+    assert(s2.data() == cVal.getV() && s2.size() == 1);
+}
+
 struct A{};
 
 int main(int, char**)
 {
     static_assert(testConstexprSpan<int>(),    "");
     static_assert(testConstexprSpan<long>(),   "");
     static_assert(testConstexprSpan<double>(), "");
     static_assert(testConstexprSpan<A>(),      "");
 
+    static_assert(testConstexprSpanStatic<int>(),    "");
+    static_assert(testConstexprSpanStatic<long>(),   "");
+    static_assert(testConstexprSpanStatic<double>(), "");
+    static_assert(testConstexprSpanStatic<A>(),      "");
+
     testRuntimeSpan<int>();
     testRuntimeSpan<long>();
     testRuntimeSpan<double>();
     testRuntimeSpan<std::string>();
     testRuntimeSpan<A>();
 
+    testRuntimeSpanStatic<int>();
+    testRuntimeSpanStatic<long>();
+    testRuntimeSpanStatic<double>();
+    testRuntimeSpanStatic<std::string>();
+    testRuntimeSpanStatic<A>();
+
     checkCV();
 
   return 0;
 }