[libc++] Disallow dynamic -> static span conversions

[ldionne]

Summary

[libc++] Disallow dynamic -> static span conversions

This change disallows dynamic to static span conversions. This complies
with the standard, which lists the following constaint in
views.span#span.cons-20.1 [1]:

Extent == dynamic_extent || Extent == OtherExtent

Thus this should fail if Extent != dynamic_extent && Extent !=
OtherExtent, which is the case when trying to construct a static span
from a dynamic span.

[1] https://eel.is/c++draft/views.span#span.cons-20.1

[jdoerrie]

Formatting

[jdoerrie]

Full patch

[jdoerrie]

Friendly Ping :)

[jorgbrown]

+1

Thanks also for fixing the return statement of testConversionSpan() in span.pass.cpp

[jdoerrie]

Thanks Jorg! Given that the status is still "Needs Review", I assume I also still need an LGTM from Marshall, Louis or Eric, right?

[mclow.lists]

libcxx/test/std/containers/views/span.cons/span.fail.cpp

78 ↗

(On Diff #226714)

Ok. The comment here is wrong; this is testing dynamic -> static.

However, why are you removing these (failing) tests?

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

64–65

These cases (on the other hand) should be moved to a failure test.

80

Please line these up like the other ones were. Makes it easy to see copy-pasta errors:

return s1.data() == nullptr && s1.size() == 0 
    && s2.data() == nullptr && s2.size() == 0
    && s3.data() == nullptr && s3.size() == 0;

113

same comment as above re: alignment.

[jdoerrie]

Addressed Marshall's comments

libcxx/test/std/containers/views/span.cons/span.fail.cpp

78 ↗

(On Diff #226714)

Considering that Extent == dynamic_extent || Extent == OtherExtent should be checked first I felt these particular tests distracted from the actual root cause of the error. I added now a new section that performs explicit Extent checks.

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

80

FWIW my formatting was performed by clang-format. Do you think it's worth wrapping this block in

// clang-format off
...
// clang-format on

so that this doesn't regress in the future?

[jdoerrie]

Friendly Ping

[miscco]

I believe this is superseded by the implementation of P1976R2 in D74577

[jdoerrie]

Oh interesting! Yes, it looks like you are right.

[ldionne]

In D69520#1878360, @miscco wrote:

I believe this is superseded by the implementation of P1976R2 in D74577

@miscco Can you please look at the tests in this patch and salvage any test you might have omitted from your own patch?

@jdoerrie I'm sorry there was some confusion and duplicate work, but thanks for your contribution nonetheless!

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

 
 //  static -> dynamic
     {
         std::span<const          int> s1{  sp0};    // a span<const          int> pointing at int.
         std::span<      volatile int> s2{  sp0};    // a span<      volatile int> pointing at int.
         std::span<const volatile int> s3{  sp0};    // a span<const volatile int> pointing at int.
         std::span<const volatile int> s4{ vsp0};    // a span<const volatile int> pointing at volatile int.
         assert(s1.size() + s2.size() + s3.size() + s4.size() == 0);
     }
 }
 
 
 template <typename T>
 constexpr bool testConstexprSpan()
 {
   std::span<T> s0d{};
   std::span<T, 0> s0s{};
   std::span<T> s1(s0d);    // dynamic -> dynamic
   std::span<T, 0> s2(s0s); // static -> static
   std::span<T> s3(s2);     // static -> dynamic
   ASSERT_NOEXCEPT(std::span<T>{s0d});
   ASSERT_NOEXCEPT(std::span<T, 0>{s0s});
   ASSERT_NOEXCEPT(std::span<T>{s0s});
 
   return s1.data() == nullptr && s1.size() == 0 && s2.data() == nullptr &&
          s2.size() == 0 && s3.data() == nullptr && s3.size() == 0;
 }
 
 
 template <typename T>
 void testRuntimeSpan()
 {
   std::span<T> s0d{};

 }
 
 
 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<Src, 0> s0s{};
     std::span<Dest> s1(s0d);    // dynamic -> dynamic
     std::span<Dest, 0> s2(s0s); // static -> static
     std::span<Dest> s3(s0d);    // static -> dynamic
-    s1.data() == nullptr&& s1.size() == 0 &&
-        s2.data() == nullptr&& s2.size() == 0 &&
-        s3.data() == nullptr&& s3.size() == 0;
+    return s1.data() == nullptr && s1.size() == 0 && s2.data() == nullptr &&
+           s2.size() == 0 && s3.data() == nullptr && s3.size() == 0;
 }
 
 struct A{};
 
 int main(int, char**)
 {
     static_assert(testConstexprSpan<int>(),    "");
     static_assert(testConstexprSpan<long>(),   "");