[libc++] Use static_asserts for span::front() and span::back() when possible

[ldionne]

When accessing a statically-sized std::span using front() and back(), we can use
static assertions to ensure bounds correctness instead of relying on a runtime
assertion. We already do that for other methods like subspan(), but it wasn't
done for front() and back().

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

When accessing a statically-sized std::span using front() and back(), we can use
static assertions to ensure bounds correctness instead of relying on a runtime
assertion. We already do that for other methods like subspan(), but it wasn't
done for front() and back().

---

Full diff: https://github.com/llvm/llvm-project/pull/119381.diff

5 Files Affected:

• (modified) libcxx/include/span (+2-2)
• (modified) libcxx/test/libcxx/containers/views/views.span/span.elem/assert.back.pass.cpp (+10-13)
• (modified) libcxx/test/libcxx/containers/views/views.span/span.elem/assert.front.pass.cpp (+10-13)
• (added) libcxx/test/libcxx/containers/views/views.span/span.elem/back.verify.cpp (+33)
• (added) libcxx/test/libcxx/containers/views/views.span/span.elem/front.verify.cpp (+33)

diff --git a/libcxx/include/span b/libcxx/include/span
index 097efbd50a00fd..08ddc5330b4e54 100644
--- a/libcxx/include/span
+++ b/libcxx/include/span
@@ -367,12 +367,12 @@ public:
 #  endif
 
   _LIBCPP_HIDE_FROM_ABI constexpr reference front() const noexcept {
-    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!empty(), "span<T, N>::front() on empty span");
+    static_assert(_Extent >= 1, "span<T, N>::front() on empty span");
     return __data_[0];
   }
 
   _LIBCPP_HIDE_FROM_ABI constexpr reference back() const noexcept {
-    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!empty(), "span<T, N>::back() on empty span");
+    static_assert(_Extent >= 1, "span<T, N>::back() on empty span");
     return __data_[size() - 1];
   }
 
diff --git a/libcxx/test/libcxx/containers/views/views.span/span.elem/assert.back.pass.cpp b/libcxx/test/libcxx/containers/views/views.span/span.elem/assert.back.pass.cpp
index ea98fe81ee2f8a..8d30f9659afe2a 100644
--- a/libcxx/test/libcxx/containers/views/views.span/span.elem/assert.back.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/views.span/span.elem/assert.back.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>
@@ -23,17 +24,13 @@
 #include "check_assertion.h"
 
 int main(int, char**) {
-    {
-        std::array<int, 3> array{0, 1, 2};
-        std::span<int> const s(array.data(), 0);
-        TEST_LIBCPP_ASSERT_FAILURE(s.back(), "span<T>::back() on empty span");
-    }
-
-    {
-        std::array<int, 3> array{0, 1, 2};
-        std::span<int, 0> const s(array.data(), 0);
-        TEST_LIBCPP_ASSERT_FAILURE(s.back(), "span<T, N>::back() on empty span");
-    }
-
-    return 0;
+  {
+    std::array<int, 3> array{0, 1, 2};
+    std::span<int> const s(array.data(), 0);
+    TEST_LIBCPP_ASSERT_FAILURE(s.back(), "span<T>::back() on empty span");
+  }
+
+  // back() on a span with a static extent is caught statically and tested in front.verify.cpp
+
+  return 0;
 }
diff --git a/libcxx/test/libcxx/containers/views/views.span/span.elem/assert.front.pass.cpp b/libcxx/test/libcxx/containers/views/views.span/span.elem/assert.front.pass.cpp
index 2660ca1f90c141..6e5a4157ba6df2 100644
--- a/libcxx/test/libcxx/containers/views/views.span/span.elem/assert.front.pass.cpp
+++ b/libcxx/test/libcxx/containers/views/views.span/span.elem/assert.front.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>
@@ -23,17 +24,13 @@
 #include "check_assertion.h"
 
 int main(int, char**) {
-    {
-        std::array<int, 3> array{0, 1, 2};
-        std::span<int> const s(array.data(), 0);
-        TEST_LIBCPP_ASSERT_FAILURE(s.front(), "span<T>::front() on empty span");
-    }
-
-    {
-        std::array<int, 3> array{0, 1, 2};
-        std::span<int, 0> const s(array.data(), 0);
-        TEST_LIBCPP_ASSERT_FAILURE(s.front(), "span<T, N>::front() on empty span");
-    }
-
-    return 0;
+  {
+    std::array<int, 3> array{0, 1, 2};
+    std::span<int> const s(array.data(), 0);
+    TEST_LIBCPP_ASSERT_FAILURE(s.front(), "span<T>::front() on empty span");
+  }
+
+  // front() on a span with a static extent is caught statically and tested in front.verify.cpp
+
+  return 0;
 }
diff --git a/libcxx/test/libcxx/containers/views/views.span/span.elem/back.verify.cpp b/libcxx/test/libcxx/containers/views/views.span/span.elem/back.verify.cpp
new file mode 100644
index 00000000000000..3060db0402a5b8
--- /dev/null
+++ b/libcxx/test/libcxx/containers/views/views.span/span.elem/back.verify.cpp
@@ -0,0 +1,33 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <span>
+//
+// constexpr reference back() const noexcept;
+
+// Make sure that accessing a statically-sized span out-of-bounds triggers a
+// compile-time error.
+
+#include <array>
+#include <span>
+
+int main(int, char**) {
+  std::array<int, 3> array{0, 1, 2};
+  {
+    std::span<int, 0> const s(array.data(), 0);
+    s.back(); // expected-error@span:* {{span<T, N>::back() on empty span}}
+  }
+  {
+    std::span<int, 3> const s(array.data(), 3);
+    s.back(); // nothing
+  }
+
+  return 0;
+}
diff --git a/libcxx/test/libcxx/containers/views/views.span/span.elem/front.verify.cpp b/libcxx/test/libcxx/containers/views/views.span/span.elem/front.verify.cpp
new file mode 100644
index 00000000000000..793eb70259f70a
--- /dev/null
+++ b/libcxx/test/libcxx/containers/views/views.span/span.elem/front.verify.cpp
@@ -0,0 +1,33 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <span>
+//
+// constexpr reference front() const noexcept;
+
+// Make sure that accessing a statically-sized span out-of-bounds triggers a
+// compile-time error.
+
+#include <array>
+#include <span>
+
+int main(int, char**) {
+  std::array<int, 3> array{0, 1, 2};
+  {
+    std::span<int, 0> const s(array.data(), 0);
+    s.front(); // expected-error@span:* {{span<T, N>::front() on empty span}}
+  }
+  {
+    std::span<int, 3> const s(array.data(), 3);
+    s.front(); // nothing
+  }
+
+  return 0;
+}

[philnik777]

Is this allowed? You could have code like

if (!span.empty()) { span.front(); }

which doesn't violate the precondition, but would be rejected with this patch.

[ldionne]

Is this allowed? You could have code like

if (!span.empty()) { span.front(); }

which doesn't violate the precondition, but would be rejected with this patch.

Right, that's actually so obvious if you think about it. I was led down that path because std::span::subspan<n>() uses a static_assert, but that's because the input is a constant so it's OK to make it ill-formed in that case. That reasoning doesn't apply here.

It's still a shame that we can't guarantee a diagnostic at compile-time. I tried thinking of ways to make that work with __builtin_constant_p but I can't get to anything that works since the static_assert always triggers when the function is instantiated.

[ldionne]

For added fun, it looks like it's not the first time we explore this: https://reviews.llvm.org/D71995
And also 9ab85a6.

[philnik777]

We could add __attribute__((diagnose_if(_Extent == 0, "warning", "front() cannot be called on a span of size zero"))), but that would probably result in lots of false-positives.