[libcxx-commits] [libcxx] a1857e2 - [libcxx][test] Fix span tests.

Stephan T. Lavavej via libcxx-commits libcxx-commits at lists.llvm.org
Wed Jan 8 00:31:03 PST 2020


Author: Stephan T. Lavavej
Date: 2020-01-08T00:28:15-08:00
New Revision: a1857e2ce35e749e16d092305f53c0f2bf2e9c7b

URL: https://github.com/llvm/llvm-project/commit/a1857e2ce35e749e16d092305f53c0f2bf2e9c7b
DIFF: https://github.com/llvm/llvm-project/commit/a1857e2ce35e749e16d092305f53c0f2bf2e9c7b.diff

LOG: [libcxx][test] Fix span tests.

span.cons/container.pass.cpp
N4842 22.7.3.2 [span.cons]/13 constrains span's range constructor
for ranges::contiguous_range (among other criteria).

24.4.5 [range.refinements]/2 says that contiguous_range requires data(),
and (via contiguous_range, random_access_range, bidirectional_range,
forward_range, input_range, range) it also requires begin() and end()
(see 24.4.2 [range.range]/1).

Therefore, IsAContainer needs to provide begin() and end().

(Detected by MSVC's concept-constrained implementation.)

span.cons/stdarray.pass.cpp
This test uses std::array, so it must include <array>.
<span> isn't guaranteed to drag in <array>.

(Detected by MSVC's implementation which uses a forward declaration to
avoid dragging in <array>, for increased compiler throughput.)

span.objectrep/as_bytes.pass.cpp
span.objectrep/as_writable_bytes.pass.cpp
Testing `sp.extent == std::dynamic_extent` triggers MSVC warning
C4127 "conditional expression is constant". Using `if constexpr` is a
simple way to avoid this without disrupting anyone else (as span
requires C++20 mode).

span.tuple/get.pass.cpp
22.7.3.2 [span.cons]/4.3: "Preconditions: If extent is not equal to
dynamic_extent, then count is equal to extent."

These lines were triggering undefined behavior (detected by assertions
in MSVC's implementation).

I changed the count arguments in the first two chunks, followed by
changing the span extents, in order to preserve the test's coverage
and follow the existing pattern.

span.cons/span.pass.cpp
22.7.3.2 [span.cons]/18.1 constrains span's converting constructor with
"Extent == dynamic_extent || Extent == OtherExtent is true".

This means that converting from dynamic extent to static extent is
not allowed. (Other constructors tested elsewhere, like
span(It first, size_type count), can be used to write such code.)

As this is the test for the converting constructor, I have:

* Removed the "dynamic -> static" case from checkCV(), which is
comprehensive.

* Changed the initialization of std::span<T, 0> s1{}; in
testConstexprSpan() and testRuntimeSpan(), because s1 is used below.

* Removed ASSERT_NOEXCEPT(std::span<T, 0>{s0}); from those functions,
as they are otherwise comprehensive.

* Deleted testConversionSpan() entirely. Note that this could never
compile (it had a bool return type, but forgot to say `return`). And it
couldn't have provided useful coverage, as the /18.2 constraint
"OtherElementType(*)[] is convertible to ElementType(*)[]"
permits only cv-qualifications, which are already tested by checkCV().

Added: 
    

Modified: 
    libcxx/test/std/containers/views/span.cons/container.pass.cpp
    libcxx/test/std/containers/views/span.cons/span.pass.cpp
    libcxx/test/std/containers/views/span.cons/stdarray.pass.cpp
    libcxx/test/std/containers/views/span.objectrep/as_bytes.pass.cpp
    libcxx/test/std/containers/views/span.objectrep/as_writable_bytes.pass.cpp
    libcxx/test/std/containers/views/span.tuple/get.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/test/std/containers/views/span.cons/container.pass.cpp b/libcxx/test/std/containers/views/span.cons/container.pass.cpp
index d42013bd268b..d4758c694ee2 100644
--- a/libcxx/test/std/containers/views/span.cons/container.pass.cpp
+++ b/libcxx/test/std/containers/views/span.cons/container.pass.cpp
@@ -39,6 +39,10 @@ struct IsAContainer {
     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_;

diff  --git a/libcxx/test/std/containers/views/span.cons/span.pass.cpp b/libcxx/test/std/containers/views/span.cons/span.pass.cpp
index 62f8b9da11b8..4ec80efc8080 100644
--- a/libcxx/test/std/containers/views/span.cons/span.pass.cpp
+++ b/libcxx/test/std/containers/views/span.cons/span.pass.cpp
@@ -63,14 +63,7 @@ void checkCV()
         assert(s1.size() + s2.size() + s3.size() + s4.size() == 0);
     }
 
-//  dynamic -> static
-    {
-        std::span<const          int, 0> s1{  sp};  // a span<const          int> pointing at int.
-        std::span<      volatile int, 0> s2{  sp};  // a span<      volatile int> pointing at int.
-        std::span<const volatile int, 0> s3{  sp};  // a span<const volatile int> pointing at int.
-        std::span<const volatile int, 0> s4{ vsp};  // a span<const volatile int> pointing at volatile int.
-        assert(s1.size() + s2.size() + s3.size() + s4.size() == 0);
-    }
+//  dynamic -> static (not allowed)
 }
 
 
@@ -78,12 +71,11 @@ template <typename T>
 constexpr bool testConstexprSpan()
 {
     std::span<T>    s0{};
-    std::span<T, 0> s1(s0); // dynamic -> static
+    std::span<T, 0> s1{};
     std::span<T>    s2(s1); // static -> dynamic
     ASSERT_NOEXCEPT(std::span<T>   {s0});
     ASSERT_NOEXCEPT(std::span<T, 0>{s1});
     ASSERT_NOEXCEPT(std::span<T>   {s1});
-    ASSERT_NOEXCEPT(std::span<T, 0>{s0});
 
     return
         s1.data() == nullptr && s1.size() == 0
@@ -95,30 +87,17 @@ template <typename T>
 void testRuntimeSpan()
 {
     std::span<T>    s0{};
-    std::span<T, 0> s1(s0); // dynamic -> static
+    std::span<T, 0> s1{};
     std::span<T>    s2(s1); // static -> dynamic
     ASSERT_NOEXCEPT(std::span<T>   {s0});
     ASSERT_NOEXCEPT(std::span<T, 0>{s1});
     ASSERT_NOEXCEPT(std::span<T>   {s1});
-    ASSERT_NOEXCEPT(std::span<T, 0>{s0});
 
     assert(s1.data() == nullptr && s1.size() == 0);
     assert(s2.data() == nullptr && s2.size() == 0);
 }
 
 
-template <typename Dest, typename Src>
-bool testConversionSpan()
-{
-    static_assert(std::is_convertible_v<Src(*)[], Dest(*)[]>, "Bad input types to 'testConversionSpan");
-    std::span<Src>    s0d{};
-    std::span<Src>    s0s{};
-    std::span<Dest, 0> s1(s0d); // dynamic -> static
-    std::span<Dest>    s2(s0s); // static -> dynamic
-        s1.data() == nullptr && s1.size() == 0
-    &&  s2.data() == nullptr && s2.size() == 0;
-}
-
 struct A{};
 
 int main(int, char**)
@@ -134,9 +113,6 @@ int main(int, char**)
     testRuntimeSpan<std::string>();
     testRuntimeSpan<A>();
 
-//  TODO: Add some conversion tests here that aren't "X --> const X"
-//  assert((testConversionSpan<unsigned char, char>()));
-
     checkCV();
 
   return 0;

diff  --git a/libcxx/test/std/containers/views/span.cons/stdarray.pass.cpp b/libcxx/test/std/containers/views/span.cons/stdarray.pass.cpp
index 03fdd2534260..88f46a889544 100644
--- a/libcxx/test/std/containers/views/span.cons/stdarray.pass.cpp
+++ b/libcxx/test/std/containers/views/span.cons/stdarray.pass.cpp
@@ -22,6 +22,7 @@
 
 
 #include <span>
+#include <array>
 #include <cassert>
 #include <string>
 

diff  --git a/libcxx/test/std/containers/views/span.objectrep/as_bytes.pass.cpp b/libcxx/test/std/containers/views/span.objectrep/as_bytes.pass.cpp
index ff1038e0b11c..6d757f355568 100644
--- a/libcxx/test/std/containers/views/span.objectrep/as_bytes.pass.cpp
+++ b/libcxx/test/std/containers/views/span.objectrep/as_bytes.pass.cpp
@@ -33,7 +33,7 @@ void testRuntimeSpan(Span sp)
     using SB = decltype(spBytes);
     ASSERT_SAME_TYPE(const std::byte, typename SB::element_type);
 
-    if (sp.extent == std::dynamic_extent)
+    if constexpr (sp.extent == std::dynamic_extent)
         assert(spBytes.extent == std::dynamic_extent);
     else
         assert(spBytes.extent == sizeof(typename Span::element_type) * sp.extent);

diff  --git a/libcxx/test/std/containers/views/span.objectrep/as_writable_bytes.pass.cpp b/libcxx/test/std/containers/views/span.objectrep/as_writable_bytes.pass.cpp
index 01852ff65a13..735bfa0c00f7 100644
--- a/libcxx/test/std/containers/views/span.objectrep/as_writable_bytes.pass.cpp
+++ b/libcxx/test/std/containers/views/span.objectrep/as_writable_bytes.pass.cpp
@@ -33,7 +33,7 @@ void testRuntimeSpan(Span sp)
     using SB = decltype(spBytes);
     ASSERT_SAME_TYPE(std::byte, typename SB::element_type);
 
-    if (sp.extent == std::dynamic_extent)
+    if constexpr (sp.extent == std::dynamic_extent)
         assert(spBytes.extent == std::dynamic_extent);
     else
         assert(spBytes.extent == sizeof(typename Span::element_type) * sp.extent);

diff  --git a/libcxx/test/std/containers/views/span.tuple/get.pass.cpp b/libcxx/test/std/containers/views/span.tuple/get.pass.cpp
index 4c8c85ef93ab..b2bed4e15011 100644
--- a/libcxx/test/std/containers/views/span.tuple/get.pass.cpp
+++ b/libcxx/test/std/containers/views/span.tuple/get.pass.cpp
@@ -45,9 +45,9 @@ int main(int, char**)
 {
 
 //  static size
-    static_assert(testConstexprSpan<0>(std::span<const int, 4>(iArr1, 1), iArr1 + 0), "");
-    static_assert(testConstexprSpan<1>(std::span<const int, 4>(iArr1, 2), iArr1 + 1), "");
-    static_assert(testConstexprSpan<2>(std::span<const int, 4>(iArr1, 3), iArr1 + 2), "");
+    static_assert(testConstexprSpan<0>(std::span<const int, 4>(iArr1, 4), iArr1 + 0), "");
+    static_assert(testConstexprSpan<1>(std::span<const int, 4>(iArr1, 4), iArr1 + 1), "");
+    static_assert(testConstexprSpan<2>(std::span<const int, 4>(iArr1, 4), iArr1 + 2), "");
     static_assert(testConstexprSpan<3>(std::span<const int, 4>(iArr1, 4), iArr1 + 3), "");
 
     static_assert(testConstexprSpan<0>(std::span<const int, 1>(iArr1 + 1, 1), iArr1 + 1), "");
@@ -57,13 +57,13 @@ int main(int, char**)
 
 //  static size
     testRuntimeSpan<0>(std::span<int, 4>(iArr2, 4), iArr2);
-    testRuntimeSpan<1>(std::span<int, 4>(iArr2, 1), iArr2 + 1);
-    testRuntimeSpan<2>(std::span<int, 4>(iArr2, 2), iArr2 + 2);
-    testRuntimeSpan<3>(std::span<int, 4>(iArr2, 3), iArr2 + 3);
+    testRuntimeSpan<1>(std::span<int, 4>(iArr2, 4), iArr2 + 1);
+    testRuntimeSpan<2>(std::span<int, 4>(iArr2, 4), iArr2 + 2);
+    testRuntimeSpan<3>(std::span<int, 4>(iArr2, 4), iArr2 + 3);
 
-    testRuntimeSpan<0>(std::span<int, 4>(iArr2 + 1, 1), iArr2 + 1);
-    testRuntimeSpan<1>(std::span<int, 4>(iArr2 + 2, 2), iArr2 + 3);
-    testRuntimeSpan<2>(std::span<int, 4>(iArr2 + 3, 3), iArr2 + 5);
+    testRuntimeSpan<0>(std::span<int, 1>(iArr2 + 1, 1), iArr2 + 1);
+    testRuntimeSpan<1>(std::span<int, 2>(iArr2 + 2, 2), iArr2 + 3);
+    testRuntimeSpan<2>(std::span<int, 3>(iArr2 + 3, 3), iArr2 + 5);
     testRuntimeSpan<3>(std::span<int, 4>(iArr2 + 4, 4), iArr2 + 7);
 
 


        


More information about the libcxx-commits mailing list