On Mon, 21 Sep 2020, Patrick Palka wrote:

> For a span with empty static extent, we currently model the
> preconditions of front(), back(), and operator[] as if they were
> mandates, by using a static_assert to verify that extent != 0.  This
> causes us to incorrectly reject valid programs that instantiate these
> member functions but never call them.
> 
> libstdc++-v3/ChangeLog:
> 
>       * include/std/span (span::front): Remove static_assert.
>       (span::back): Likewise.
>       (span::operator[]): Likewise.
>       * testsuite/23_containers/span/back_neg.cc: Remove.
>       * testsuite/23_containers/span/front_neg.cc: Remove.
>       * testsuite/23_containers/span/index_op_neg.cc: Remove.

Here's a version that rewrites rather than removes the testcases:

-- >8 --

Subject: [PATCH] libstdc++: Remove overzealous static_asserts from std::span

For a span with empty static extent, we currently model the
preconditions of front(), back(), and operator[] as if they were
mandates, by using a static_assert to verify that extent != 0.  This
causes us to incorrectly reject valid programs that instantiate these
member functions but would never call them at runtime.

libstdc++-v3/ChangeLog:

        * include/std/span (span::front): Remove static_assert.
        (span::back): Likewise.
        (span::operator[]): Likewise.
        * testsuite/23_containers/span/back_neg.cc: Rewrite to verify
        that we check the preconditions of back() only when it's called.
        * testsuite/23_containers/span/front_neg.cc: Likewise for
        front().
        * testsuite/23_containers/span/index_op_neg.cc: Likewise for
        operator[].
---
 libstdc++-v3/include/std/span                      |  3 ---
 .../testsuite/23_containers/span/back_neg.cc       | 14 ++++++++++----
 .../testsuite/23_containers/span/front_neg.cc      | 14 ++++++++++----
 .../testsuite/23_containers/span/index_op_neg.cc   | 14 ++++++++++----
 4 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/libstdc++-v3/include/std/span b/libstdc++-v3/include/std/span
index f658adb04cf..1cdc0589ddb 100644
--- a/libstdc++-v3/include/std/span
+++ b/libstdc++-v3/include/std/span
@@ -264,7 +264,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       constexpr reference
       front() const noexcept
       {
-       static_assert(extent != 0);
        __glibcxx_assert(!empty());
        return *this->_M_ptr;
       }
@@ -272,7 +271,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       constexpr reference
       back() const noexcept
       {
-       static_assert(extent != 0);
        __glibcxx_assert(!empty());
        return *(this->_M_ptr + (size() - 1));
       }
@@ -280,7 +278,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       constexpr reference
       operator[](size_type __idx) const noexcept
       {
-       static_assert(extent != 0);
        __glibcxx_assert(__idx < size());
        return *(this->_M_ptr + __idx);
       }
diff --git a/libstdc++-v3/testsuite/23_containers/span/back_neg.cc 
b/libstdc++-v3/testsuite/23_containers/span/back_neg.cc
index c451ed10df8..f777edfa20c 100644
--- a/libstdc++-v3/testsuite/23_containers/span/back_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/span/back_neg.cc
@@ -20,10 +20,16 @@
 
 #include <span>
 
-void
-test01()
+constexpr bool
+test01(bool b)
 {
   std::span<int, 0> s;
-  s.back(); // { dg-error "here" }
+  if (b || !s.empty())
+    s.back();
+  return true;
 }
-// { dg-error "static assertion failed" "" { target *-*-* } 0 }
+
+static_assert(test01(false));
+static_assert(test01(true)); // { dg-error "non-constant" }
+// { dg-error "assert" "" { target *-*-* } 0 }
+// { dg-prune-output "in 'constexpr' expansion" }
diff --git a/libstdc++-v3/testsuite/23_containers/span/front_neg.cc 
b/libstdc++-v3/testsuite/23_containers/span/front_neg.cc
index 38f87aa2cd5..14e5bc1e100 100644
--- a/libstdc++-v3/testsuite/23_containers/span/front_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/span/front_neg.cc
@@ -20,10 +20,16 @@
 
 #include <span>
 
-void
-test01()
+constexpr bool
+test01(bool b)
 {
   std::span<int, 0> s;
-  s.front(); // { dg-error "here" }
+  if (b || !s.empty())
+    s.front();
+  return true;
 }
-// { dg-error "static assertion failed" "" { target *-*-* } 0 }
+
+static_assert(test01(false));
+static_assert(test01(true)); // { dg-error "non-constant" }
+// { dg-error "assert" "" { target *-*-* } 0 }
+// { dg-prune-output "in 'constexpr' expansion" }
diff --git a/libstdc++-v3/testsuite/23_containers/span/index_op_neg.cc 
b/libstdc++-v3/testsuite/23_containers/span/index_op_neg.cc
index 1e8b2d8724e..6a3bb8834b4 100644
--- a/libstdc++-v3/testsuite/23_containers/span/index_op_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/span/index_op_neg.cc
@@ -20,10 +20,16 @@
 
 #include <span>
 
-void
-test01()
+constexpr bool
+test01(bool b)
 {
   std::span<int, 0> s;
-  s[99]; // { dg-error "here" }
+  if (b || !s.empty())
+    s[99];
+  return true;
 }
-// { dg-error "static assertion failed" "" { target *-*-* } 0 }
+
+static_assert(test01(false));
+static_assert(test01(true)); // { dg-error "non-constant" }
+// { dg-error "assert" "" { target *-*-* } 0 }
+// { dg-prune-output "in 'constexpr' expansion" }
-- 
2.28.0.497.g54e85e7af1

Reply via email to