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:
-- >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
