On Sat, 16 Dec 2023 at 09:14, Jonathan Wakely wrote:
>
> 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.
The attached patch seems simpler, I'm testing it now.
commit 5d70c6c2965647077749a869e9cdbf7e91dba4c7
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Sat Dec 16 09:31:40 2023
libstdc++: Fix errors for constexpr __gnu_debug::vector in C++23 [PR109536]
In the commit log for r14-6553-g7d00a59229ee17 I noted some tests FAIL
in C++20 mode. Patrick identified that they were due to the variable
definitions of non-literal type __scoped_lock, which were prohibited in
a constexpr function (even if that code was never executed) until
C++23's P2242R3.
We can move the problematic code into new non-constexpr functions that
are not called during constant evaluation.
There's also a warning about a constexpr _M_swap function which is never
defined. That's simply because I added the _GLIBCXX20_CONSTEXPR macro on
a member that doesn't need it.
libstdc++-v3/ChangeLog:
PR libstdc++/109536
* include/debug/safe_base.h (_Safe_sequence_base::_M_swap):
Remove _GLIBCXX20_CONSTEXPR from non-inline member function.
* include/debug/safe_iterator.h (_Safe_iterator::_M_move_assign)
diff --git a/libstdc++-v3/include/debug/safe_base.h
b/libstdc++-v3/include/debug/safe_base.h
index d9c17b52b48..1519ad809a4 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 26f008982f8..bde34e1f99c 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -295,7 +295,13 @@ namespace __gnu_debug
base() = __x.base();
return *this;
}
+ _M_move_assign(std::move(__x));
+ return *this;
+ }
+ void
+ _M_move_assign(_Safe_iterator&& __x) noexcept
+ {
_GLIBCXX_DEBUG_VERIFY(!__x._M_singular()
|| __x._M_value_initialized(),
_M_message(__msg_copy_singular)
@@ -303,7 +309,7 @@ namespace __gnu_debug
._M_iterator(__x, "other"));
if (std::__addressof(__x) == this)
- return *this;
+ return;
if (this->_M_sequence && this->_M_sequence == __x._M_sequence)
{
@@ -320,7 +326,6 @@ namespace __gnu_debug
__x._M_detach();
__x.base() = _Iterator();
- return *this;
}
#endif
@@ -370,17 +375,20 @@ namespace __gnu_debug
operator++() _GLIBCXX_NOEXCEPT
{
if (std::__is_constant_evaluated())
- {
- ++base();
- return *this;
- }
+ ++base();
+ else
+ _M_increment();
+ return *this;
+ }
+ void
+ _M_increment() _GLIBCXX_NOEXCEPT
+ {
_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();
- return *this;
}
/**
@@ -689,17 +697,20 @@ namespace __gnu_debug
operator--() _GLIBCXX_NOEXCEPT
{
if (std::__is_constant_evaluated())
- {
- --this->base();
- return *this;
- }
+ --this->base();
+ else
+ _M_decrement();
+ return *this;
+ }
+ void
+ _M_decrement() _GLIBCXX_NOEXCEPT
+ {
_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();
- return *this;
}
/**
