[libcxx] span: Fix incorrect static asserts

[miscco]

Summary

The static asserts in span<T, N>::front() and span<T, N>::back() are incorrect as they may be triggered from valid code
due to evaluation of a never taken branch.

span<int, 0> foo;
if (!foo.empty()) {
    auto x = foo.front();
}

The problem is that the branch is always evaluated by the compiler creating invalid compile errors for span<T, 0>.

[ldionne]

It's sad to get rid of the opportunistic static_assert, but I agree your example should compile. Can you add tests with your example?

[miscco]

Add tests

[miscco]

@ldionne I aggree with your sentiment and I only discovered this when I implemented span for MSVC STL and added the same static assert to operator[]

I believe that this should be something for if consteval

I added some tests

[miscco]

Remove superfluous newlines

[miscco]

Make STL happy, as they apply [[nodiscard]] everywhere

[ldionne]

Thanks!

libcxx/include/span

//  ~span() noexcept = default;
     _LIBCPP_INLINE_VISIBILITY constexpr reference operator[](size_type __idx) const noexcept
     {
         _LIBCPP_ASSERT(__idx >= 0 && __idx < size(), "span<T,N>[] index out of bounds");
         return __data[__idx];
     }
 
     _LIBCPP_INLINE_VISIBILITY constexpr reference front() const noexcept
     {
-        static_assert(_Extent > 0, "span<T,N>[].front() on empty span");
+        _LIBCPP_ASSERT(!empty(), "span<T, N>::front() on empty span");
         return __data[0];
     }
 
     _LIBCPP_INLINE_VISIBILITY constexpr reference back() const noexcept
     {
-        static_assert(_Extent > 0, "span<T,N>[].back() on empty span");
+        _LIBCPP_ASSERT(!empty(), "span<T, N>::back() on empty span");
         return __data[size()-1];
     }
 
     _LIBCPP_INLINE_VISIBILITY constexpr pointer data()                         const noexcept { return __data; }
 
 // [span.iter], span iterator support
     _LIBCPP_INLINE_VISIBILITY constexpr iterator                 begin() const noexcept { return iterator(data()); }
     _LIBCPP_INLINE_VISIBILITY constexpr iterator                   end() const noexcept { return iterator(data() + size()); }

libcxx/test/std/containers/views/span.elem/back.pass.cpp

 
 template <typename Span>
 void testRuntimeSpan(Span sp)
 {
     LIBCPP_ASSERT(noexcept(sp.back()));
     assert(std::addressof(sp.back()) == sp.data() + sp.size() - 1);
 }
 
+template <typename Span>
+void testEmptySpan(Span sp)
+{
+    if (!sp.empty())
+        sp.back();
+}
+
 
 struct A{};
 constexpr int iArr1[] = { 0,  1,  2,  3,  4,  5,  6,  7,  8,  9};
           int iArr2[] = {10, 11, 12, 13, 14, 15, 16, 17, 18, 19};
 
 int main(int, char**)
 {
     static_assert(testConstexprSpan(std::span<const int>(iArr1, 1)), "");
int main(int, char**)
     testRuntimeSpan(std::span<int, 2>(iArr2, 2));
     testRuntimeSpan(std::span<int, 3>(iArr2, 3));
     testRuntimeSpan(std::span<int, 4>(iArr2, 4));
 
     std::string s;
     testRuntimeSpan(std::span<std::string>   (&s, 1));
     testRuntimeSpan(std::span<std::string, 1>(&s, 1));
 
+    std::span<int, 0> sp;
+    testEmptySpan(sp);
+
     return 0;
 }

libcxx/test/std/containers/views/span.elem/front.pass.cpp

 
 template <typename Span>
 void testRuntimeSpan(Span sp)
 {
     LIBCPP_ASSERT(noexcept(sp.front()));
     assert(std::addressof(sp.front()) == sp.data());
 }
 
+template <typename Span>
+void testEmptySpan(Span sp)
+{
+    if (!sp.empty())
+        sp.front();
+}
 
 struct A{};
 constexpr int iArr1[] = { 0,  1,  2,  3,  4,  5,  6,  7,  8,  9};
           int iArr2[] = {10, 11, 12, 13, 14, 15, 16, 17, 18, 19};
 
 int main(int, char**)
 {
     static_assert(testConstexprSpan(std::span<const int>(iArr1, 1)), "");
int main(int, char**)
     testRuntimeSpan(std::span<int, 2>(iArr2, 2));
     testRuntimeSpan(std::span<int, 3>(iArr2, 3));
     testRuntimeSpan(std::span<int, 4>(iArr2, 4));
 
     std::string s;
     testRuntimeSpan(std::span<std::string>   (&s, 1));
     testRuntimeSpan(std::span<std::string, 1>(&s, 1));
 
+    std::span<int, 0> sp;
+    testEmptySpan(sp);
+
     return 0;
 }