[libcxx-commits] [libcxx] [libc++] Fix rejects-valid in std::span copy construction (PR #104500)

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Thu Aug 15 13:36:59 PDT 2024


https://github.com/ldionne created https://github.com/llvm/llvm-project/pull/104500

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

>From 0d10b73ce76a7fde51701c08d898848160084db5 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 15 Aug 2024 16:32:24 -0400
Subject: [PATCH] [libc++] Fix rejects-valid in std::span copy construction

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
---
 libcxx/include/span                           |   2 +-
 .../views/views.span/span.cons/copy.pass.cpp  | 126 ++++++++++++------
 2 files changed, 86 insertions(+), 42 deletions(-)

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;
 }



More information about the libcxx-commits mailing list