diff options
author | Patrick Palka <ppalka@redhat.com> | 2024-01-18 10:36:07 -0500 |
---|---|---|
committer | Patrick Palka <ppalka@redhat.com> | 2024-01-18 10:36:07 -0500 |
commit | 3d3145e9e1461e92bef02d55d36990261dd0444d (patch) | |
tree | 1108f7fd678929d1cfc83ddb0b97d49bad7c4c28 | |
parent | 48c8d26d771a5dcf721529b1ca91737a2eff6c13 (diff) | |
download | gcc-3d3145e9e1461e92bef02d55d36990261dd0444d.zip gcc-3d3145e9e1461e92bef02d55d36990261dd0444d.tar.gz gcc-3d3145e9e1461e92bef02d55d36990261dd0444d.tar.bz2 |
libstdc++/debug: Fix constexpr _Safe_iterator in C++20 mode [PR109536]
Some _Safe_iterator member functions define a variable of non-literal
type __gnu_cxx::__scoped_lock, which automatically disqualifies them from
being constexpr in C++20 mode even if that code path is never constant
evaluated. This restriction was lifted by P2242R3 for C++23, but we
need to work around it in C++20 mode. To that end this patch defines
a pair of macros that encapsulate the lambda-based workaround mentioned
in that paper and uses it to make these functions valid C++20 constexpr
functions. The augmented std::vector test element_access/constexpr.cc
now successfully compiles in C++20 mode with -D_GLIBCXX_DEBUG (and it
should test all member functions modified by this patch).
PR libstdc++/109536
libstdc++-v3/ChangeLog:
* include/debug/safe_base.h (_Safe_sequence_base::_M_swap):
Remove _GLIBCXX20_CONSTEXPR from non-inline member function.
* include/debug/safe_iterator.h
(_GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN): Define.
(_GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END): Define.
(_Safe_iterator::operator=): Use them around the code path that
defines a variable of type __gnu_cxx::__scoped_lock.
(_Safe_iterator::operator++): Likewise.
(_Safe_iterator::operator--): Likewise.
(_Safe_iterator::operator+=): Likewise.
(_Safe_iterator::operator-=): Likewise.
* testsuite/23_containers/vector/element_access/constexpr.cc
(test_iterators): Test more iterator operations.
* testsuite/23_containers/vector/bool/element_access/constexpr.cc
(test_iterators): Likewise.
* testsuite/std/ranges/adaptors/all.cc (test08) [_GLIBCXX_DEBUG]:
Remove.
Reviewed-by: Jonathan Wakely <jwakely@redhat.com>
5 files changed, 72 insertions, 17 deletions
diff --git a/libstdc++-v3/include/debug/safe_base.h b/libstdc++-v3/include/debug/safe_base.h index 107fef3..d5fbe4b 100644 --- a/libstdc++-v3/include/debug/safe_base.h +++ b/libstdc++-v3/include/debug/safe_base.h @@ -268,7 +268,6 @@ namespace __gnu_debug * operation is complete all iterators that originally referenced * one container now reference the other container. */ - _GLIBCXX20_CONSTEXPR void _M_swap(_Safe_sequence_base& __x) _GLIBCXX_USE_NOEXCEPT; diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h index 1bc7c90..d3e959b 100644 --- a/libstdc++-v3/include/debug/safe_iterator.h +++ b/libstdc++-v3/include/debug/safe_iterator.h @@ -65,6 +65,20 @@ _GLIBCXX_DEBUG_VERIFY_OPERANDS(_Lhs, _Rhs, __msg_distance_bad, \ __msg_distance_different) +// This pair of macros helps with writing valid C++20 constexpr functions that +// contain a non-constexpr code path that defines a non-literal variable, which +// was otherwise disallowed until P2242R3 for C++23. We use them below around +// __gnu_cxx::__scoped_lock variables so that the containing functions are still +// considered valid C++20 constexpr functions. + +#if __cplusplus >= 202002L && __cpp_constexpr < 202110L +# define _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN [&]() -> void +# define _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END (); +#else +# define _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN +# define _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END +#endif + namespace __gnu_debug { /** Helper struct to deal with sequence offering a before_begin @@ -266,11 +280,11 @@ namespace __gnu_debug ._M_iterator(__x, "other")); if (this->_M_sequence && this->_M_sequence == __x._M_sequence) - { + _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN { __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); base() = __x.base(); _M_version = __x._M_sequence->_M_version; - } + } _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END else { _M_detach(); @@ -306,11 +320,11 @@ namespace __gnu_debug return *this; if (this->_M_sequence && this->_M_sequence == __x._M_sequence) - { + _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN { __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); base() = __x.base(); _M_version = __x._M_sequence->_M_version; - } + } _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END else { _M_detach(); @@ -378,8 +392,10 @@ namespace __gnu_debug _GLIBCXX_DEBUG_VERIFY(this->_M_incrementable(), _M_message(__msg_bad_inc) ._M_iterator(*this, "this")); - __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); - ++base(); + _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN { + __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); + ++base(); + } _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END return *this; } @@ -697,8 +713,10 @@ namespace __gnu_debug _GLIBCXX_DEBUG_VERIFY(this->_M_decrementable(), _M_message(__msg_bad_dec) ._M_iterator(*this, "this")); - __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); - --this->base(); + _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN { + __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); + --this->base(); + } _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END return *this; } @@ -912,8 +930,10 @@ namespace __gnu_debug _GLIBCXX_DEBUG_VERIFY(this->_M_can_advance(__n), _M_message(__msg_advance_oob) ._M_iterator(*this)._M_integer(__n)); - __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); - this->base() += __n; + _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN { + __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); + this->base() += __n; + } _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END return *this; } @@ -930,8 +950,10 @@ namespace __gnu_debug _GLIBCXX_DEBUG_VERIFY(this->_M_can_advance(-__n), _M_message(__msg_retreat_oob) ._M_iterator(*this)._M_integer(__n)); - __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); - this->base() -= __n; + _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN { + __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); + this->base() -= __n; + } _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END return *this; } @@ -1156,6 +1178,8 @@ _GLIBCXX_END_NAMESPACE_VERSION } #endif +#undef _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END +#undef _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN #undef _GLIBCXX_DEBUG_VERIFY_DIST_OPERANDS #undef _GLIBCXX_DEBUG_VERIFY_REL_OPERANDS #undef _GLIBCXX_DEBUG_VERIFY_EQ_OPERANDS diff --git a/libstdc++-v3/testsuite/23_containers/vector/bool/element_access/constexpr.cc b/libstdc++-v3/testsuite/23_containers/vector/bool/element_access/constexpr.cc index d6b657e..bff9f7b 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/bool/element_access/constexpr.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/bool/element_access/constexpr.cc @@ -18,22 +18,40 @@ test_iterators() VERIFY( v.crend() == v.rend() ); auto it = v.begin(); + VERIFY( it[0] == 0 ); VERIFY( *it == v.front() ); + VERIFY( it[1] == v[1] ); VERIFY( it++ == v.begin() ); VERIFY( ++it == v.end() ); VERIFY( (it - 2) == v.begin() ); + VERIFY( (it - v.begin()) == 2 ); it -= 2; it += 1; VERIFY( (it + 1) == v.end() ); + VERIFY( (1 + it) == v.end() ); + it = it + 1; + auto it2 = v.begin(); + std::swap(it, it2); + VERIFY( it == v.begin() ); + VERIFY( it2 == v.end() ); auto rit = v.rbegin(); + VERIFY( rit[0] == 0 ); VERIFY( *rit == v.back() ); + VERIFY( rit[1] == v[0] ); VERIFY( rit++ == v.rbegin() ); VERIFY( ++rit == v.rend() ); VERIFY( (rit - 2) == v.rbegin() ); + VERIFY( (rit - v.rbegin()) == 2 ); rit -= 2; rit += 1; VERIFY( (rit + 1) == v.rend() ); + VERIFY( (1 + rit) == v.rend() ); + rit = rit + 1; + auto rit2 = v.rbegin(); + std::swap(rit, rit2); + VERIFY( rit == v.rbegin() ); + VERIFY( rit2 == v.rend() ); return true; } diff --git a/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc b/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc index ee93d2f..19c91d2 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc @@ -18,22 +18,40 @@ test_iterators() VERIFY( v.crend() == v.rend() ); auto it = v.begin(); + VERIFY( it[0] == 0 ); VERIFY( &*it == &v.front() ); + VERIFY( &it[1] == &v[1] ); VERIFY( it++ == v.begin() ); VERIFY( ++it == v.end() ); VERIFY( (it - 2) == v.begin() ); + VERIFY( (it - v.begin()) == 2 ); it -= 2; it += 1; VERIFY( (it + 1) == v.end() ); + VERIFY( (1 + it) == v.end() ); + it = it + 1; + auto it2 = v.begin(); + std::swap(it, it2); + VERIFY( it == v.begin() ); + VERIFY( it2 == v.end() ); auto rit = v.rbegin(); + VERIFY( rit[0] == 0 ); VERIFY( &*rit == &v.back() ); + VERIFY( &rit[1] == &v[0] ); VERIFY( rit++ == v.rbegin() ); VERIFY( ++rit == v.rend() ); VERIFY( (rit - 2) == v.rbegin() ); + VERIFY( (rit - v.rbegin()) == 2 ); rit -= 2; rit += 1; VERIFY( (rit + 1) == v.rend() ); + VERIFY( (1 + rit) == v.rend() ); + rit = rit + 1; + auto rit2 = v.rbegin(); + std::swap(rit, rit2); + VERIFY( rit == v.rbegin() ); + VERIFY( rit2 == v.rend() ); return true; } diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc index e7010f8..5f7206d 100644 --- a/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc @@ -156,11 +156,7 @@ test07() constexpr bool test08() { -#ifdef _GLIBCXX_DEBUG - using std::_GLIBCXX_STD_C::vector; -#else using std::vector; -#endif // Verify P2415R2 "What is a view?" changes. // In particular, rvalue non-view non-borrowed ranges are now viewable. |