Re: [PATCH] Fix std::erase_if and std::erase for debug containers

Sun, 18 Jan 2026 21:55:17 -0800 


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























					Reply via email to