| Message ID | 20200921184533.3909335-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | libstdc++: Remove overzealous static_asserts from std::span | expand |
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" }
On 21/09/20 15:07 -0400, Patrick Palka via Libstdc++ wrote: >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: OK fortrunk, thanks. We might want to backport this too.
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 deleted file mode 100644 index c451ed10df8..00000000000 --- a/libstdc++-v3/testsuite/23_containers/span/back_neg.cc +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (C) 2019-2020 Free Software Foundation, Inc. -// -// This file is part of the GNU ISO C++ Library. This library is free -// software; you can redistribute it and/or modify it under the -// terms of the GNU General Public License as published by the -// Free Software Foundation; either version 3, or (at your option) -// any later version. - -// This library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License along -// with this library; see the file COPYING3. If not see -// <http://www.gnu.org/licenses/>. - -// { dg-options "-std=gnu++2a" } -// { dg-do compile { target c++2a } } - -#include <span> - -void -test01() -{ - std::span<int, 0> s; - s.back(); // { dg-error "here" } -} -// { dg-error "static assertion failed" "" { target *-*-* } 0 } diff --git a/libstdc++-v3/testsuite/23_containers/span/front_neg.cc b/libstdc++-v3/testsuite/23_containers/span/front_neg.cc deleted file mode 100644 index 38f87aa2cd5..00000000000 --- a/libstdc++-v3/testsuite/23_containers/span/front_neg.cc +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (C) 2019-2020 Free Software Foundation, Inc. -// -// This file is part of the GNU ISO C++ Library. This library is free -// software; you can redistribute it and/or modify it under the -// terms of the GNU General Public License as published by the -// Free Software Foundation; either version 3, or (at your option) -// any later version. - -// This library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License along -// with this library; see the file COPYING3. If not see -// <http://www.gnu.org/licenses/>. - -// { dg-options "-std=gnu++2a" } -// { dg-do compile { target c++2a } } - -#include <span> - -void -test01() -{ - std::span<int, 0> s; - s.front(); // { dg-error "here" } -} -// { dg-error "static assertion failed" "" { target *-*-* } 0 } diff --git a/libstdc++-v3/testsuite/23_containers/span/index_op_neg.cc b/libstdc++-v3/testsuite/23_containers/span/index_op_neg.cc deleted file mode 100644 index 1e8b2d8724e..00000000000 --- a/libstdc++-v3/testsuite/23_containers/span/index_op_neg.cc +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (C) 2019-2020 Free Software Foundation, Inc. -// -// This file is part of the GNU ISO C++ Library. This library is free -// software; you can redistribute it and/or modify it under the -// terms of the GNU General Public License as published by the -// Free Software Foundation; either version 3, or (at your option) -// any later version. - -// This library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License along -// with this library; see the file COPYING3. If not see -// <http://www.gnu.org/licenses/>. - -// { dg-options "-std=gnu++2a" } -// { dg-do compile { target c++2a } } - -#include <span> - -void -test01() -{ - std::span<int, 0> s; - s[99]; // { dg-error "here" } -} -// { dg-error "static assertion failed" "" { target *-*-* } 0 }