[llvm-branch-commits] [libcxx] add3ab7 - [libc++] Add workaround to avoid breaking users of <span> when <ranges> are disabled
Louis Dionne via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Wed Mar 16 06:04:30 PDT 2022
Author: Louis Dionne
Date: 2022-03-15T17:13:27-04:00
New Revision: add3ab7f4c8a7f25c2940889d397cbdc9f267666
URL: https://github.com/llvm/llvm-project/commit/add3ab7f4c8a7f25c2940889d397cbdc9f267666
DIFF: https://github.com/llvm/llvm-project/commit/add3ab7f4c8a7f25c2940889d397cbdc9f267666.diff
LOG: [libc++] Add workaround to avoid breaking users of <span> when <ranges> are disabled
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.
NOTE: This is a pretty modified version of d4c39f1ab94 since that one
didn't apply properly onto the release/14.x branch.
(cherry picked from commit d4c39f1ab94abc1dd4fff1e82dd4fa97265940e1)
Differential Revision: https://reviews.llvm.org/D121739
Added:
libcxx/test/libcxx/containers/views/span.cons/range.pass.cpp
libcxx/test/libcxx/containers/views/span.cons/range.verify.cpp
Modified:
libcxx/include/span
Removed:
################################################################################
diff --git a/libcxx/include/span b/libcxx/include/span
index fd95ecca17f7b..b8dbc7e01fd61 100644
--- a/libcxx/include/span
+++ b/libcxx/include/span
@@ -170,7 +170,25 @@ 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_CONCEPTS) && !defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
+#if defined(_LIBCPP_HAS_NO_CONCEPTS) || 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, class = void>
+struct __span_compatible_range : false_type { };
+
+template <class _Range, class _ElementType>
+struct __span_compatible_range<_Range, _ElementType, void_t<
+ enable_if_t<!__is_std_span<remove_cvref_t<_Range>>::value>,
+ enable_if_t<!__is_std_array<remove_cvref_t<_Range>>::value>,
+ enable_if_t<!is_array_v<remove_cvref_t<_Range>>>,
+ decltype(data(declval<_Range>())),
+ decltype(size(declval<_Range>())),
+ enable_if_t<is_convertible_v<remove_pointer_t<decltype(data(declval<_Range&>()))>(*)[], _ElementType(*)[]>>
+>> : true_type { };
+#else
template <class _Range, class _ElementType>
concept __span_compatible_range =
ranges::contiguous_range<_Range> &&
@@ -248,7 +266,22 @@ public:
_LIBCPP_INLINE_VISIBILITY
constexpr span(const array<_OtherElementType, _Extent>& __arr) noexcept : __data{__arr.data()} {}
-#if !defined(_LIBCPP_HAS_NO_CONCEPTS) && !defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
+#if defined(_LIBCPP_HAS_NO_CONCEPTS) || defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
+ template <class _Container, class = enable_if_t<
+ __span_compatible_range<_Container, element_type>::value
+ >>
+ _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, class = enable_if_t<
+ __span_compatible_range<const _Container, element_type>::value
+ >>
+ _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)} {
@@ -434,7 +467,18 @@ public:
_LIBCPP_INLINE_VISIBILITY
constexpr span(const array<_OtherElementType, _Sz>& __arr) noexcept : __data{__arr.data()}, __size{_Sz} {}
-#if !defined(_LIBCPP_HAS_NO_CONCEPTS) && !defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
+#if defined(_LIBCPP_HAS_NO_CONCEPTS) || defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
+ template <class _Container, class = enable_if_t<
+ __span_compatible_range<_Container, element_type>::value
+ >>
+ _LIBCPP_INLINE_VISIBILITY
+ constexpr span(_Container& __c) : __data(std::data(__c)), __size{std::size(__c)} {}
+ template <class _Container, class = enable_if_t<
+ __span_compatible_range<const _Container, element_type>::value
+ >>
+ _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)} {}
diff --git a/libcxx/test/libcxx/containers/views/span.cons/range.pass.cpp b/libcxx/test/libcxx/containers/views/span.cons/range.pass.cpp
new file mode 100644
index 0000000000000..efa1001bdb5b8
--- /dev/null
+++ b/libcxx/test/libcxx/containers/views/span.cons/range.pass.cpp
@@ -0,0 +1,141 @@
+//===---------------------------------------------------------------------===//
+//
+// 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
diff erent
+ {
+ 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;
+}
diff --git a/libcxx/test/libcxx/containers/views/span.cons/range.verify.cpp b/libcxx/test/libcxx/containers/views/span.cons/range.verify.cpp
new file mode 100644
index 0000000000000..f0edf4f935367
--- /dev/null
+++ b/libcxx/test/libcxx/containers/views/span.cons/range.verify.cpp
@@ -0,0 +1,118 @@
+//===---------------------------------------------------------------------===//
+//
+// 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 {{chosen constructor is explicit in copy-initialization}}
+}
+
+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;
+}
More information about the llvm-branch-commits
mailing list