On Thu, Sep 25, 2025 at 4:23 PM Jonathan Wakely <[email protected]> wrote:
> This removes the use of std::ref that meant that __remove_if used an
> indirection through the reference, which might be a pessimization. Users
> can always use std::ref to pass expensive predicates into erase_if, but
> we shouldn't do it unconditionally. We can std::move the predicate so
> that if it's not cheap to copy and the user didn't use std::ref, then we
> try to use a cheaper move instead of a copy.
>
> There's no reason that std::erase shouldn't just be implemented by
> forwarding to std::erase_if. I probably should have done that in
> r12-4083-gacf3a21cbc26b3 when std::erase started to call __remove_if
> directly.
>
> libstdc++-v3/ChangeLog:
>
> * include/std/deque (erase_if): Move predicate instead of
> wrapping with std::ref.
> (erase): Forward to erase_if.
> * include/std/inplace_vector (erase_if, erase): Likewise.
> * include/std/string (erase_if, erase): Likewise.
> * include/std/vector (erase_if, erase): Likewise.
> ---
>
> Tested powerpc64le-linux and x86_64-linux.
>
Also LGTM. Makes sense to put it in separate commit.
>
> libstdc++-v3/include/std/deque | 19 ++-----------------
> libstdc++-v3/include/std/inplace_vector | 18 ++----------------
> libstdc++-v3/include/std/string | 14 +++-----------
> libstdc++-v3/include/std/vector | 19 ++-----------------
> 4 files changed, 9 insertions(+), 61 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/deque
> b/libstdc++-v3/include/std/deque
> index 3b1a7dea2b10..c82f9dff2869 100644
> --- a/libstdc++-v3/include/std/deque
> +++ b/libstdc++-v3/include/std/deque
> @@ -66,7 +66,6 @@
> #include <bits/stl_construct.h>
> #include <bits/stl_uninitialized.h>
> #include <bits/stl_deque.h>
> -#include <bits/refwrap.h>
> #include <bits/range_access.h>
> #include <bits/deque.tcc>
>
> @@ -109,7 +108,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> const auto __osz = __cont.size();
> const auto __end = __ucont.end();
> auto __removed = std::__remove_if(__ucont.begin(), __end,
> - std::ref(__pred));
> + std::move(__pred));
> if (__removed != __end)
> {
> __cont.erase(__niter_wrap(__cont.begin(), __removed),
> @@ -124,22 +123,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> typename _Up _GLIBCXX26_DEF_VAL_T(_Tp)>
> inline typename deque<_Tp, _Alloc>::size_type
> erase(deque<_Tp, _Alloc>& __cont, const _Up& __value)
> - {
> - using namespace __gnu_cxx;
> - _GLIBCXX_STD_C::deque<_Tp, _Alloc>& __ucont = __cont;
> - const auto __osz = __cont.size();
> - const auto __end = __ucont.end();
> - auto __removed = std::__remove_if(__ucont.begin(), __end,
> - __ops::__equal_to(__value));
> - if (__removed != __end)
> - {
> - __cont.erase(__niter_wrap(__cont.begin(), __removed),
> - __cont.end());
> - return __osz - __cont.size();
> - }
> + { return std::erase_if(__cont,
> __gnu_cxx::__ops::__equal_to(__value)); }
>
> - return 0;
> - }
> _GLIBCXX_END_NAMESPACE_VERSION
> } // namespace std
> #endif // __cpp_lib_erase_if
> diff --git a/libstdc++-v3/include/std/inplace_vector
> b/libstdc++-v3/include/std/inplace_vector
> index b32aa4f3708c..91ceace08f5b 100644
> --- a/libstdc++-v3/include/std/inplace_vector
> +++ b/libstdc++-v3/include/std/inplace_vector
> @@ -42,7 +42,6 @@
> #include <bits/ranges_base.h> // borrowed_iterator_t,
> __detail::__container_compatible_range
> #include <bits/ranges_util.h> // subrange
> #include <bits/ranges_uninitialized.h>
> -#include <bits/refwrap.h>
> #include <bits/stl_construct.h>
> #include <bits/stl_uninitialized.h>
> #include <bits/stl_algo.h> // rotate
> @@ -1343,7 +1342,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> const auto __osz = __cont.size();
> const auto __end = __cont.end();
> auto __removed = std::__remove_if(__cont.begin(), __end,
> - std::ref(__pred));
> + std::move(__pred));
> if (__removed != __end)
> {
> __cont.erase(__niter_wrap(__cont.begin(), __removed),
> @@ -1357,20 +1356,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> template<typename _Tp, size_t _Nm, typename _Up = _Tp>
> constexpr size_t
> erase(inplace_vector<_Tp, _Nm>& __cont, const _Up& __value)
> - {
> - using namespace __gnu_cxx;
> - const auto __osz = __cont.size();
> - const auto __end = __cont.end();
> - auto __removed = std::__remove_if(__cont.begin(), __end,
> - __ops::__equal_to(__value));
> - if (__removed != __end)
> - {
> - __cont.erase(__niter_wrap(__cont.begin(), __removed),
> - __cont.end());
> - return __osz - __cont.size();
> - }
> - return 0;
> - }
> + { return std::erase_if(__cont,
> __gnu_cxx::__ops::__equal_to(__value)); }
>
> _GLIBCXX_END_NAMESPACE_VERSION
> } // namespace
> diff --git a/libstdc++-v3/include/std/string
> b/libstdc++-v3/include/std/string
> index 74e0c7f2be58..4b84aeaa857b 100644
> --- a/libstdc++-v3/include/std/string
> +++ b/libstdc++-v3/include/std/string
> @@ -51,7 +51,6 @@
> #include <bits/stl_function.h> // For less
> #include <ext/numeric_traits.h>
> #include <bits/stl_algobase.h>
> -#include <bits/refwrap.h>
> #include <bits/range_access.h>
> #include <bits/basic_string.h>
> #include <bits/basic_string.tcc>
> @@ -104,7 +103,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> const auto __osz = __cont.size();
> const auto __end = __cont.end();
> auto __removed = std::__remove_if(__cont.begin(), __end,
> - std::ref(__pred));
> + std::move(__pred));
> __cont.erase(__removed, __end);
> return __osz - __cont.size();
> }
> @@ -114,15 +113,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> _GLIBCXX20_CONSTEXPR
> inline typename basic_string<_CharT, _Traits, _Alloc>::size_type
> erase(basic_string<_CharT, _Traits, _Alloc>& __cont, const _Up&
> __value)
> - {
> - using namespace __gnu_cxx;
> - const auto __osz = __cont.size();
> - const auto __end = __cont.end();
> - auto __removed = std::__remove_if(__cont.begin(), __end,
> - __ops::__equal_to(__value));
> - __cont.erase(__removed, __end);
> - return __osz - __cont.size();
> - }
> + { return std::erase_if(__cont,
> __gnu_cxx::__ops::__equal_to(__value)); }
> +
> _GLIBCXX_END_NAMESPACE_VERSION
> } // namespace std
> #endif // __cpp_lib_erase_if
> diff --git a/libstdc++-v3/include/std/vector
> b/libstdc++-v3/include/std/vector
> index 47105210bc0c..cdc30cbff6de 100644
> --- a/libstdc++-v3/include/std/vector
> +++ b/libstdc++-v3/include/std/vector
> @@ -67,7 +67,6 @@
> #include <bits/stl_uninitialized.h>
> #include <bits/stl_vector.h>
> #include <bits/stl_bvector.h>
> -#include <bits/refwrap.h>
> #include <bits/range_access.h>
>
> #ifndef _GLIBCXX_EXPORT_TEMPLATE
> @@ -122,7 +121,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> const auto __osz = __cont.size();
> const auto __end = __ucont.end();
> auto __removed = std::__remove_if(__ucont.begin(), __end,
> - std::ref(__pred));
> + std::move(__pred));
> if (__removed != __end)
> {
> __cont.erase(__niter_wrap(__cont.begin(), __removed),
> @@ -138,22 +137,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> _GLIBCXX20_CONSTEXPR
> inline typename vector<_Tp, _Alloc>::size_type
> erase(vector<_Tp, _Alloc>& __cont, const _Up& __value)
> - {
> - using namespace __gnu_cxx;
> - _GLIBCXX_STD_C::vector<_Tp, _Alloc>& __ucont = __cont;
> - const auto __osz = __cont.size();
> - const auto __end = __ucont.end();
> - auto __removed = std::__remove_if(__ucont.begin(), __end,
> - __ops::__equal_to(__value));
> - if (__removed != __end)
> - {
> - __cont.erase(__niter_wrap(__cont.begin(), __removed),
> - __cont.end());
> - return __osz - __cont.size();
> - }
> + { return std::erase_if(__cont,
> __gnu_cxx::__ops::__equal_to(__value)); }
>
> - return 0;
> - }
> _GLIBCXX_END_NAMESPACE_VERSION
> } // namespace std
> #endif // __cpp_lib_erase_if
> --
> 2.51.0
>
>
