| Message ID | 20240118024806.436235-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | libstdc++: Fix constexpr _Safe_iterator in C++20 mode | expand |
On Thu, 18 Jan 2024 at 02:48, Patrick Palka wrote: > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? Please add PR109536 to the commit message. > > -- >8 -- > > 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 them to make the 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 > tests all modified member functions). > > libstdc++-v3/ChangeLog: > > * include/debug/safe_base.h (_Safe_sequence_base::_M_swap): > Remove _GLIBCXX20_CONSTEXPR. > * include/debug/safe_iterator.h (_GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN): > (_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): Also test copy and move assignment. > * testsuite/std/ranges/adaptors/all.cc (test08) [_GLIBCXX_DEBUG]: > Use std::vector unconditionally. > --- > libstdc++-v3/include/debug/safe_base.h | 1 - > libstdc++-v3/include/debug/safe_iterator.h | 48 ++++++++++++++----- > .../vector/element_access/constexpr.cc | 2 + > .../testsuite/std/ranges/adaptors/all.cc | 4 -- > 4 files changed, 38 insertions(+), 17 deletions(-) > > diff --git a/libstdc++-v3/include/debug/safe_base.h b/libstdc++-v3/include/debug/safe_base.h > index 107fef3cb02..d5fbe4b1320 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 1bc7c904ee0..929fd9b0ade 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 for > +// __gnu_cxx::__scoped_lock 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 { do > +# define _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END while(false); }(); Do we need the do-while to create a single statement from the block? Isn't the lambda body enough to create a single statement from it, which can't be broken by a dangling else or anything like that? > +#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/element_access/constexpr.cc b/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc > index ee93d2fd95e..ab1e7f1bb70 100644 > --- a/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc > +++ b/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc > @@ -25,6 +25,8 @@ test_iterators() > it -= 2; > it += 1; > VERIFY( (it + 1) == v.end() ); > + it = it + 1; > + it = it; I think we also need to test these operators here: it[n] n + it it - it And also for the reverse iterator. I think that invokes all the operators. For vector, none of those operators do anything different for positive or negative arguments, so we don't need to test cases like it[-1], it+-1, -1+it etc. > > auto rit = v.rbegin(); > VERIFY( &*rit == &v.back() ); > diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc > index e7010f80e18..5f7206dc8c3 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 Oh that's nice to remove. > > // Verify P2415R2 "What is a view?" changes. > // In particular, rvalue non-view non-borrowed ranges are now viewable. > -- > 2.43.0.367.g186b115d30 >
On Thu, 18 Jan 2024, Jonathan Wakely wrote: > On Thu, 18 Jan 2024 at 02:48, Patrick Palka wrote: > > > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? > > Please add PR109536 to the commit message. Done. > > > > > > > -- >8 -- > > > > 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 them to make the 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 > > tests all modified member functions). > > > > libstdc++-v3/ChangeLog: > > > > * include/debug/safe_base.h (_Safe_sequence_base::_M_swap): > > Remove _GLIBCXX20_CONSTEXPR. > > * include/debug/safe_iterator.h (_GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN): > > (_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): Also test copy and move assignment. > > * testsuite/std/ranges/adaptors/all.cc (test08) [_GLIBCXX_DEBUG]: > > Use std::vector unconditionally. > > --- > > libstdc++-v3/include/debug/safe_base.h | 1 - > > libstdc++-v3/include/debug/safe_iterator.h | 48 ++++++++++++++----- > > .../vector/element_access/constexpr.cc | 2 + > > .../testsuite/std/ranges/adaptors/all.cc | 4 -- > > 4 files changed, 38 insertions(+), 17 deletions(-) > > > > diff --git a/libstdc++-v3/include/debug/safe_base.h b/libstdc++-v3/include/debug/safe_base.h > > index 107fef3cb02..d5fbe4b1320 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 1bc7c904ee0..929fd9b0ade 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 for > > +// __gnu_cxx::__scoped_lock 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 { do > > +# define _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END while(false); }(); > > Do we need the do-while to create a single statement from the block? > Isn't the lambda body enough to create a single statement from it, > which can't be broken by a dangling else or anything like that? I was thinking that the do-while gives compile-time assurance that the macros are used properly and in particular every ..._BEGIN is matched with an ..._END, so that e.g. _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN { do_stuff(); } // omitted ..._END doesn't parse. But it turns out that won't parse even without the do-while, due to a missing semicolon. And the parse error is much more readable when the do-while isn't used. One risk without the do-while is that the seemingly innocent _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN { do_stuff(); }; will parse, and (in C++20 mode) define a lambda that's never invoked, and thus do_stuff() is never invoked. But tests should catch that, so consider the do-while removed. > > > > +#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/element_access/constexpr.cc b/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc > > index ee93d2fd95e..ab1e7f1bb70 100644 > > --- a/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc > > +++ b/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc > > @@ -25,6 +25,8 @@ test_iterators() > > it -= 2; > > it += 1; > > VERIFY( (it + 1) == v.end() ); > > + it = it + 1; > > + it = it; > > I think we also need to test these operators here: > > it[n] > n + it > it - it > > And also for the reverse iterator. > > I think that invokes all the operators. For vector, none of those > operators do anything different for positive or negative arguments, so > we don't need to test cases like it[-1], it+-1, -1+it etc. Done. I also adjusted the vector/bool test to match. How does the following look? -- >8 -- Subject: [PATCH] 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 them to make the 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): (_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]: Use std::vector unconditionally. --- libstdc++-v3/include/debug/safe_base.h | 1 - libstdc++-v3/include/debug/safe_iterator.h | 48 ++++++++++++++----- .../vector/bool/element_access/constexpr.cc | 18 +++++++ .../vector/element_access/constexpr.cc | 18 +++++++ .../testsuite/std/ranges/adaptors/all.cc | 4 -- 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 107fef3cb02..d5fbe4b1320 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 1bc7c904ee0..d3e959b8fa7 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 d6b657e0161..bff9f7b4e0f 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 ee93d2fd95e..19c91d28cd6 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 e7010f80e18..5f7206dc8c3 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.
On Thu, 18 Jan 2024 at 13:51, Patrick Palka <ppalka@redhat.com> wrote: > > On Thu, 18 Jan 2024, Jonathan Wakely wrote: > > > On Thu, 18 Jan 2024 at 02:48, Patrick Palka wrote: > > > > > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? > > > > Please add PR109536 to the commit message. > > Done. > > > > > > > > > > > > > -- >8 -- > > > > > > 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 them to make the 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 > > > tests all modified member functions). > > > > > > libstdc++-v3/ChangeLog: > > > > > > * include/debug/safe_base.h (_Safe_sequence_base::_M_swap): > > > Remove _GLIBCXX20_CONSTEXPR. > > > * include/debug/safe_iterator.h (_GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN): > > > (_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): Also test copy and move assignment. > > > * testsuite/std/ranges/adaptors/all.cc (test08) [_GLIBCXX_DEBUG]: > > > Use std::vector unconditionally. > > > --- > > > libstdc++-v3/include/debug/safe_base.h | 1 - > > > libstdc++-v3/include/debug/safe_iterator.h | 48 ++++++++++++++----- > > > .../vector/element_access/constexpr.cc | 2 + > > > .../testsuite/std/ranges/adaptors/all.cc | 4 -- > > > 4 files changed, 38 insertions(+), 17 deletions(-) > > > > > > diff --git a/libstdc++-v3/include/debug/safe_base.h b/libstdc++-v3/include/debug/safe_base.h > > > index 107fef3cb02..d5fbe4b1320 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 1bc7c904ee0..929fd9b0ade 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 for > > > +// __gnu_cxx::__scoped_lock 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 { do > > > +# define _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END while(false); }(); > > > > Do we need the do-while to create a single statement from the block? > > Isn't the lambda body enough to create a single statement from it, > > which can't be broken by a dangling else or anything like that? > > I was thinking that the do-while gives compile-time assurance that the > macros are used properly and in particular every ..._BEGIN is matched > with an ..._END, so that e.g. > > _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN { > do_stuff(); > } // omitted ..._END > > doesn't parse. But it turns out that won't parse even without the > do-while, due to a missing semicolon. And the parse error is much more > readable when the do-while isn't used. > > One risk without the do-while is that the seemingly innocent > > _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN { > do_stuff(); > }; > > will parse, and (in C++20 mode) define a lambda that's never invoked, > and thus do_stuff() is never invoked. But tests should catch that, > so consider the do-while removed. > > > > > > > > +#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/element_access/constexpr.cc b/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc > > > index ee93d2fd95e..ab1e7f1bb70 100644 > > > --- a/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc > > > +++ b/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc > > > @@ -25,6 +25,8 @@ test_iterators() > > > it -= 2; > > > it += 1; > > > VERIFY( (it + 1) == v.end() ); > > > + it = it + 1; > > > + it = it; > > > > I think we also need to test these operators here: > > > > it[n] > > n + it > > it - it > > > > And also for the reverse iterator. > > > > I think that invokes all the operators. For vector, none of those > > operators do anything different for positive or negative arguments, so > > we don't need to test cases like it[-1], it+-1, -1+it etc. > > Done. I also adjusted the vector/bool test to match. How does the > following look? Looks great, thanks for fixing this. OK for trunk. > > -- >8 -- > > Subject: [PATCH] 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 them to make the 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): > (_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]: > Use std::vector unconditionally. > --- > libstdc++-v3/include/debug/safe_base.h | 1 - > libstdc++-v3/include/debug/safe_iterator.h | 48 ++++++++++++++----- > .../vector/bool/element_access/constexpr.cc | 18 +++++++ > .../vector/element_access/constexpr.cc | 18 +++++++ > .../testsuite/std/ranges/adaptors/all.cc | 4 -- > 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 107fef3cb02..d5fbe4b1320 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 1bc7c904ee0..d3e959b8fa7 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 d6b657e0161..bff9f7b4e0f 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 ee93d2fd95e..19c91d28cd6 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 e7010f80e18..5f7206dc8c3 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. > -- > 2.43.0.367.g186b115d30 >
diff --git a/libstdc++-v3/include/debug/safe_base.h b/libstdc++-v3/include/debug/safe_base.h index 107fef3cb02..d5fbe4b1320 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 1bc7c904ee0..929fd9b0ade 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 for +// __gnu_cxx::__scoped_lock 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 { do +# define _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END while(false); }(); +#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/element_access/constexpr.cc b/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc index ee93d2fd95e..ab1e7f1bb70 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc @@ -25,6 +25,8 @@ test_iterators() it -= 2; it += 1; VERIFY( (it + 1) == v.end() ); + it = it + 1; + it = it; auto rit = v.rbegin(); VERIFY( &*rit == &v.back() ); diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc index e7010f80e18..5f7206dc8c3 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.