On Wed, 3 Sep 2025, Tomasz KamiƄski wrote:

> This patch refactors ranges::_Partial to be implemented using _Bind_back_t.
> This allows it to benefit from the changes in r16-3398-g250dd5b5604fbc,
> specifically making the closure trivially copyable. Since _Bind_back_t
> already provides an optimized implementation for a single bound argument,
> specializations for _Partial with a single argument are now removed.
> 
> We still preserve a specialization of _Partial for trivially 
> copy-constructible
> arguments that define only a const overload of operator(). To avoid
> re-checking invocability constraints, this specialization calls the 
> now-public,
> unconstrained _Binder::_S_call static method instead of the constrained
> _Binder::operator().
> 
> The primary specialization of _Partial retains its operator(), which
> uses a simpler __adaptor_invocable constraint that does not consider
> member pointers, as they are not relevant here. This implementation also
> calls _Binder::_S_call to avoid re-performing overload resolution and
> invocability checks for _Binder::operator().

Thanks for preserving/working around these _Partial optimizatinos!  Note that
the primary template (in C++20 mode) also just defines 3 operator()
overloads instead of the full 8, sacrificing const&& invocability which
presumably isn't useful in practice.  (The _Pipe closure object does
similar tricks.)

The simpler __adaptor_invocable constraint was added not just because
it's cheaper than is_invocable, but also because it made overload
resolution failure diagnostics actually explain why the adaptor wasn't
invocable.  But now that we have builtins for is_invocable, and
satisfaction failure replays __is_invocable failures (after Nathaniel's
r16-2652-gbfb8615031a8c2), we might be able to get away with using
is_invocable_v directly without sacrificing compile time performance
and diagnostics.  We could look into that as a follow-up.

LGTM, the second patch in the series LGTM too.

> 
> Finally, the _M_binder member (_Bind_back_t) is now marked
> [[no_unique_address]]. This is beneficial as ranges::_Partial is used with
> ranges::to, which commonly has zero or empty bound arguments (e.g., stateless
> allocators, comparators, or hash functions).
> 
> libstdc++-v3/ChangeLog:
> 
>       * include/bits/binders.h (_Binder::_S_call): Make public.
>       * include/std/ranges (ranges::_Partial<_Adaptor, _Args...>):
>       Replace tuple<_Args...> with _Bind_back_t<_Adaptor, _Args...>.
>       (ranges::_Partial<_Adaptor, _Arg>): Remove.
> ---
> Tested on x86_64-linux locally. OK for trunk?
> 
>  libstdc++-v3/include/bits/binders.h |  10 +--
>  libstdc++-v3/include/std/ranges     | 104 ++++------------------------
>  2 files changed, 17 insertions(+), 97 deletions(-)
> 
> diff --git a/libstdc++-v3/include/bits/binders.h 
> b/libstdc++-v3/include/bits/binders.h
> index 08a330da6fe..f274389e5b4 100644
> --- a/libstdc++-v3/include/bits/binders.h
> +++ b/libstdc++-v3/include/bits/binders.h
> @@ -186,11 +186,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       void operator()(_CallArgs&&...) const && = delete;
>  #endif
>  
> -    private:
> -      using _BoundArgsStorage
> -     // _BoundArgs are required to be move-constructible, so this is valid.
> -     = 
> decltype(__make_bound_args<_BoundArgs...>(std::declval<_BoundArgs>()...));
> -
>        template<typename _Tp, typename... _CallArgs>
>       static constexpr
>       decltype(auto)
> @@ -214,6 +209,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>                                std::forward<_CallArgs>(__call_args)...);
>       }
>  
> +    private:
> +      using _BoundArgsStorage
> +     // _BoundArgs are required to be move-constructible, so this is valid.
> +     = 
> decltype(__make_bound_args<_BoundArgs...>(std::declval<_BoundArgs>()...));
> +
>        [[no_unique_address]] _Fd _M_fd;
>        [[no_unique_address]] _BoundArgsStorage _M_bound_args;
>      };
> diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
> index 2970b2cbe00..98154a02e9e 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -51,6 +51,7 @@
>  #include <utility>
>  #include <variant>
>  #endif
> +#include <bits/binders.h>
>  #include <bits/ranges_util.h>
>  #include <bits/refwrap.h>
>  
> @@ -1047,14 +1048,15 @@ namespace views::__adaptor
>    template<typename _Adaptor, typename... _Args>
>      struct _Partial : _RangeAdaptorClosure<_Partial<_Adaptor, _Args...>>
>      {
> -      tuple<_Args...> _M_args;
> +      using _Binder = _Bind_back_t<_Adaptor, _Args...>;
> +      [[no_unique_address]] _Binder _M_binder;
>  
>        // First parameter is to ensure this constructor is never used
>        // instead of the copy/move constructor.
>        template<typename... _Ts>
>       constexpr
>       _Partial(int, _Ts&&... __args)
> -       : _M_args(std::forward<_Ts>(__args)...)
> +       : _M_binder(0, _Adaptor(), std::forward<_Ts>(__args)...)
>       { }
>  
>        // Invoke _Adaptor with arguments __r, _M_args... according to the
> @@ -1065,75 +1067,21 @@ namespace views::__adaptor
>       constexpr auto
>       operator()(this _Self&& __self, _Range&& __r)
>       {
> -       auto __forwarder = [&__r] (auto&&... __args) {
> -         return _Adaptor{}(std::forward<_Range>(__r),
> -                           std::forward<decltype(__args)>(__args)...);
> -       };
> -       return std::apply(__forwarder, __like_t<_Self, 
> _Partial>(__self)._M_args);
> +       return _Binder::_S_call(__like_t<_Self, _Partial>(__self)._M_binder,
> +                                std::forward<_Range>(__r));
>       }
>  #else
>        template<typename _Range>
>       requires __adaptor_invocable<_Adaptor, _Range, const _Args&...>
>       constexpr auto
>       operator()(_Range&& __r) const &
> -     {
> -       auto __forwarder = [&__r] (const auto&... __args) {
> -         return _Adaptor{}(std::forward<_Range>(__r), __args...);
> -       };
> -       return std::apply(__forwarder, _M_args);
> -     }
> +     { return _Binder::_S_call(_M_binder, std::forward<_Range>(__r)); }
>  
>        template<typename _Range>
>       requires __adaptor_invocable<_Adaptor, _Range, _Args...>
>       constexpr auto
>       operator()(_Range&& __r) &&
> -     {
> -       auto __forwarder = [&__r] (auto&... __args) {
> -         return _Adaptor{}(std::forward<_Range>(__r), std::move(__args)...);
> -       };
> -       return std::apply(__forwarder, _M_args);
> -     }
> -
> -      template<typename _Range>
> -     constexpr auto
> -     operator()(_Range&& __r) const && = delete;
> -#endif
> -    };
> -
> -  // A lightweight specialization of the above primary template for
> -  // the common case where _Adaptor accepts a single extra argument.
> -  template<typename _Adaptor, typename _Arg>
> -    struct _Partial<_Adaptor, _Arg> : 
> _RangeAdaptorClosure<_Partial<_Adaptor, _Arg>>
> -    {
> -      _Arg _M_arg;
> -
> -      template<typename _Tp>
> -     constexpr
> -     _Partial(int, _Tp&& __arg)
> -       : _M_arg(std::forward<_Tp>(__arg))
> -     { }
> -
> -#if __cpp_explicit_this_parameter
> -      template<typename _Self, typename _Range>
> -     requires __adaptor_invocable<_Adaptor, _Range, __like_t<_Self, _Arg>>
> -     constexpr auto
> -     operator()(this _Self&& __self, _Range&& __r)
> -     {
> -       return _Adaptor{}(std::forward<_Range>(__r),
> -                         __like_t<_Self, _Partial>(__self)._M_arg);
> -     }
> -#else
> -      template<typename _Range>
> -     requires __adaptor_invocable<_Adaptor, _Range, const _Arg&>
> -     constexpr auto
> -     operator()(_Range&& __r) const &
> -     { return _Adaptor{}(std::forward<_Range>(__r), _M_arg); }
> -
> -      template<typename _Range>
> -     requires __adaptor_invocable<_Adaptor, _Range, _Arg>
> -     constexpr auto
> -     operator()(_Range&& __r) &&
> -     { return _Adaptor{}(std::forward<_Range>(__r), std::move(_M_arg)); }
> +     { return _Binder::_S_call(std::move(_M_binder), 
> std::forward<_Range>(__r)); }
>  
>        template<typename _Range>
>       constexpr auto
> @@ -1150,12 +1098,13 @@ namespace views::__adaptor
>        && (is_trivially_copy_constructible_v<_Args> && ...)
>      struct _Partial<_Adaptor, _Args...> : 
> _RangeAdaptorClosure<_Partial<_Adaptor, _Args...>>
>      {
> -      tuple<_Args...> _M_args;
> +       using _Binder = _Bind_back_t<_Adaptor, _Args...>;
> +       [[no_unique_address]] _Binder _M_binder;
>  
>        template<typename... _Ts>
>       constexpr
>       _Partial(int, _Ts&&... __args)
> -       : _M_args(std::forward<_Ts>(__args)...)
> +       : _M_binder(0, _Adaptor(), std::forward<_Ts>(__args)...)
>       { }
>  
>        // Invoke _Adaptor with arguments __r, const _M_args&... regardless
> @@ -1164,36 +1113,7 @@ namespace views::__adaptor
>       requires __adaptor_invocable<_Adaptor, _Range, const _Args&...>
>       constexpr auto
>       operator()(_Range&& __r) const
> -     {
> -       auto __forwarder = [&__r] (const auto&... __args) {
> -         return _Adaptor{}(std::forward<_Range>(__r), __args...);
> -       };
> -       return std::apply(__forwarder, _M_args);
> -     }
> -
> -      static constexpr bool _S_has_simple_call_op = true;
> -    };
> -
> -  // A lightweight specialization of the above template for the common case
> -  // where _Adaptor accepts a single extra argument.
> -  template<typename _Adaptor, typename _Arg>
> -    requires __adaptor_has_simple_extra_args<_Adaptor, _Arg>
> -      && is_trivially_copy_constructible_v<_Arg>
> -    struct _Partial<_Adaptor, _Arg> : 
> _RangeAdaptorClosure<_Partial<_Adaptor, _Arg>>
> -    {
> -      _Arg _M_arg;
> -
> -      template<typename _Tp>
> -     constexpr
> -     _Partial(int, _Tp&& __arg)
> -       : _M_arg(std::forward<_Tp>(__arg))
> -     { }
> -
> -      template<typename _Range>
> -     requires __adaptor_invocable<_Adaptor, _Range, const _Arg&>
> -     constexpr auto
> -     operator()(_Range&& __r) const
> -     { return _Adaptor{}(std::forward<_Range>(__r), _M_arg); }
> +     { return _Binder::_S_call(_M_binder, std::forward<_Range>(__r)); }
>  
>        static constexpr bool _S_has_simple_call_op = true;
>      };
> -- 
> 2.51.0
> 
> 

Reply via email to