On Sat, 16 Dec 2023 at 00:27, Patrick Palka wrote: > > On Wed, 6 Dec 2023, Jonathan Wakely wrote: > > > Any comments on this approach? > > > > -- >8 -- > > > > This makes constexpr std::vector (mostly) work in Debug Mode. All safe > > iterator instrumentation and checking is disabled during constant > > evaluation, because it requires mutex locks and calls to non-inline > > functions defined in libstdc++.so. It should be OK to disable the safety > > checks, because most UB should be detected during constant evaluation > > anyway. > > > > We could try to enable the full checking in constexpr, but it would mean > > wrapping all the non-inline functions like _M_attach with an inline > > _M_constexpr_attach that does the iterator housekeeping inline without > > mutex locks when calling for constant evaluation, and calls the > > non-inline function at runtime. That could be done in future if we find > > that we've lost safety or useful checking by disabling the safe > > iterators. > > > > There are a few test failures in C++20 mode, which I'm unable to > > explain. The _Safe_iterator::operator++() member gives errors for using > > non-constexpr functions during constant evaluation, even though those > > functions are guarded by std::is_constant_evaluated() checks. The same > > code works fine for C++23 and up. > > AFAICT these C++20 test failures are really due to the variable > definition of non-literal type > > 381 __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); > > which were prohibited in a constexpr function (even if that code was > never executed) until C++23's P2242R3.
Ah, I figured it was a core change but I couldn't recall which one. Thanks. > We can use an immediately invoked lambda to work around this: > > 381 [this] { > 382 __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); > 383 ++base(); > 384 }(); > 385 return *this; We'd need some #if as this code has to work for C++98. But that's doable. > > > > libstdc++-v3/ChangeLog: > > > > PR libstdc++/109536 > > * include/bits/c++config (__glibcxx_constexpr_assert): Remove > > macro. > > * include/bits/stl_algobase.h (__niter_base, __copy_move_a) > > (__copy_move_backward_a, __fill_a, __fill_n_a, __equal_aux) > > (__lexicographical_compare_aux): Add constexpr to overloads for > > debug mode iterators. > > * include/debug/helper_functions.h (__unsafe): Add constexpr. > > * include/debug/macros.h (_GLIBCXX_DEBUG_VERIFY_COND_AT): Remove > > macro, folding it into ... > > (_GLIBCXX_DEBUG_VERIFY_AT_F): ... here. Do not use > > __glibcxx_constexpr_assert. > > * include/debug/safe_base.h (_Safe_iterator_base): Add constexpr > > to some member functions. Omit attaching, detaching and checking > > operations during constant evaluation. > > * include/debug/safe_container.h (_Safe_container): Likewise. > > * include/debug/safe_iterator.h (_Safe_iterator): Likewise. > > * include/debug/safe_iterator.tcc (__niter_base, __copy_move_a) > > (__copy_move_backward_a, __fill_a, __fill_n_a, __equal_aux) > > (__lexicographical_compare_aux): Add constexpr. > > * include/debug/vector (_Safe_vector, vector): Add constexpr. > > Omit safe iterator operations during constant evaluation. > > * testsuite/23_containers/vector/bool/capacity/constexpr.cc: > > Remove dg-xfail-if for debug mode. > > * testsuite/23_containers/vector/bool/cmp_c++20.cc: Likewise. > > * testsuite/23_containers/vector/bool/cons/constexpr.cc: > > Likewise. > > * testsuite/23_containers/vector/bool/element_access/1.cc: > > Likewise. > > * testsuite/23_containers/vector/bool/element_access/constexpr.cc: > > Likewise. > > * testsuite/23_containers/vector/bool/modifiers/assign/constexpr.cc: > > Likewise. > > * testsuite/23_containers/vector/bool/modifiers/constexpr.cc: > > Likewise. > > * testsuite/23_containers/vector/bool/modifiers/swap/constexpr.cc: > > Likewise. > > * testsuite/23_containers/vector/capacity/constexpr.cc: > > Likewise. > > * testsuite/23_containers/vector/cmp_c++20.cc: Likewise. > > * testsuite/23_containers/vector/cons/constexpr.cc: Likewise. > > * testsuite/23_containers/vector/data_access/constexpr.cc: > > Likewise. > > * testsuite/23_containers/vector/element_access/constexpr.cc: > > Likewise. > > * testsuite/23_containers/vector/modifiers/assign/constexpr.cc: > > Likewise. > > * testsuite/23_containers/vector/modifiers/constexpr.cc: > > Likewise. > > * testsuite/23_containers/vector/modifiers/swap/constexpr.cc: > > Likewise. > > --- > > libstdc++-v3/include/bits/c++config | 9 - > > libstdc++-v3/include/bits/stl_algobase.h | 15 ++ > > libstdc++-v3/include/debug/helper_functions.h | 1 + > > libstdc++-v3/include/debug/macros.h | 9 +- > > libstdc++-v3/include/debug/safe_base.h | 35 +++- > > libstdc++-v3/include/debug/safe_container.h | 15 +- > > libstdc++-v3/include/debug/safe_iterator.h | 186 +++++++++++++++--- > > libstdc++-v3/include/debug/safe_iterator.tcc | 15 ++ > > libstdc++-v3/include/debug/vector | 146 ++++++++++++-- > > .../vector/bool/capacity/constexpr.cc | 1 - > > .../23_containers/vector/bool/cmp_c++20.cc | 1 - > > .../vector/bool/cons/constexpr.cc | 1 - > > .../vector/bool/element_access/1.cc | 1 - > > .../vector/bool/element_access/constexpr.cc | 1 - > > .../vector/bool/modifiers/assign/constexpr.cc | 1 - > > .../vector/bool/modifiers/constexpr.cc | 1 - > > .../vector/bool/modifiers/swap/constexpr.cc | 3 +- > > .../vector/capacity/constexpr.cc | 1 - > > .../23_containers/vector/cmp_c++20.cc | 1 - > > .../23_containers/vector/cons/constexpr.cc | 1 - > > .../vector/data_access/constexpr.cc | 1 - > > .../vector/element_access/constexpr.cc | 1 - > > .../vector/modifiers/assign/constexpr.cc | 1 - > > .../vector/modifiers/constexpr.cc | 1 - > > .../vector/modifiers/swap/constexpr.cc | 1 - > > 25 files changed, 369 insertions(+), 80 deletions(-) > > > > diff --git a/libstdc++-v3/include/bits/c++config > > b/libstdc++-v3/include/bits/c++config > > index 284d24d933f..13d416845c3 100644 > > --- a/libstdc++-v3/include/bits/c++config > > +++ b/libstdc++-v3/include/bits/c++config > > @@ -565,15 +565,6 @@ namespace std > > # define _GLIBCXX_EXTERN_TEMPLATE -1 > > #endif > > > > - > > -#if _GLIBCXX_HAVE_IS_CONSTANT_EVALUATED > > -# define __glibcxx_constexpr_assert(cond) \ > > - if (std::__is_constant_evaluated() && !bool(cond)) \ > > - __builtin_unreachable() /* precondition violation detected! */ > > -#else > > -# define __glibcxx_constexpr_assert(unevaluated) > > -#endif > > - > > #undef _GLIBCXX_VERBOSE_ASSERT > > > > // Assert. > > diff --git a/libstdc++-v3/include/bits/stl_algobase.h > > b/libstdc++-v3/include/bits/stl_algobase.h > > index 01ca4496dfd..77d0ee7bcf5 100644 > > --- a/libstdc++-v3/include/bits/stl_algobase.h > > +++ b/libstdc++-v3/include/bits/stl_algobase.h > > @@ -318,6 +318,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > { return __it; } > > > > template<typename _Ite, typename _Seq> > > + _GLIBCXX20_CONSTEXPR > > _Ite > > __niter_base(const ::__gnu_debug::_Safe_iterator<_Ite, _Seq, > > std::random_access_iterator_tag>&); > > @@ -545,6 +546,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER > > > > template<bool _IsMove, > > typename _Ite, typename _Seq, typename _Cat, typename _OI> > > + _GLIBCXX20_CONSTEXPR > > _OI > > __copy_move_a(const ::__gnu_debug::_Safe_iterator<_Ite, _Seq, _Cat>&, > > const ::__gnu_debug::_Safe_iterator<_Ite, _Seq, _Cat>&, > > @@ -552,6 +554,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER > > > > template<bool _IsMove, > > typename _II, typename _Ite, typename _Seq, typename _Cat> > > + _GLIBCXX20_CONSTEXPR > > __gnu_debug::_Safe_iterator<_Ite, _Seq, _Cat> > > __copy_move_a(_II, _II, > > const ::__gnu_debug::_Safe_iterator<_Ite, _Seq, _Cat>&); > > @@ -559,6 +562,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER > > template<bool _IsMove, > > typename _IIte, typename _ISeq, typename _ICat, > > typename _OIte, typename _OSeq, typename _OCat> > > + _GLIBCXX20_CONSTEXPR > > ::__gnu_debug::_Safe_iterator<_OIte, _OSeq, _OCat> > > __copy_move_a(const ::__gnu_debug::_Safe_iterator<_IIte, _ISeq, > > _ICat>&, > > const ::__gnu_debug::_Safe_iterator<_IIte, _ISeq, _ICat>&, > > @@ -812,6 +816,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER > > > > template<bool _IsMove, > > typename _Ite, typename _Seq, typename _Cat, typename _OI> > > + _GLIBCXX20_CONSTEXPR > > _OI > > __copy_move_backward_a( > > const ::__gnu_debug::_Safe_iterator<_Ite, _Seq, _Cat>&, > > @@ -820,6 +825,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER > > > > template<bool _IsMove, > > typename _II, typename _Ite, typename _Seq, typename _Cat> > > + _GLIBCXX20_CONSTEXPR > > __gnu_debug::_Safe_iterator<_Ite, _Seq, _Cat> > > __copy_move_backward_a(_II, _II, > > const ::__gnu_debug::_Safe_iterator<_Ite, _Seq, _Cat>&); > > @@ -827,6 +833,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER > > template<bool _IsMove, > > typename _IIte, typename _ISeq, typename _ICat, > > typename _OIte, typename _OSeq, typename _OCat> > > + _GLIBCXX20_CONSTEXPR > > ::__gnu_debug::_Safe_iterator<_OIte, _OSeq, _OCat> > > __copy_move_backward_a( > > const ::__gnu_debug::_Safe_iterator<_IIte, _ISeq, _ICat>&, > > @@ -977,6 +984,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER > > { std::__fill_a1(__first, __last, __value); } > > > > template<typename _Ite, typename _Seq, typename _Cat, typename _Tp> > > + _GLIBCXX20_CONSTEXPR > > void > > __fill_a(const ::__gnu_debug::_Safe_iterator<_Ite, _Seq, _Cat>&, > > const ::__gnu_debug::_Safe_iterator<_Ite, _Seq, _Cat>&, > > @@ -1082,6 +1090,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER > > > > template<typename _Ite, typename _Seq, typename _Cat, typename _Size, > > typename _Tp> > > + _GLIBCXX20_CONSTEXPR > > ::__gnu_debug::_Safe_iterator<_Ite, _Seq, _Cat> > > __fill_n_a(const ::__gnu_debug::_Safe_iterator<_Ite, _Seq, _Cat>& > > __first, > > _Size __n, const _Tp& __value, > > @@ -1230,18 +1239,21 @@ _GLIBCXX_END_NAMESPACE_CONTAINER > > } > > > > template<typename _II1, typename _Seq1, typename _Cat1, typename _II2> > > + _GLIBCXX20_CONSTEXPR > > bool > > __equal_aux(const ::__gnu_debug::_Safe_iterator<_II1, _Seq1, _Cat1>&, > > const ::__gnu_debug::_Safe_iterator<_II1, _Seq1, _Cat1>&, > > _II2); > > > > template<typename _II1, typename _II2, typename _Seq2, typename _Cat2> > > + _GLIBCXX20_CONSTEXPR > > bool > > __equal_aux(_II1, _II1, > > const ::__gnu_debug::_Safe_iterator<_II2, _Seq2, _Cat2>&); > > > > template<typename _II1, typename _Seq1, typename _Cat1, > > typename _II2, typename _Seq2, typename _Cat2> > > + _GLIBCXX20_CONSTEXPR > > bool > > __equal_aux(const ::__gnu_debug::_Safe_iterator<_II1, _Seq1, _Cat1>&, > > const ::__gnu_debug::_Safe_iterator<_II1, _Seq1, _Cat1>&, > > @@ -1430,6 +1442,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER > > > > template<typename _Iter1, typename _Seq1, typename _Cat1, > > typename _II2> > > + _GLIBCXX20_CONSTEXPR > > bool > > __lexicographical_compare_aux( > > const ::__gnu_debug::_Safe_iterator<_Iter1, _Seq1, _Cat1>&, > > @@ -1438,6 +1451,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER > > > > template<typename _II1, > > typename _Iter2, typename _Seq2, typename _Cat2> > > + _GLIBCXX20_CONSTEXPR > > bool > > __lexicographical_compare_aux( > > _II1, _II1, > > @@ -1446,6 +1460,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER > > > > template<typename _Iter1, typename _Seq1, typename _Cat1, > > typename _Iter2, typename _Seq2, typename _Cat2> > > + _GLIBCXX20_CONSTEXPR > > bool > > __lexicographical_compare_aux( > > const ::__gnu_debug::_Safe_iterator<_Iter1, _Seq1, _Cat1>&, > > diff --git a/libstdc++-v3/include/debug/helper_functions.h > > b/libstdc++-v3/include/debug/helper_functions.h > > index 052b36b484c..4b76cb00f9a 100644 > > --- a/libstdc++-v3/include/debug/helper_functions.h > > +++ b/libstdc++-v3/include/debug/helper_functions.h > > @@ -324,6 +324,7 @@ namespace __gnu_debug > > > > /* Remove debug mode safe iterator layer, if any. */ > > template<typename _Iterator> > > + _GLIBCXX_CONSTEXPR > > inline _Iterator > > __unsafe(_Iterator __it) > > { return __it; } > > diff --git a/libstdc++-v3/include/debug/macros.h > > b/libstdc++-v3/include/debug/macros.h > > index 0fef0a006fc..4a3d0f2ea84 100644 > > --- a/libstdc++-v3/include/debug/macros.h > > +++ b/libstdc++-v3/include/debug/macros.h > > @@ -38,15 +38,12 @@ > > * the user error and where the error is reported. > > * > > */ > > -#define _GLIBCXX_DEBUG_VERIFY_COND_AT(_Cond,_ErrMsg,_File,_Line,_Func) > > \ > > - if (__builtin_expect(!bool(_Cond), false)) \ > > - __gnu_debug::_Error_formatter::_S_at(_File, _Line, _Func) > > \ > > - ._ErrMsg._M_error() > > > > #define _GLIBCXX_DEBUG_VERIFY_AT_F(_Cond,_ErrMsg,_File,_Line,_Func) \ > > do { > > \ > > - __glibcxx_constexpr_assert(_Cond); > > \ > > - _GLIBCXX_DEBUG_VERIFY_COND_AT(_Cond,_ErrMsg,_File,_Line,_Func); \ > > + if (__builtin_expect(!bool(_Cond), false)) > > \ > > + __gnu_debug::_Error_formatter::_S_at(_File, _Line, _Func) > > \ > > + ._ErrMsg._M_error(); \ > > } while (false) > > > > #define _GLIBCXX_DEBUG_VERIFY_AT(_Cond,_ErrMsg,_File,_Line) \ > > diff --git a/libstdc++-v3/include/debug/safe_base.h > > b/libstdc++-v3/include/debug/safe_base.h > > index 1dfa9f68b65..d9c17b52b48 100644 > > --- a/libstdc++-v3/include/debug/safe_base.h > > +++ b/libstdc++-v3/include/debug/safe_base.h > > @@ -75,6 +75,7 @@ namespace __gnu_debug > > > > protected: > > /** Initializes the iterator and makes it singular. */ > > + _GLIBCXX20_CONSTEXPR > > _Safe_iterator_base() > > : _M_sequence(0), _M_version(0), _M_prior(0), _M_next(0) > > { } > > @@ -86,18 +87,31 @@ namespace __gnu_debug > > * singular. Otherwise, the iterator will reference @p __seq and > > * be nonsingular. > > */ > > + _GLIBCXX20_CONSTEXPR > > _Safe_iterator_base(const _Safe_sequence_base* __seq, bool __constant) > > : _M_sequence(0), _M_version(0), _M_prior(0), _M_next(0) > > - { this->_M_attach(const_cast<_Safe_sequence_base*>(__seq), > > __constant); } > > + { > > + if (!std::__is_constant_evaluated()) > > + this->_M_attach(const_cast<_Safe_sequence_base*>(__seq), __constant); > > + } > > > > /** Initializes the iterator to reference the same sequence that > > @p __x does. @p __constant is true if this is a constant > > iterator, and false if it is mutable. */ > > + _GLIBCXX20_CONSTEXPR > > _Safe_iterator_base(const _Safe_iterator_base& __x, bool __constant) > > : _M_sequence(0), _M_version(0), _M_prior(0), _M_next(0) > > - { this->_M_attach(__x._M_sequence, __constant); } > > + { > > + if (!std::__is_constant_evaluated()) > > + this->_M_attach(__x._M_sequence, __constant); > > + } > > > > - ~_Safe_iterator_base() { this->_M_detach(); } > > + _GLIBCXX20_CONSTEXPR > > + ~_Safe_iterator_base() > > + { > > + if (!std::__is_constant_evaluated()) > > + this->_M_detach(); > > + } > > > > /** For use in _Safe_iterator. */ > > __gnu_cxx::__mutex& > > @@ -201,24 +215,34 @@ namespace __gnu_debug > > > > protected: > > // Initialize with a version number of 1 and no iterators > > + _GLIBCXX20_CONSTEXPR > > _Safe_sequence_base() _GLIBCXX_NOEXCEPT > > : _M_iterators(0), _M_const_iterators(0), _M_version(1) > > { } > > > > #if __cplusplus >= 201103L > > + _GLIBCXX20_CONSTEXPR > > _Safe_sequence_base(const _Safe_sequence_base&) noexcept > > : _Safe_sequence_base() { } > > > > // Move constructor swap iterators. > > + _GLIBCXX20_CONSTEXPR > > _Safe_sequence_base(_Safe_sequence_base&& __seq) noexcept > > : _Safe_sequence_base() > > - { _M_swap(__seq); } > > + { > > + if (!std::__is_constant_evaluated()) > > + _M_swap(__seq); > > + } > > #endif > > > > /** Notify all iterators that reference this sequence that the > > sequence is being destroyed. */ > > + _GLIBCXX20_CONSTEXPR > > ~_Safe_sequence_base() > > - { this->_M_detach_all(); } > > + { > > + if (!std::__is_constant_evaluated()) > > + this->_M_detach_all(); > > + } > > > > /** Detach all iterators, leaving them singular. */ > > void > > @@ -244,6 +268,7 @@ 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; > > With -Wsystem-headers on some ranges tests I get: > > /gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/safe_base.h:273:5: > warning: inline function ‘constexpr void > __gnu_debug::_Safe_sequence_base::_M_swap(__gnu_debug::_Safe_sequence_base&)’ > used but never defined > 273 | _M_swap(_Safe_sequence_base& __x) _GLIBCXX_USE_NOEXCEPT; > | ^~~~~~~ Oh I think we can remove the CONSTEXPR macro. I added it too aggressively and forgot to remove it again in some places.