[libc++] Add workaround to avoid breaking users of <span> when <ranges> are disabled

[ldionne]

Summary

Back in 3a208c68942e, we implemented the range-based constructor for <span>.
However, in doing so, we removed a previous non-standard constructor that
we provided before shipping <ranges>. Unfortunately, that breaks code that
was relying on a range-based constructor until we ship all of <ranges>.

This patch reintroduces the old non-conforming constructors and tests
that were removed in 3a208c68942e and uses them whenever <ranges> is
not provided (e.g. in LLVM 14). This is only a temporary workaround
until we enable <ranges> by default in C++20, which should hopefully
happen by LLVM 15.

The goal is to cherry-pick this workaround back to the LLVM 14 release
branch, since I suspect the constructor removal may otherwise cause
breakage out there, like the breakage I saw internally.

We could have avoided this situation by waiting for C++20 to be finalized
before shipping std::span. For example, we could have guarded it with
something like _LIBCPP_HAS_NO_INCOMPLETE_RANGES to prevent users from
accidentally starting to depend on it before it is stable. We did not
have these mechanisms when std::span was first implemented, though.

[ldionne]

Poke CI to try to avoid flake.

[jloser]

Seems reasonable to me. I left a nit, but do we also need to restore support for deduction guides in the incomplete ranges case? E.g., prior to the range constructor, we had:

template<class _Container>
    span(_Container&) -> span<typename _Container::value_type>;

template<class _Container>
    span(const _Container&) -> span<const typename _Container::value_type>;

If so, please add them back along with associated tests.

libcxx/include/span

275

Nit: I think we (mostly) use west-const everywhere, right? So, s/_Container const/const _Container/g for example.

[ldionne]

Address review comment and poke CI. The GCC failure was a flake.

[ldionne]

Fix windows CI.

[ldionne]

In D121626#3381033, @jloser wrote:

Seems reasonable to me. I left a nit, but do we also need to restore support for deduction guides in the incomplete ranges case? E.g., prior to the range constructor, we had:

template<class _Container>
    span(_Container&) -> span<typename _Container::value_type>;

template<class _Container>
    span(const _Container&) -> span<const typename _Container::value_type>;

If so, please add them back along with associated tests.

I would be tempted to say that it's OK to omit the deduction guides -- I don't expect those to have nearly as much usage, and we're breaking them slightly anyway, since we're going from Container::value_type to range_reference_t<Container> for the type stored in the span when <ranges> is implemented.

[ldionne]

I'll ship this so I can cherry-pick it onto release/14.x ASAP.

libcxx/include/span

 struct __is_std_array<array<_Tp, _Sz>> : true_type {};
 
 template <class _Tp>
 struct __is_std_span : false_type {};
 
 template <class _Tp, size_t _Sz>
 struct __is_std_span<span<_Tp, _Sz>> : true_type {};
 
-#if !defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
+#if defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
+// This is a temporary workaround until we ship <ranges> -- we've unfortunately been
+// shipping <span> before its API was finalized, and we used to provide a constructor
+// from container types that had the requirements below. To avoid breaking code that
+// has started relying on the range-based constructor until we ship all of <ranges>,
+// we emulate the constructor requirements like this.
+template <class _Range, class _ElementType>
+concept __span_compatible_range =
+  !__is_std_span<remove_cvref_t<_Range>>::value  &&
+  !__is_std_array<remove_cvref_t<_Range>>::value &&
+  !is_array_v<remove_cvref_t<_Range>> &&
+  requires (_Range&& __r) {
+      data(std::forward<_Range>(__r));
+      size(std::forward<_Range>(__r));
+  } &&
+  is_convertible_v<remove_reference_t<ranges::range_reference_t<_Range>>(*)[], _ElementType(*)[]>;
+#else
 template <class _Range, class _ElementType>
 concept __span_compatible_range =
   ranges::contiguous_range<_Range> &&
   ranges::sized_range<_Range> &&
   (ranges::borrowed_range<_Range> || is_const_v<_ElementType>) &&
   !__is_std_span<remove_cvref_t<_Range>>::value  &&
   !__is_std_array<remove_cvref_t<_Range>>::value &&
   !is_array_v<remove_cvref_t<_Range>> &&
// [span.cons], span constructors, copy, assignment, and destructor
     _LIBCPP_INLINE_VISIBILITY
     constexpr span(array<_OtherElementType, _Extent>& __arr) noexcept : __data{__arr.data()} {}
 
     template <class _OtherElementType,
               enable_if_t<is_convertible_v<const _OtherElementType(*)[], element_type (*)[]>, nullptr_t> = nullptr>
     _LIBCPP_INLINE_VISIBILITY
     constexpr span(const array<_OtherElementType, _Extent>& __arr) noexcept : __data{__arr.data()} {}
 
-#if !defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
+#if defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
+    template <class _Container>
+        requires __span_compatible_range<_Container, element_type>
+    _LIBCPP_INLINE_VISIBILITY
+    constexpr explicit span(_Container& __c) : __data{std::data(__c)} {
+      _LIBCPP_ASSERT(std::size(__c) == _Extent, "size mismatch in span's constructor (range)");
+    }
+    template <class _Container>
+        requires __span_compatible_range<const _Container, element_type>
+    _LIBCPP_INLINE_VISIBILITY
+    constexpr explicit span(const _Container& __c) : __data{std::data(__c)} {
+      _LIBCPP_ASSERT(std::size(__c) == _Extent, "size mismatch in span's constructor (range)");
+    }
+#else
     template <__span_compatible_range<element_type> _Range>
     _LIBCPP_INLINE_VISIBILITY
     constexpr explicit span(_Range&& __r) : __data{ranges::data(__r)} {
       _LIBCPP_ASSERT(ranges::size(__r) == _Extent, "size mismatch in span's constructor (range)");
     }
 #endif
 
     template <class _OtherElementType>
// [span.cons], span constructors, copy, assignment, and destructor
     _LIBCPP_INLINE_VISIBILITY
     constexpr span(array<_OtherElementType, _Sz>& __arr) noexcept : __data{__arr.data()}, __size{_Sz} {}
 
     template <class _OtherElementType, size_t _Sz,
               enable_if_t<is_convertible_v<const _OtherElementType(*)[], element_type (*)[]>, nullptr_t> = nullptr>
     _LIBCPP_INLINE_VISIBILITY
     constexpr span(const array<_OtherElementType, _Sz>& __arr) noexcept : __data{__arr.data()}, __size{_Sz} {}
 
-#if !defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
+#if defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
+    template <class _Container>
+        requires __span_compatible_range<_Container, element_type>
+    _LIBCPP_INLINE_VISIBILITY
+    constexpr span(_Container& __c) : __data(std::data(__c)), __size{std::size(__c)} {}
+    template <class _Container>
+        requires __span_compatible_range<const _Container, element_type>
+    _LIBCPP_INLINE_VISIBILITY
+    constexpr span(const _Container& __c) : __data(std::data(__c)), __size{std::size(__c)} {}
+#else
     template <__span_compatible_range<element_type> _Range>
     _LIBCPP_INLINE_VISIBILITY
     constexpr span(_Range&& __r) : __data(ranges::data(__r)), __size{ranges::size(__r)} {}
 #endif
 
     template <class _OtherElementType, size_t _OtherExtent>
     _LIBCPP_INLINE_VISIBILITY
         constexpr span(const span<_OtherElementType, _OtherExtent>& __other,

libcxx/test/libcxx/containers/views/span.cons/range.pass.cpp

+//===---------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===---------------------------------------------------------------------===//
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <span>
+
+//  template<class Container>
+//    constexpr explicit(Extent != dynamic_extent) span(Container&);
+//  template<class Container>
+//    constexpr explicit(Extent != dynamic_extent) span(Container const&);
+
+// This test checks for libc++'s non-conforming temporary extension to std::span
+// to support construction from containers that look like contiguous ranges.
+//
+// This extension is only supported when we don't ship <ranges>, and we can
+// remove it once we get rid of _LIBCPP_HAS_NO_INCOMPLETE_RANGES.
+
+#include <span>
+#include <cassert>
+#include <string>
+#include <vector>
+
+#include "test_macros.h"
+
+//  Look ma - I'm a container!
+template <typename T>
+struct IsAContainer {
+    constexpr IsAContainer() : v_{} {}
+    constexpr size_t size() const {return 1;}
+    constexpr       T *data() {return &v_;}
+    constexpr const T *data() const {return &v_;}
+    constexpr       T *begin() {return &v_;}
+    constexpr const T *begin() const {return &v_;}
+    constexpr       T *end() {return &v_ + 1;}
+    constexpr const T *end() const {return &v_ + 1;}
+
+    constexpr T const *getV() const {return &v_;} // for checking
+    T v_;
+};
+
+
+void checkCV()
+{
+    std::vector<int> v  = {1,2,3};
+
+//  Types the same
+    {
+    std::span<               int> s1{v};    // a span<               int> pointing at int.
+    }
+
+//  types different
+    {
+    std::span<const          int> s1{v};    // a span<const          int> pointing at int.
+    std::span<      volatile int> s2{v};    // a span<      volatile int> pointing at int.
+    std::span<      volatile int> s3{v};    // a span<      volatile int> pointing at const int.
+    std::span<const volatile int> s4{v};    // a span<const volatile int> pointing at int.
+    }
+
+//  Constructing a const view from a temporary
+    {
+    std::span<const int>    s1{IsAContainer<int>()};
+    std::span<const int>    s3{std::vector<int>()};
+    (void) s1;
+    (void) s3;
+    }
+}
+
+
+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};
+    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;
+}

libcxx/test/libcxx/containers/views/span.cons/range.verify.cpp

+//===---------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===---------------------------------------------------------------------===//
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <span>
+
+//  template<class Container>
+//    constexpr explicit(Extent != dynamic_extent) span(Container&);
+//  template<class Container>
+//    constexpr explicit(Extent != dynamic_extent) span(Container const&);
+
+// This test checks for libc++'s non-conforming temporary extension to std::span
+// to support construction from containers that look like contiguous ranges.
+//
+// This extension is only supported when we don't ship <ranges>, and we can
+// remove it once we get rid of _LIBCPP_HAS_NO_INCOMPLETE_RANGES.
+
+#include <span>
+#include <cassert>
+#include <deque>
+#include <forward_list>
+#include <list>
+#include <vector>
+
+#include "test_macros.h"
+
+//  Look ma - I'm a container!
+template <typename T>
+struct IsAContainer {
+    constexpr IsAContainer() : v_{} {}
+    constexpr size_t size() const {return 1;}
+    constexpr       T *data() {return &v_;}
+    constexpr const T *data() const {return &v_;}
+
+    constexpr const T *getV() const {return &v_;} // for checking
+    T v_;
+};
+
+template <typename T>
+struct NotAContainerNoData {
+    size_t size() const {return 0;}
+};
+
+template <typename T>
+struct NotAContainerNoSize {
+    const T *data() const {return nullptr;}
+};
+
+template <typename T>
+struct NotAContainerPrivate {
+private:
+    size_t size() const {return 0;}
+    const T *data() const {return nullptr;}
+};
+
+template<class T, size_t extent, class container>
+std::span<T, extent> createImplicitSpan(container c) {
+    return {c}; // expected-error-re {{no matching constructor for initialization of 'std::span<{{.+}}>'}}
+}
+
+int main(int, char**)
+{
+
+//  Making non-const spans from const sources (a temporary binds to `const &`)
+    {
+    std::span<int>    s1{IsAContainer<int>()};          // expected-error {{no matching constructor for initialization of 'std::span<int>'}}
+    std::span<int>    s3{std::vector<int>()};           // expected-error {{no matching constructor for initialization of 'std::span<int>'}}
+    }
+
+//  Missing size and/or data
+    {
+    std::span<const int>    s1{NotAContainerNoData<int>()};   // expected-error {{no matching constructor for initialization of 'std::span<const int>'}}
+    std::span<const int>    s3{NotAContainerNoSize<int>()};   // expected-error {{no matching constructor for initialization of 'std::span<const int>'}}
+    std::span<const int>    s5{NotAContainerPrivate<int>()};  // expected-error {{no matching constructor for initialization of 'std::span<const int>'}}
+
+//  Again with the standard containers
+    std::span<const int>    s11{std::deque<int>()};           // expected-error {{no matching constructor for initialization of 'std::span<const int>'}}
+    std::span<const int>    s13{std::list<int>()};            // expected-error {{no matching constructor for initialization of 'std::span<const int>'}}
+    std::span<const int>    s15{std::forward_list<int>()};    // expected-error {{no matching constructor for initialization of 'std::span<const int>'}}
+    }
+
+//  Not the same type
+    {
+    IsAContainer<int> c;
+    std::span<float>    s1{c};   // expected-error {{no matching constructor for initialization of 'std::span<float>'}}
+    }
+
+//  CV wrong
+    {
+    IsAContainer<const          int> c;
+    IsAContainer<const volatile int> cv;
+    IsAContainer<      volatile int> v;
+
+    std::span<               int> s1{c};    // expected-error {{no matching constructor for initialization of 'std::span<int>'}}
+    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>'}}
+    }
+
+// explicit constructor necessary
+    {
+    IsAContainer<int> c;
+    const IsAContainer<int> cc;
+
+    createImplicitSpan<int, 1>(c);
+    createImplicitSpan<int, 1>(cc);
+    }
+
+    return 0;
+}