[libcxx-commits] [libcxx] [libc++] Fix rejects-valid in std::span copy construction (PR #104500)
via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Aug 15 13:37:30 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: Louis Dionne (ldionne)
<details>
<summary>Changes</summary>
Trying to copy-construct a std::span from another std::span holding an incomplete type would fail as we evaluate the SFINAE for the range-based constructor. The problem was that we checked for __is_std_span after checking for the range being a contiguous_range, which hard-errored because of arithmetic on a pointer to incomplete type.
As a drive-by, refactor the whole test and format it.
Fixes #<!-- -->104496
---
Full diff: https://github.com/llvm/llvm-project/pull/104500.diff
2 Files Affected:
- (modified) libcxx/include/span (+1-1)
- (modified) libcxx/test/std/containers/views/views.span/span.cons/copy.pass.cpp (+85-41)
``````````diff
diff --git a/libcxx/include/span b/libcxx/include/span
index 60d76d830f0f31..da631cdc3f90e6 100644
--- a/libcxx/include/span
+++ b/libcxx/include/span
@@ -206,10 +206,10 @@ struct __is_std_span<span<_Tp, _Sz>> : true_type {};
template <class _Range, class _ElementType>
concept __span_compatible_range =
+ !__is_std_span<remove_cvref_t<_Range>>::value && //
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>> && //
is_convertible_v<remove_reference_t<ranges::range_reference_t<_Range>> (*)[], _ElementType (*)[]>;
diff --git a/libcxx/test/std/containers/views/views.span/span.cons/copy.pass.cpp b/libcxx/test/std/containers/views/views.span/span.cons/copy.pass.cpp
index 28f13e122ddc5e..d3990fd60a459a 100644
--- a/libcxx/test/std/containers/views/views.span/span.cons/copy.pass.cpp
+++ b/libcxx/test/std/containers/views/views.span/span.cons/copy.pass.cpp
@@ -5,6 +5,7 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
+
// UNSUPPORTED: c++03, c++11, c++14, c++17
// <span>
@@ -14,58 +15,101 @@
#include <span>
#include <cassert>
#include <string>
+#include <utility>
#include "test_macros.h"
-template <typename T>
-constexpr bool doCopy(const T &rhs)
-{
- ASSERT_NOEXCEPT(T{rhs});
- T lhs{rhs};
- return lhs.data() == rhs.data()
- && lhs.size() == rhs.size();
-}
+template <class T>
+constexpr void test() {
+ ASSERT_NOEXCEPT(std::span<T>(std::declval<std::span<T> const&>()));
+ ASSERT_NOEXCEPT(std::span<T>{std::declval<std::span<T> const&>()});
-struct A{};
-
-template <typename T>
-void testCV ()
-{
- int arr[] = {1,2,3};
- assert((doCopy(std::span<T> () )));
- assert((doCopy(std::span<T,0>() )));
- assert((doCopy(std::span<T> (&arr[0], 1))));
- assert((doCopy(std::span<T,1>(&arr[0], 1))));
- assert((doCopy(std::span<T> (&arr[0], 2))));
- assert((doCopy(std::span<T,2>(&arr[0], 2))));
+ // dynamic_extent
+ {
+ std::span<T> x;
+ std::span<T> copy(x);
+ assert(copy.data() == x.data());
+ assert(copy.size() == x.size());
+ }
+ {
+ T array[3] = {};
+ std::span<T> x(array, 3);
+ std::span<T> copy(x);
+ assert(copy.data() == array);
+ assert(copy.size() == 3);
+ }
+ {
+ T array[3] = {};
+ std::span<T> x(array, 2);
+ std::span<T> copy(x);
+ assert(copy.data() == array);
+ assert(copy.size() == 2);
+ }
+
+ // static extent
+ {
+ std::span<T, 0> x;
+ std::span<T, 0> copy(x);
+ assert(copy.data() == x.data());
+ assert(copy.size() == x.size());
+ }
+ {
+ T array[3] = {};
+ std::span<T, 3> x(array);
+ std::span<T, 3> copy(x);
+ assert(copy.data() == array);
+ assert(copy.size() == 3);
+ }
+ {
+ T array[2] = {};
+ std::span<T, 2> x(array);
+ std::span<T, 2> copy(x);
+ assert(copy.data() == array);
+ assert(copy.size() == 2);
+ }
}
+struct Foo {};
+
+constexpr bool test_all() {
+ test<int>();
+ test<const int>();
+ test<volatile int>();
+ test<const volatile int>();
-int main(int, char**)
-{
- constexpr int carr[] = {1,2,3};
+ test<long>();
+ test<const long>();
+ test<volatile long>();
+ test<const volatile long>();
- static_assert(doCopy(std::span< int> ()), "");
- static_assert(doCopy(std::span< int,0>()), "");
- static_assert(doCopy(std::span<const int> (&carr[0], 1)), "");
- static_assert(doCopy(std::span<const int,1>(&carr[0], 1)), "");
- static_assert(doCopy(std::span<const int> (&carr[0], 2)), "");
- static_assert(doCopy(std::span<const int,2>(&carr[0], 2)), "");
+ test<double>();
+ test<const double>();
+ test<volatile double>();
+ test<const volatile double>();
- static_assert(doCopy(std::span<long>()), "");
- static_assert(doCopy(std::span<double>()), "");
- static_assert(doCopy(std::span<A>()), "");
+ // Note: Can't test non-fundamental types with volatile because we require `T*` to be indirectly_readable,
+ // which isn't the case when T is volatile.
+ test<Foo>();
+ test<const Foo>();
- std::string s;
- assert(doCopy(std::span<std::string> () ));
- assert(doCopy(std::span<std::string, 0>() ));
- assert(doCopy(std::span<std::string> (&s, 1)));
- assert(doCopy(std::span<std::string, 1>(&s, 1)));
+ test<std::string>();
+ test<const std::string>();
+
+ // Regression test for https://github.com/llvm/llvm-project/issues/104496
+ {
+ struct Incomplete;
+ std::span<Incomplete> x;
+ std::span<Incomplete> copy(x);
+ assert(copy.data() == x.data());
+ assert(copy.size() == x.size());
+ }
+
+ return true;
+}
- testCV< int>();
- testCV<const int>();
- testCV< volatile int>();
- testCV<const volatile int>();
+int main(int, char**) {
+ test_all();
+ static_assert(test_all());
return 0;
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/104500
More information about the libcxx-commits
mailing list