[PATCH] Fix std::erase_if and std::erase for debug containers
François Dumont
frs.dumont@gmail.com
Mon Jan 19 05:54:52 GMT 2026
On 1/17/26 13:02, Jonathan Wakely wrote:
> On Thu, 18 Dec 2025 at 22:33 +0100, François Dumont wrote:
>> Hi
>>
>> libstdc++: Fix std::erase_if behavior for std::__debug containers
>>
>> Complete fix of std::erase_if/std::erase for all std::__debug
>> containers and
>> __gnu_debug::basic_string. Make sure that iterators erased by
>> this function
>> will be properly detected as such by the debug container and so
>> considered as
>> invalid.
>>
>> Doing so introduce a new std::__detail::__erase_if function
>> dealing, similarly
>> to std::__detail::__erase_node_if, with non-node containers.
>>
>> libstdc++-v3/ChangeLog:
>>
>> * include/bits/erase_if.h (__detail::__erase_if): New.
>> * include/debug/deque (std::erase_if<>(__debug::deque<>&,
>> _Pred): Use latter.
>> * include/debug/inplace_vector
>> (std::erase_if<>(__debug::inplace_vector<>&, _Pred)):
>> Likewise.
>> * include/debug/vector
>> (std::erase_if<>(__debug::vector<>&, _Pred)): Likewise.
>> * include/std/deque: Include erase_if.h.
>> (std::erase_if<>(std::vector<>&, _Pred)): Adapt to use
>> __detail::__erase_if.
>> * include/std/inplace_vector
>> (std::erase_if<>(std::inplace_vector<>&, _Pred)):
>> Likewise.
>> * include/std/string
>> (std::erase_if<>(std::basic_string<>&, _Pred)): Likewise.
>> * include/std/vector (std::erase_if<>(std::vector<>&,
>> _Pred)): Likewise.
>> * include/debug/forward_list
>> (std::erase_if<>(__debug::forward_list<>&, _Pred)): New.
>> (std::erase<>(__debug::forward_list<>&, const _Up&)): New.
>> * include/debug/list
>> (std::erase_if<>(__debug::list<>&, _Pred)): New.
>> (std::erase<>(__debug::list<>&, const _Up&)): New.
>> * include/debug/map (std::erase_if<>(__debug::map<>&,
>> _Pred)): New.
>> (std::erase_if<>(__debug::multimap<>&, _Pred)): New.
>> * include/debug/set (std::erase_if<>(__debug::set<>&,
>> _Pred)): New.
>> (std::erase_if<>(__debug::multiset<>&, _Pred)): New.
>> * include/debug/string
>> (std::erase_if<>(__gnu_debug::basic_string<>&, _Pred)): New.
>> (std::erase<>(__gnu_debug::basic_string<>&, const _Up&)): New.
>> * include/debug/unordered_map
>> (std::erase_if<>(__debug::unordered_map<>&, _Pred)): New.
>> (std::erase_if<>(__debug::unordered_multimap<>&, _Pred)): New.
>> * include/debug/unordered_set
>> (std::erase_if<>(__debug::unordered_set<>&, _Pred)): New.
>> (std::erase_if<>(__debug::unordered_multiset<>&, _Pred)): New.
>> * include/std/forward_list
>> (std::erase_if<>(std::forward_list<>&, _Pred)):
>> Adapt to work exclusively for normal implementation.
>> (std::erase<>(std::forward_list<>&, const _Up&)): Likewise.
>> * include/std/list (std::erase_if<>(std::list<>&,
>> _Pred)): Likewise.
>> (std::erase<>(std::list<>&, const _Up&)): Likewise.
>> * include/std/map (std::erase_if<>(std::map<>&, _Pred)):
>> Likewise.
>> (std::erase_if<>(std::multimap<>&, _Pred)): Likewise.
>> * include/debug/set (std::erase_if<>(std::set<>&,
>> _Pred)): Likewise.
>> (std::erase_if<>(std::multiset<>&, _Pred)): Likewise.
>> * include/std/unordered_map
>> (std::erase_if<>(std::unordered_map<>&, _Pred)): Likewise.
>> (std::erase_if<>(std::unordered_multimap<>&, _Pred)): Likewise.
>> * include/std/unordered_set
>> (std::erase_if<>(std::unordered_set<>&, _Pred)): Likewise.
>> (std::erase_if<>(std::unordered_multiset<>&, _Pred)): Likewise.
>> * testsuite/21_strings/basic_string/debug/erase.cc: New
>> test case.
>> * testsuite/23_containers/forward_list/debug/erase.cc:
>> New test case.
>> *
>> testsuite/23_containers/forward_list/debug/invalidation/erase.cc: New
>> test case.
>> * testsuite/23_containers/list/debug/erase.cc: New test
>> case.
>> *
>> testsuite/23_containers/list/debug/invalidation/erase.cc: New test case.
>> * testsuite/23_containers/map/debug/erase_if.cc: New test
>> case.
>> *
>> testsuite/23_containers/map/debug/invalidation/erase_if.cc: New test
>> case.
>> * testsuite/23_containers/multimap/debug/erase_if.cc: New
>> test case.
>> *
>> testsuite/23_containers/multimap/debug/invalidation/erase_if.cc: New
>> test case.
>> * testsuite/23_containers/multiset/debug/erase_if.cc: New
>> test case.
>> *
>> testsuite/23_containers/multiset/debug/invalidation/erase_if.cc: New
>> test case.
>> * testsuite/23_containers/set/debug/erase_if.cc: New test
>> case.
>> *
>> testsuite/23_containers/set/debug/invalidation/erase_if.cc: New test
>> case.
>> *
>> testsuite/23_containers/unordered_map/debug/erase_if.cc: New test case.
>> *
>> testsuite/23_containers/unordered_map/debug/invalidation/erase_if.cc:
>> New test case.
>> *
>> testsuite/23_containers/unordered_multimap/debug/erase_if.cc: New
>> test case.
>> *
>> testsuite/23_containers/unordered_multimap/debug/invalidation/erase_if.cc:
>> New test case.
>> *
>> testsuite/23_containers/unordered_multiset/debug/erase_if.cc: New
>> test case.
>> *
>> testsuite/23_containers/unordered_multiset/debug/invalidation/erase_if.cc:
>> New test case.
>> *
>> testsuite/23_containers/unordered_set/debug/erase_if.cc: New test case.
>> *
>> testsuite/23_containers/unordered_set/debug/invalidation/erase_if.cc:
>> New test case.
>>
>> All testsuite/23_containers tests run under Linux x64 normal and
>> _GLIBCXX_DEBUG modes.
>>
>> https://forge.sourceware.org/gcc/gcc-TEST/pulls/131
>>
>> Ok to commit ?
>>
>> François
>>
>
>> diff --git a/libstdc++-v3/include/bits/erase_if.h
>> b/libstdc++-v3/include/bits/erase_if.h
>> index fc086b56728..ecf4e7fd4d3 100644
>> --- a/libstdc++-v3/include/bits/erase_if.h
>> +++ b/libstdc++-v3/include/bits/erase_if.h
>> @@ -35,6 +35,7 @@
>> #endif
>>
>> #include <bits/c++config.h>
>> +#include <bits/stl_algobase.h>
>>
>> // Used by C++17 containers and Library Fundamentals v2 headers.
>> #if __cplusplus >= 201402L
>> @@ -44,6 +45,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>
>> namespace __detail
>> {
>> + template<typename _Container, typename _UnsafeContainer,
>> + typename _Predicate>
>> + _GLIBCXX20_CONSTEXPR
>> + typename _Container::size_type
>> + __erase_if(_Container& __cont, _UnsafeContainer& __ucont,
>> + _Predicate __pred)
>> + {
>> + const auto __osz = __ucont.size();
>> + const auto __end = __ucont.end();
>> + auto __removed = std::__remove_if(__ucont.begin(), __end,
>> + std::move(__pred));
>> + if (__removed != __end)
>> + {
>> + __cont.erase(__niter_wrap(__cont.cbegin(), __removed),
>> + __cont.cend());
>> + return __osz - __ucont.size();
>> + }
>> +
>> + return 0;
>> + }
>> +
>> template<typename _Container, typename _UnsafeContainer,
>> typename _Predicate>
>> typename _Container::size_type
>> diff --git a/libstdc++-v3/include/debug/deque
>> b/libstdc++-v3/include/debug/deque
>> index b2e5dd32717..2358070bd0a 100644
>> --- a/libstdc++-v3/include/debug/deque
>> +++ b/libstdc++-v3/include/debug/deque
>> @@ -779,18 +779,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> erase_if(__debug::deque<_Tp, _Alloc>& __cont, _Predicate __pred)
>
> I keep forgetting why we even need this second overload, but it's for
> the case where _GLIBCXX_DEBUG is not defined and __gnu_debug::deque is
> used explicitly (instead of being used implicitly because std::deque
> resolves to it with _GLIBCXX_DEBUG defined).
>
> Maybe it would help me if this overload was guarded with #ifndef
> _GLIBCXX_DEBUG and a comment saying that the existing
> std::erase_if(deque, p) overload works fine for debug mode. But we
> could add that later if I continue being confused.
At least now there are tests to check that.
I also fear overload ambiguity issues. Maybe when using __gnu_debug
container while _GLIBCXX_DEBUG is defined.
But I might be too caution...
>
>> {
>> _GLIBCXX_STD_C::deque<_Tp, _Alloc>& __unsafe_cont = __cont;
>> - const auto __osz = __cont.size();
>> - const auto __end = __unsafe_cont.end();
>> - auto __removed = std::__remove_if(__unsafe_cont.begin(), __end,
>> - std::move(__pred));
>> - if (__removed != __end)
>> - {
>> - __cont.erase(__niter_wrap(__cont.begin(), __removed),
>> - __cont.end());
>> - return __osz - __cont.size();
>> - }
>> -
>> - return 0;
>> + return __detail::__erase_if(__cont, __unsafe_cont,
>> std::move(__pred));
>
> It seems like this would be simpler as just:
>
> return __detail::__erase_if(__cont, __cont._M_base(),
> std::move(__pred));
Yes, I've done this simplification.
Even if for the std::__debug::inplace_vector it forced me to make
_M_base() public.
>
> I think we could even implement __erase_if so it doesn't need two
> references to the container, by using:
>
> constexpr bool __is_debug_seq
> = __is_base_of(__gnu_debug::_Safe_sequence_base, _Container);
>
> and then using if-constexpr to decide whether to use __cont._M_base()
> and __niter_wrap, or just act on __cont directly.
>
> That might make it a little easier to optimize __erase_if for the
> non-debug containers, because it would only have one reference and
> would not need to use __niter_wrap. But maybe the compiler can already
> optimize the extra code away when the references are the same type
> and __niter_wrap is the no-op always_inline overload.
Note that on a performance point of view I also try to avoid the
_GLIBCXX_DEBUG impact on std::__detail::__erase_if which is that you
need to instantiate _Safe_iterator<> to call the debug container erase
member. std::__detail::__erase_node_if do not suffer from this overhead
because the debug container erase member is called with a normal iterator.
But then I started to have ambiguity issues with the erase member on
debug container taking normal iterators because of the
iterator/const_iterator potential combinations so I quit.
>
> I'll think about this later and maybe check the generated code.
>
> OK for trunk.
Committed, thanks.
François
More information about the Libstdc++
mailing list