On Mon, 06 Jul 2026 at 18:40 +0200, François Dumont wrote:
libstdc++: [_GLIBCXX_DEBUG] Enhance __gnu_debug::inplace_vector
implementation
Review invalidation of iterators to use a RAII approach. A guard
type has been
implemented that is in charge of running an invalidation policy
on destruction
when not in a constant evaluation context.
Using RAII changes the semantics of these functions when the base
class operation throws an exception. In some cases that's fine.
For resize(n), if it throws then the size doesn't change, and the
_InvalidatePolicy does nothing. Great.
For append_range, if constructing a new element throws, the size
doesn't change, and the guard does nothing. Great.
But for emplace_back, if the new element throws an exception
from its constructor, then the standard says there are no effects. The
size doesn't change, and no iterators are invalidated. But with this
change, the pedantic guard will
Invalidation of iterators on assign operation has been reviewed.
All iterators
are being invalidated when using the _GLIBCXX_DEBUG_PEDANTIC
mode. Otherwise
invalidate only iterators that were removed by the assignment
operation.
libstdc++-v3/ChangeLog:
* include/debug/assertions.h (_GLIBCXX_DEBUG_STD_C): New.
* include/debug/inplace_vector
(__gnu_debug::_Inplace_vector_guard): New.
(inplace_vector::_InvalidatePolicy): New.
(inplace_vector::_InvalidateGuard): New.
(inplace_vector::_InvalidateAfterNthPolicy): New.
(inplace_vector::_InvalidateAfterNthGuard): New.
[_GLIBCXX_DEBUG_PEDANTIC](inplace_vector::_PedanticInvalidatePolicy): New.
(inplace_vector::_AssignGuard): New.
(inplace_vector): Adapt using latter guards.
*
testsuite/23_containers/inplace_vector/debug/initializer_list.cc:
New test case.
*
testsuite/23_containers/inplace_vector/debug/initializer_list_pedantic.cc:
New test case.
Tested under Linux x64_86 _GLIBCXX_DEBUG mode.
Ok to commit ?
François
diff --git a/libstdc++-v3/include/debug/assertions.h
b/libstdc++-v3/include/debug/assertions.h
index c4993dc8596..768a7f05228 100644
--- a/libstdc++-v3/include/debug/assertions.h
+++ b/libstdc++-v3/include/debug/assertions.h
@@ -31,6 +31,12 @@
#include <bits/c++config.h>
+#ifdef _GLIBCXX_DEBUG
+# define _GLIBCXX_DEBUG_STD_C ::std::_GLIBCXX_STD_C
+#else
+# define _GLIBCXX_DEBUG_STD_C ::std
+#endif
I've wanted this many times over the years :-)
There are probably other places that we could use this (but not as
part of this patch).
#ifndef _GLIBCXX_DEBUG
// Verify that [_First, _Last) forms a non-empty iterator range.
# define __glibcxx_requires_non_empty_range(_First,_Last) \
diff --git a/libstdc++-v3/include/debug/inplace_vector
b/libstdc++-v3/include/debug/inplace_vector
index e579e8636f9..6ce4d006c90 100644
--- a/libstdc++-v3/include/debug/inplace_vector
+++ b/libstdc++-v3/include/debug/inplace_vector
@@ -46,6 +46,33 @@ namespace std _GLIBCXX_VISIBILITY(default) { namespace
__debug {
#include <debug/safe_sequence.h>
#include <debug/safe_iterator.h>
+namespace __gnu_debug
+{
+ template<typename _Tp, size_t _Nm, typename _InvalidationPolicy>
+ struct _Inplace_vector_guard
+ {
+ using _SafeInplaceVector = std::__debug::inplace_vector<_Tp, _Nm>;
+ using _StdInplaceVector = _GLIBCXX_DEBUG_STD_C::inplace_vector<_Tp, _Nm>;
+ using difference_type = typename _StdInplaceVector::difference_type;
+
+ constexpr
+ _Inplace_vector_guard(_SafeInplaceVector& __vector,
+ difference_type __offset = 0)
+ : _M_vector(__vector), _M_invalidation_policy(__vector, __offset) { }
+
+ constexpr
+ ~_Inplace_vector_guard()
+ {
+ if (!std::is_constant_evaluated())
+ _M_invalidation_policy(_M_vector);
This is C++26 code, so should be just:
if ! consteval {
_M_invalidation_policy(_M_vector);
}
This is simpler to compile.
+ }
+
+ private:
+ _SafeInplaceVector& _M_vector;
+ _InvalidationPolicy _M_invalidation_policy;
+ };
+}
+
namespace std _GLIBCXX_VISIBILITY(default)
{
namespace __debug
@@ -80,6 +107,70 @@ namespace __debug
using reverse_iterator = std::reverse_iterator<iterator>;
using const_reverse_iterator = std::reverse_iterator<const_iterator>;
+ private:
+ struct _InvalidatePolicy
+ {
+ size_type _M_size;
+ _Base_const_iterator _M_end;
+
+ constexpr
+ _InvalidatePolicy(const inplace_vector& __vector,
+ difference_type)
+ : _M_size(__vector.size()), _M_end(__vector._M_base().end()) { }
+
+ void
+ operator()(inplace_vector& __vector) const noexcept
+ {
+ const size_type __size = __vector.size();
+ if (__size < _M_size)
+ __vector._M_invalidate_after_nth(__size);
+ else if (__size > _M_size)
+ __vector._M_invalidate_if(_Equal(_M_end));
+ }
+ };
+
+ using _InvalidateGuard =
+ __gnu_debug::_Inplace_vector_guard<_Tp, _Nm, _InvalidatePolicy>;
+
+ struct _InvalidateAfterNthPolicy
+ {
+ size_type _M_size;
+ difference_type _M_offset;
+
+ constexpr
+ _InvalidateAfterNthPolicy(const inplace_vector& __vector,
+ difference_type __offset)
+ : _M_size(__vector.size()), _M_offset(__offset) { }
+
+ void
+ operator()(inplace_vector& __vector) const noexcept
+ {
+ if (__vector.size() != _M_size)
+ __vector._M_invalidate_after_nth(_M_offset);
+ }
+ };
+
+ using _InvalidateAfterNthGuard =
+ __gnu_debug::_Inplace_vector_guard<_Tp, _Nm, _InvalidateAfterNthPolicy>;
+
+#ifdef _GLIBCXX_DEBUG_PEDANTIC
Please add comments explaining how and why we're deviating from what
the standard says.
+ struct _PedanticInvalidatePolicy
+ {
+ constexpr
+ _PedanticInvalidatePolicy(const inplace_vector&, difference_type) { }
+
+ void
+ operator()(inplace_vector& __vector) const noexcept
+ { __vector._M_invalidate_all(); }
+ };
+
+ using _AssignGuard =
+ __gnu_debug::_Inplace_vector_guard<_Tp, _Nm, _PedanticInvalidatePolicy>;
+#else
+ using _AssignGuard = _InvalidateGuard;
+#endif
+
+ public:
constexpr
inplace_vector() noexcept = default;
@@ -120,8 +211,8 @@ namespace __debug
constexpr inplace_vector&
operator=(initializer_list<_Tp> __il)
{
+ _AssignGuard __guard(*this);
_Base::operator=(__il);
- this->_M_invalidate_all();
return *this;
}
@@ -129,65 +220,63 @@ namespace __debug
constexpr void
assign(_InputIterator __first, _InputIterator __last)
{
+ if (std::is_constant_evaluated())
Use 'if consteval' here.
+ return _Base::assign(__gnu_debug::__unsafe(__first),
+ __gnu_debug::__unsafe(__last));
+
typename __gnu_debug::_Distance_traits<_InputIterator>::__type __dist;
__glibcxx_check_valid_range2(__first, __last, __dist);
- const auto __size = size();
- const auto __end = _Base::end();
+ _AssignGuard __guard(*this);
if (__dist.second >= __gnu_debug::__dp_sign)
_Base::assign(__gnu_debug::__unsafe(__first),
__gnu_debug::__unsafe(__last));
else
_Base::assign(__first, __last);
-
- if (size() < __size)
- _M_invalidate_after_nth(size());
- else if (size() > __size)
- this->_M_invalidate_if(_Equal(__end));
}
template<__detail::__container_compatible_range<_Tp> _Rg>
constexpr void
assign_range(_Rg&& __rg)
{
+ _AssignGuard __guard(*this);
_Base::assign_range(std::forward<_Rg>(__rg));
- this->_M_invalidate_all();
}
constexpr void
assign(size_type __n, const _Tp& __u)
{
+ _AssignGuard __guard(*this);
_Base::assign(__n, __u);
- this->_M_invalidate_all();
}
constexpr void
assign(initializer_list<_Tp> __il)
{
+ _AssignGuard __guard(*this);
_Base::assign(__il);
- this->_M_invalidate_all();
}
// iterators
[[nodiscard]]
constexpr iterator
begin() noexcept
- { return { _Base::begin(), this }; }
+ { return iterator(_Base::begin(), this); }
[[nodiscard]]
constexpr const_iterator
begin() const noexcept
- { return { _Base::begin(), this }; }
+ { return const_iterator(_Base::begin(), this); }
[[nodiscard]]
constexpr iterator
end() noexcept
- { return { _Base::end(), this }; }
+ { return iterator(_Base::end(), this); }
[[nodiscard]]
constexpr const_iterator
end() const noexcept
- { return { _Base::end(), this }; }
+ { return const_iterator(_Base::end(), this); }
[[nodiscard]]
constexpr reverse_iterator
@@ -210,12 +299,12 @@ namespace __debug
[[nodiscard]]
constexpr const_iterator
cbegin() const noexcept
- { return { _Base::cbegin(), this }; }
+ { return const_iterator(_Base::cbegin(), this); }
[[nodiscard]]
constexpr const_iterator
cend() const noexcept
- { return { _Base::cend(), this }; }
+ { return const_iterator(_Base::cend(), this); }
[[nodiscard]]
constexpr const_reverse_iterator
@@ -233,15 +322,15 @@ namespace __debug
constexpr void
resize(size_type __n)
{
+ _InvalidateGuard __guard(*this);
_Base::resize(__n);
- _M_invalidate_after_nth(__n);
}
constexpr void
resize(size_type __n, const _Tp& __c)
{
+ _InvalidateGuard __guard(*this);
_Base::resize(__n, __c);
- _M_invalidate_after_nth(__n);
}
using _Base::reserve;
@@ -304,10 +393,8 @@ namespace __debug
constexpr _Tp&
emplace_back(_Args&&... __args)
{
- const auto __end = _Base::cend();
- _Tp& __res = _Base::emplace_back(std::forward<_Args>(__args)...);
- this->_M_invalidate_if(_Equal(__end));
- return __res;
+ _InvalidateGuard __guard(*this);
+ return _Base::emplace_back(std::forward<_Args>(__args)...);
}
constexpr _Tp&
@@ -322,18 +409,15 @@ namespace __debug
constexpr void
append_range(_Rg&& __rg)
{
- const auto __size = size();
- const auto __end = _Base::cend();
+ _InvalidateGuard __guard(*this);
_Base::append_range(__rg);
- if (size() != __size)
- this->_M_invalidate_if(_Equal(__end));
}
constexpr void
pop_back()
{
__glibcxx_check_nonempty();
- _M_invalidate_after_nth(_Base::size() - 1);
+ _InvalidateGuard __guard(*this);
_Base::pop_back();
}
@@ -341,51 +425,30 @@ namespace __debug
constexpr optional<_Tp&>
try_emplace_back(_Args&&... __args)
{
- auto __end = _Base::cend();
- optional<_Tp&> __res
- = _Base::try_emplace_back(std::forward<_Args>(__args)...);
-
- if (__res)
- this->_M_invalidate_if(_Equal(__end));
-
- return __res;
+ _InvalidateGuard __guard(*this);
+ return _Base::try_emplace_back(std::forward<_Args>(__args)...);
}
constexpr optional<_Tp&>
try_push_back(const _Tp& __x)
{
- const auto __end = _Base::cend();
- optional<_Tp&> __res = _Base::try_push_back(__x);
-
- if (__res)
- this->_M_invalidate_if(_Equal(__end));
-
- return __res;
+ _InvalidateGuard __guard(*this);
+ return _Base::try_push_back(__x);
}
constexpr optional<_Tp&>
try_push_back(_Tp&& __x)
{
- const auto __end = _Base::cend();
- optional<_Tp&> __res = _Base::try_push_back(std::move(__x));
-
- if (__res)
- this->_M_invalidate_if(_Equal(__end));
-
- return __res;
+ _InvalidateGuard __guard(*this);
+ return _Base::try_push_back(std::move(__x));
}
template<typename... _Args>
constexpr _Tp&
unchecked_emplace_back(_Args&&... __args)
{
I think the following comment would be useful here:
// Precondition is checked in _Base::unchecked_emplace_back
Because I was just thinking that the debug inplace_vector should check
the precondition even though this is the "unchecked" API. But it is
already checked with __glibcxx_assert in the base.
- const auto __end = _Base::cend();
- _Tp& __res =
- _Base::unchecked_emplace_back(std::forward<_Args>(__args)...);
-
- this->_M_invalidate_if(_Equal(__end));
-
- return __res;
+ _InvalidateGuard __guard(*this);
+ return _Base::unchecked_emplace_back(std::forward<_Args>(__args)...);
}
constexpr _Tp&
@@ -407,9 +470,12 @@ namespace __debug
__glibcxx_check_insert(__position);
const difference_type __offset = __position.base() - _Base::cbegin();
- _Base_iterator __res = _Base::emplace(__position.base(),
- std::forward<_Args>(__args)...);
- _M_invalidate_after_nth(__offset);
+ _Base_iterator __res;
+ {
Why is the extra block scope needed here?
Ah, because otherwise we invalidate the return value as soon as we
construct it? I think that deserves a comment. Arguably, the code was
simpler before. Using the RAII guard take extra lines, and has the
subtle dependency on a new scope. But it does guarantee we invalidate
even if the _Base::emplace call throws.
What if we just called _M_invalidate_after_nth(__offset) before
calling _Base::emplace? Do we lose anything that way?
It's possible that one of the arguments to emplace could be an
iterator into the vector (or something that contains such an iterator)
and invalidating it would break construction of the new element. But I
think that case is undefined anyway, accessing an element of the
vector while emplacing a new element into it violates the reentrancy
rules.
+ _InvalidateAfterNthGuard __guard(*this, __offset);
+ __res = _Base::emplace(__position.base(),
+ std::forward<_Args>(__args)...);
+ }
return { __res, this };
Should the line above be changed to return iterator(__res, this) as
done elsewhere by this patch?
}
@@ -427,11 +493,14 @@ namespace __debug
if (std::is_constant_evaluated())
Use 'if consteval' here.
return iterator(_Base::insert(__position.base(), __n, __x), this);
- __glibcxx_check_insert(__position);
+ __glibcxx_check_insert(__position);
const difference_type __offset = __position.base() - _Base::cbegin();
- _Base_iterator __res = _Base::insert(__position.base(), __n, __x);
- _M_invalidate_after_nth(__offset);
- return { __res, this };
+ _Base_iterator __res;
+ {
+ _InvalidateAfterNthGuard __guard(*this, __offset);
+ __res = _Base::insert(__position.base(), __n, __x);
+ }
+ return iterator(__res, this);
}
template<__any_input_iterator _InputIterator>
@@ -449,14 +518,16 @@ namespace __debug
const difference_type __offset = __position.base() - _Base::cbegin();
_Base_iterator __res;
- if (__dist.second >= __gnu_debug::__dp_sign)
- __res = _Base::insert(__position.base(),
- __gnu_debug::__unsafe(__first),
- __gnu_debug::__unsafe(__last));
- else
- __res = _Base::insert(__position.base(), __first, __last);
+ {
+ _InvalidateAfterNthGuard __guard(*this, __offset);
+ if (__dist.second >= __gnu_debug::__dp_sign)
+ __res = _Base::insert(__position.base(),
+ __gnu_debug::__unsafe(__first),
+ __gnu_debug::__unsafe(__last));
+ else
+ __res = _Base::insert(__position.base(), __first, __last);
+ }
- _M_invalidate_after_nth(__offset);
return { __res, this };
return iterator(__res, this) ?
}
@@ -464,11 +535,12 @@ namespace __debug
constexpr iterator
insert_range(const_iterator __position, _Rg&& __rg)
{
- const auto __size = size();
const difference_type __offset = __position.base() - _Base::cbegin();
- auto __res = _Base::insert_range(__position.base(), __rg);
- if (size() > __size)
- this->_M_invalidate_after_nth(__offset);
+ _Base_iterator __res;
+ {
+ _InvalidateAfterNthGuard __guard(*this, __offset);
+ __res = _Base::insert_range(__position.base(), __rg);
+ }
return iterator(__res, this);
}
@@ -480,11 +552,13 @@ namespace __debug
return iterator(_Base::insert(__position.base(), __il), this);
__glibcxx_check_insert(__position);
- const auto __size = size();
difference_type __offset = __position.base() - _Base::begin();
- _Base_iterator __res = _Base::insert(__position.base(), __il);
- if (size() > __size)
- this->_M_invalidate_after_nth(__offset);
+ _Base_iterator __res;
+ {
+ _InvalidateAfterNthGuard __guard(*this, __offset);
+ __res = _Base::insert(__position.base(), __il);
+ }
+
return iterator(__res, this);
}
@@ -496,8 +570,11 @@ namespace __debug
__glibcxx_check_erase(__position);
difference_type __offset = __position.base() - _Base::cbegin();
- _Base_iterator __res = _Base::erase(__position.base());
- this->_M_invalidate_after_nth(__offset);
+ _Base_iterator __res;
+ {
+ _InvalidateAfterNthGuard __guard(*this, __offset);
+ __res = _Base::erase(__position.base());
+ }
return iterator(__res, this);
}
@@ -509,13 +586,14 @@ namespace __debug
this);
__glibcxx_check_erase_range(__first, __last);
-
if (__first.base() != __last.base())
{
difference_type __offset = __first.base() - _Base::cbegin();
- _Base_iterator __res = _Base::erase(__first.base(),
- __last.base());
- this->_M_invalidate_after_nth(__offset);
+ _Base_iterator __res;
+ {
+ _InvalidateAfterNthGuard __guard(*this, __offset);
+ __res = _Base::erase(__first.base(), __last.base());
+ }
return { __res, this };
}
else
diff --git
a/libstdc++-v3/testsuite/23_containers/inplace_vector/debug/initializer_list.cc
b/libstdc++-v3/testsuite/23_containers/inplace_vector/debug/initializer_list.cc
new file mode 100644
index 00000000000..35d2916021e
--- /dev/null
+++
b/libstdc++-v3/testsuite/23_containers/inplace_vector/debug/initializer_list.cc
@@ -0,0 +1,26 @@
+// { dg-do run { target c++26 } }
+
+#include <debug/inplace_vector>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+ __gnu_debug::inplace_vector<int, 6> v = { 0, 1, 2, 3, 4, 5 };
+
+ auto it1 = v.begin() + 1;
+ auto it4 = v.begin() + 4;
+
+ VERIFY( !it1._M_singular() );
+ VERIFY( !it4._M_singular() );
+
+ v = { 10, 11, 12, 13 };
+
+ VERIFY( !it1._M_singular() );
+ VERIFY( it4._M_singular() );
+}
+
+int main()
+{
+ test01();
+}
diff --git
a/libstdc++-v3/testsuite/23_containers/inplace_vector/debug/initializer_list_pedantic.cc
b/libstdc++-v3/testsuite/23_containers/inplace_vector/debug/initializer_list_pedantic.cc
new file mode 100644
index 00000000000..060161e36f2
--- /dev/null
+++
b/libstdc++-v3/testsuite/23_containers/inplace_vector/debug/initializer_list_pedantic.cc
@@ -0,0 +1,27 @@
+// { dg-do run { target c++26 } }
+// { dg-options "-D_GLIBCXX_DEBUG_PEDANTIC" }
+
+#include <debug/inplace_vector>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+ __gnu_debug::inplace_vector<int, 6> v = { 0, 1, 2, 3, 4, 5 };
+
+ auto it1 = v.begin() + 1;
+ auto it4 = v.begin() + 4;
+
+ VERIFY( !it1._M_singular() );
+ VERIFY( !it4._M_singular() );
+
+ v = { 10, 11, 12, 13 };
+
+ VERIFY( it1._M_singular() );
+ VERIFY( it4._M_singular() );
+}
+
+int main()
+{
+ test01();
+}
