On Tue, 15 Jun 2021 at 20:29, Patrick Palka via Libstdc++
<libstd...@gcc.gnu.org> wrote:
>
> On Tue, 15 Jun 2021, Patrick Palka wrote:
>
> > This force-enables perfect forwarding call wrapper semantics whenever
> > the extra arguments of a partially applied range adaptor aren't all
> > trivially copyable, so as to avoid incurring unnecessary copies of
> > potentially expensive-to-copy objects (such as std::function objects)
> > when invoking the adaptor.
> >
> > Tested on x86_64-pc-linux-gnu, does this look OK for trunk/11?
> >
> > PR libstdc++/100940
> >
> > libstdc++-v3/ChangeLog:
> >
> > * include/std/ranges (__adaptor::__adaptor_has_simple_extra_args):
> > Also
> > require that the extra arguments are trivially copyable.
> > * testsuite/std/ranges/adaptors/100577.cc (test04): New test.
> > ---
> > libstdc++-v3/include/std/ranges | 6 ++++--
> > .../testsuite/std/ranges/adaptors/100577.cc | 19 +++++++++++++++++++
> > 2 files changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/libstdc++-v3/include/std/ranges
> > b/libstdc++-v3/include/std/ranges
> > index 856975c6934..e858df88088 100644
> > --- a/libstdc++-v3/include/std/ranges
> > +++ b/libstdc++-v3/include/std/ranges
> > @@ -818,8 +818,10 @@ namespace views::__adaptor
> > // True if the behavior of the range adaptor non-closure _Adaptor is
> > // independent of the value category of its extra arguments _Args.
> > template<typename _Adaptor, typename... _Args>
> > - concept __adaptor_has_simple_extra_args =
> > _Adaptor::_S_has_simple_extra_args
> > - || _Adaptor::template _S_has_simple_extra_args<_Args...>;
> > + concept __adaptor_has_simple_extra_args
> > + = (_Adaptor::_S_has_simple_extra_args
> > + || _Adaptor::template _S_has_simple_extra_args<_Args...>)
> > + && (is_trivially_copyable_v<_Args> && ...);
>
> On second thought, perhaps it'd be cleaner to leave this concept alone
> and instead encode the trivial-copyability requirement as a separate
> constraint on the relevant partial specializations of _Partial?
> Something like:
OK for trunk and 11, thanks.
>
> -- >8 --
>
>
> PR libstdc++/100940
>
> libstdc++-v3/ChangeLog:
>
> * include/std/ranges (__adaptor::_Partial): For the "simple"
> forwarding partial specializations, also require that
> the extra arguments are trivially copyable.
> * testsuite/std/ranges/adaptors/100577.cc (test04): New test.
> ---
> libstdc++-v3/include/std/ranges | 8 +++++---
> .../testsuite/std/ranges/adaptors/100577.cc | 14 ++++++++++++++
> 2 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
> index 856975c6934..24411124580 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -892,11 +892,12 @@ namespace views::__adaptor
> };
>
> // Partial specialization of the primary template for the case where the
> extra
> - // arguments of the adaptor can always be safely forwarded by const
> reference.
> - // This lets us get away with a single operator() overload, which makes
> - // overload resolution failure diagnostics more concise.
> + // arguments of the adaptor can always be safely and efficiently forwarded
> by
> + // const reference. This lets us get away with a single operator()
> overload,
> + // which makes overload resolution failure diagnostics more concise.
> template<typename _Adaptor, typename... _Args>
> requires __adaptor_has_simple_extra_args<_Adaptor, _Args...>
> + && (is_trivially_copyable_v<_Args> && ...)
> struct _Partial<_Adaptor, _Args...> : _RangeAdaptorClosure
> {
> tuple<_Args...> _M_args;
> @@ -926,6 +927,7 @@ namespace views::__adaptor
> // where _Adaptor accepts a single extra argument.
> template<typename _Adaptor, typename _Arg>
> requires __adaptor_has_simple_extra_args<_Adaptor, _Arg>
> + && is_trivially_copyable_v<_Arg>
> struct _Partial<_Adaptor, _Arg> : _RangeAdaptorClosure
> {
> _Arg _M_arg;
> diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/100577.cc
> b/libstdc++-v3/testsuite/std/ranges/adaptors/100577.cc
> index 8ef084621f9..06be4980ddb 100644
> --- a/libstdc++-v3/testsuite/std/ranges/adaptors/100577.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/100577.cc
> @@ -21,6 +21,7 @@
> // PR libstdc++/100577
>
> #include <ranges>
> +#include <functional>
>
> namespace ranges = std::ranges;
> namespace views = std::ranges::views;
> @@ -113,4 +114,17 @@ test03()
> x | std::views::drop(S{});
> }
>
> +void
> +test04()
> +{
> + // Non-trivially-copyable extra arguments make a closure not simple.
> + using F = std::function<bool(bool)>;
> + static_assert(!std::is_trivially_copyable_v<F>);
> + using views::__adaptor::__closure_has_simple_call_op;
> +
> static_assert(!__closure_has_simple_call_op<decltype(views::take_while(std::declval<F>()))>);
> +
> static_assert(!__closure_has_simple_call_op<decltype(views::drop_while(std::declval<F>()))>);
> +
> static_assert(!__closure_has_simple_call_op<decltype(views::filter(std::declval<F>()))>);
> +
> static_assert(!__closure_has_simple_call_op<decltype(views::transform(std::declval<F>()))>);
> +}
> +
> // { dg-prune-output "in requirements" }
> --
> 2.32.0.93.g670b81a890
>
