This patch adjust the passing of parameters for the move_only_function, copyable_function and function_ref. For types that are declared as being passed by value in signature template argument, the are passed by value to the invoker, when they are small (at most two pointers), trivially move constructible and trivially destructible. The later guarantees that passing them by value has not user visible side effects.
In particular, this extents the set of types forwarded by value, that was previously limited to scalars, to also include specializations of std::span and std::string_view, and similar standard and program defined-types. Checking the suitability of the parameter types requires the types to be complete. As consequence implementation imposes requirements on instantiation of move_only_function and copyable_function. To avoid producing the errors from the implementation details, and static_assertion was added to partial specializations of copyable_function, move_only_function and function_ref. The static assertion uses existing __is_complete_or_unbounded, as arrays type parameters are automatically decayed in function type. Standard already specifies in [res.on.functions] p2.5 that instantiating these partial specialization with incomplete types leads to undefined behavior. libstdc++-v3/ChangeLog: * include/bits/funcwrap.h (__polyfunc::__pass_by_rref): Define. (__polyfunc::__param_t): Update to use __pass_by_rref. * include/bits/cpyfunc_impl.h:: Assert that are parameters type are complete. * include/bits/funcref_impl.h: Likewise. * include/bits/mofunc_impl.h: Likewise. * testsuite/20_util/copyable_function/call.cc: New test. * testsuite/20_util/function_ref/call.cc: New test. * testsuite/20_util/move_only_function/call.cc: New test. * testsuite/20_util/copyable_function/conv.cc: New test. * testsuite/20_util/function_ref/conv.cc: New test. * testsuite/20_util/move_only_function/conv.cc: New test. * testsuite/20_util/copyable_function/incomplete_neg.cc: New test. * testsuite/20_util/function_ref/incomplete_neg.cc: New test. * testsuite/20_util/move_only_function/incomplete_neg.cc: New test. --- Tested on x86_54-linux. OK for trunk? libstdc++-v3/include/bits/cpyfunc_impl.h | 4 +++ libstdc++-v3/include/bits/funcref_impl.h | 4 +++ libstdc++-v3/include/bits/funcwrap.h | 18 +++++++++- libstdc++-v3/include/bits/mofunc_impl.h | 4 +++ .../20_util/copyable_function/call.cc | 7 ++-- .../20_util/copyable_function/conv.cc | 35 +++++++++++++++++++ .../copyable_function/incomplete_neg.cc | 18 ++++++++++ .../testsuite/20_util/function_ref/call.cc | 10 +++--- .../testsuite/20_util/function_ref/conv.cc | 34 ++++++++++++++++++ .../20_util/function_ref/incomplete_neg.cc | 18 ++++++++++ .../20_util/move_only_function/call.cc | 7 ++-- .../20_util/move_only_function/conv.cc | 35 +++++++++++++++++++ .../move_only_function/incomplete_neg.cc | 18 ++++++++++ 13 files changed, 200 insertions(+), 12 deletions(-) create mode 100644 libstdc++-v3/testsuite/20_util/copyable_function/incomplete_neg.cc create mode 100644 libstdc++-v3/testsuite/20_util/function_ref/incomplete_neg.cc create mode 100644 libstdc++-v3/testsuite/20_util/move_only_function/incomplete_neg.cc diff --git a/libstdc++-v3/include/bits/cpyfunc_impl.h b/libstdc++-v3/include/bits/cpyfunc_impl.h index bc44cd3e313..f1918ddf87a 100644 --- a/libstdc++-v3/include/bits/cpyfunc_impl.h +++ b/libstdc++-v3/include/bits/cpyfunc_impl.h @@ -64,6 +64,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _GLIBCXX_MOF_REF noexcept(_Noex)> : __polyfunc::_Cpy_base { + static_assert( + (std::__is_complete_or_unbounded(__type_identity<_ArgTypes>()) && ...), + "each parameter type must be a complete class"); + using _Base = __polyfunc::_Cpy_base; using _Invoker = __polyfunc::_Invoker<_Noex, _Res, _ArgTypes...>; using _Signature = _Invoker::_Signature; diff --git a/libstdc++-v3/include/bits/funcref_impl.h b/libstdc++-v3/include/bits/funcref_impl.h index 1e19866035f..44c992281be 100644 --- a/libstdc++-v3/include/bits/funcref_impl.h +++ b/libstdc++-v3/include/bits/funcref_impl.h @@ -68,6 +68,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION class function_ref<_Res(_ArgTypes...) _GLIBCXX_MOF_CV noexcept(_Noex)> { + static_assert( + (std::__is_complete_or_unbounded(__type_identity<_ArgTypes>()) && ...), + "each parameter type must be a complete class"); + using _Invoker = __polyfunc::_Invoker<_Noex, _Res, _ArgTypes...>; using _Signature = _Invoker::_Signature; diff --git a/libstdc++-v3/include/bits/funcwrap.h b/libstdc++-v3/include/bits/funcwrap.h index cf261bcd4c8..4fa9e1e84f2 100644 --- a/libstdc++-v3/include/bits/funcwrap.h +++ b/libstdc++-v3/include/bits/funcwrap.h @@ -199,7 +199,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION }; template<typename _Tp> - using __param_t = __conditional_t<is_scalar_v<_Tp>, _Tp, _Tp&&>; + consteval bool __pass_by_rref() + { + if constexpr (is_reference_v<_Tp> || is_scalar_v<_Tp>) + return false; + else if constexpr (!std::__is_complete_or_unbounded(__type_identity<_Tp>())) + // N.B. we already asserted that types are complete in wrappers, + // avoid triggering additional errors from this function. + return true; + else if constexpr (sizeof(_Tp) <= 2 * sizeof(void*)) + return !is_trivially_move_constructible_v<_Tp> + || !is_trivially_destructible_v<_Tp>; + else + return true; + } + + template<typename _Tp> + using __param_t = __conditional_t<__pass_by_rref<_Tp>(), _Tp&&, _Tp>; template<bool _Noex, typename _Ret, typename... _Args> using _Invoker = _Base_invoker<_Noex, remove_cv_t<_Ret>, __param_t<_Args>...>; diff --git a/libstdc++-v3/include/bits/mofunc_impl.h b/libstdc++-v3/include/bits/mofunc_impl.h index 1ceb910e815..468e6855fc6 100644 --- a/libstdc++-v3/include/bits/mofunc_impl.h +++ b/libstdc++-v3/include/bits/mofunc_impl.h @@ -64,6 +64,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _GLIBCXX_MOF_REF noexcept(_Noex)> : __polyfunc::_Mo_base { + static_assert( + (std::__is_complete_or_unbounded(__type_identity<_ArgTypes>()) && ...), + "each parameter type must be a complete class"); + using _Base = __polyfunc::_Mo_base; using _Invoker = __polyfunc::_Invoker<_Noex, _Res, _ArgTypes...>; using _Signature = _Invoker::_Signature; diff --git a/libstdc++-v3/testsuite/20_util/copyable_function/call.cc b/libstdc++-v3/testsuite/20_util/copyable_function/call.cc index cf997577f62..0ac5348f2fe 100644 --- a/libstdc++-v3/testsuite/20_util/copyable_function/call.cc +++ b/libstdc++-v3/testsuite/20_util/copyable_function/call.cc @@ -204,13 +204,14 @@ test05() } struct Incomplete; +enum CompleteEnum : int; void test_params() { - std::copyable_function<void(Incomplete)> f1; - std::copyable_function<void(Incomplete&)> f2; - std::copyable_function<void(Incomplete&&)> f3; + std::copyable_function<void(Incomplete&)> f1; + std::copyable_function<void(Incomplete&&)> f2; + std::copyable_function<void(CompleteEnum)> f4; } int main() diff --git a/libstdc++-v3/testsuite/20_util/copyable_function/conv.cc b/libstdc++-v3/testsuite/20_util/copyable_function/conv.cc index e678e166172..11c839b6932 100644 --- a/libstdc++-v3/testsuite/20_util/copyable_function/conv.cc +++ b/libstdc++-v3/testsuite/20_util/copyable_function/conv.cc @@ -2,6 +2,7 @@ // { dg-require-effective-target hosted } #include <functional> +#include <string_view> #include <testsuite_hooks.h> using std::copyable_function; @@ -15,6 +16,21 @@ static_assert( !std::is_constructible_v<std::copyable_function<void()&>, static_assert( !std::is_constructible_v<std::copyable_function<void() const>, std::copyable_function<void()>> ); +using FuncType = int(int); + +// Top level const qualifiers are ignored and decay is performed in parameters +// of function_types. +static_assert( std::is_same_v<std::copyable_function<void(int const)>, + std::copyable_function<void(int)>> ); +static_assert( std::is_same_v<std::copyable_function<void(int[2])>, + std::copyable_function<void(int*)>>); +static_assert( std::is_same_v<std::copyable_function<void(int[])>, + std::copyable_function<void(int*)>>); +static_assert( std::is_same_v<std::copyable_function<void(int const[5])>, + std::copyable_function<void(int const*)>>); +static_assert( std::is_same_v<std::copyable_function<void(FuncType)>, + std::copyable_function<void(FuncType*)>>); + // Non-trivial args, guarantess that type is not passed by copy struct CountedArg { @@ -240,6 +256,24 @@ test06() VERIFY( f2(c) == 2 ); } +void +test07() +{ + // Scalar types and small trivially move constructible types are passed + // by value to invoker. So int&& signature is not compatible for such types. + auto fi = [](CountedArg const& arg, int) noexcept { return arg.counter; }; + std::copyable_function<int(CountedArg, int) const noexcept> ci1(fi); + VERIFY( ci1(c, 0) == 1 ); + std::copyable_function<int(CountedArg, int&&) const noexcept> ci2(ci1); + VERIFY( ci2(c, 0) == 2 ); + + auto fs = [](CountedArg const& arg, std::string_view) noexcept { return arg.counter; }; + std::copyable_function<int(CountedArg, std::string_view) const noexcept> cs1(fs); + VERIFY( cs1(c, "") == 1 ); + std::copyable_function<int(CountedArg, std::string_view&&) const noexcept> cs2(cs1); + VERIFY( cs2(c, "") == 2 ); +} + int main() { test01(); @@ -248,4 +282,5 @@ int main() test04(); test05(); test06(); + test07(); } diff --git a/libstdc++-v3/testsuite/20_util/copyable_function/incomplete_neg.cc b/libstdc++-v3/testsuite/20_util/copyable_function/incomplete_neg.cc new file mode 100644 index 00000000000..21ddde0d2a1 --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/copyable_function/incomplete_neg.cc @@ -0,0 +1,18 @@ +// { dg-do compile { target c++26 } } + +#include <functional> + +struct IncompleteClass; + +using T1 = std::copyable_function<int(IncompleteClass)>::result_type; // { dg-error "here" } +using T2 = std::copyable_function<int(int, IncompleteClass)>::result_type; // { dg-error "here" } + +enum Enum { + x = [] { + // Enum enumeration is incomplete here + using T3 = std::copyable_function<int(Enum)>::result_type; // { dg-error "here" } + return T3(1); + }() +}; + +// { dg-error "static assertion failed" "" { target *-*-* } 0 } diff --git a/libstdc++-v3/testsuite/20_util/function_ref/call.cc b/libstdc++-v3/testsuite/20_util/function_ref/call.cc index a91c6b4abb9..23253c33ee3 100644 --- a/libstdc++-v3/testsuite/20_util/function_ref/call.cc +++ b/libstdc++-v3/testsuite/20_util/function_ref/call.cc @@ -164,16 +164,16 @@ test05() } struct Incomplete; +enum CompleteEnum : int; void test_params() { auto f = [](auto&&) {}; - // There is discussion if this is supported. - // std::function_ref<void(Incomplete)> f1(f); - std::function_ref<void(Incomplete&)> f2(f); - // See PR120259, this should be supported. - // std::function_ref<void(Incomplete&&)> f3(f); + std::function_ref<void(Incomplete&)> f1(f); + // See PR libstdc++/120259, this should be supported. + // std::function_ref<void(Incomplete&&)> f2(f); + std::function_ref<void(CompleteEnum)> f3(f); } int main() diff --git a/libstdc++-v3/testsuite/20_util/function_ref/conv.cc b/libstdc++-v3/testsuite/20_util/function_ref/conv.cc index 7dc7b8ceac0..7606d265f98 100644 --- a/libstdc++-v3/testsuite/20_util/function_ref/conv.cc +++ b/libstdc++-v3/testsuite/20_util/function_ref/conv.cc @@ -2,6 +2,7 @@ // { dg-require-effective-target hosted } #include <functional> +#include <string_view> #include <testsuite_hooks.h> using std::function_ref; @@ -20,6 +21,21 @@ struct CountedArg }; CountedArg const c; +using FuncType = int(int); + +// Top level const qualifiers are ignored in function types, and decay +// is performed. +static_assert( std::is_same_v<std::function_ref<void(int const)>, + std::function_ref<void(int)>> ); +static_assert( std::is_same_v<std::function_ref<void(int[2])>, + std::function_ref<void(int*)>>); +static_assert( std::is_same_v<std::function_ref<void(int[])>, + std::function_ref<void(int*)>>); +static_assert( std::is_same_v<std::function_ref<void(int const[5])>, + std::function_ref<void(int const*)>>); +static_assert( std::is_same_v<std::function_ref<void(FuncType)>, + std::function_ref<void(FuncType*)>>); + // The C++26 [func.wrap.general] p2 does not currently cover funciton_ref, // so we make extra copies of arguments. @@ -244,6 +260,23 @@ test08() return true; }; +void +test09() +{ + // Scalar types and small trivially move constructible types are passed + // by value to invoker. So int&& signature is not compatible for such types. + auto fi = [](CountedArg const& arg, int) noexcept { return arg.counter; }; + std::function_ref<int(CountedArg, int) const noexcept> ri1(fi); + VERIFY( ri1(c, 0) == 1 ); + std::function_ref<int(CountedArg, int&&) const noexcept> ri2(ri1); + VERIFY( ri2(c, 0) == 2 ); + + auto fs = [](CountedArg const& arg, std::string_view) noexcept { return arg.counter; }; + std::function_ref<int(CountedArg, std::string_view) const noexcept> rs1(fs); + VERIFY( rs1(c, "") == 1 ); + std::function_ref<int(CountedArg, std::string_view&&) const noexcept> rs2(rs1); + VERIFY( rs2(c, "") == 2 ); +} int main() { @@ -254,6 +287,7 @@ int main() test05(); test06(); test07(); + test09(); static_assert( test08() ); } diff --git a/libstdc++-v3/testsuite/20_util/function_ref/incomplete_neg.cc b/libstdc++-v3/testsuite/20_util/function_ref/incomplete_neg.cc new file mode 100644 index 00000000000..c8db1eee42c --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/function_ref/incomplete_neg.cc @@ -0,0 +1,18 @@ +// { dg-do compile { target c++26 } } + +#include <functional> + +struct IncompleteClass; + +int a1 = alignof(std::function_ref<int(IncompleteClass)>); // { dg-error "here" } +int a2 = alignof(std::function_ref<int(int, IncompleteClass)>); // { dg-error "here" } + +enum Enum { + x = [] { + // Enum enumeration is incomplete here + int a3 = alignof(std::function_ref<int(Enum)>); // { dg-error "here" } + return 1; + }() +}; + +// { dg-error "static assertion failed" "" { target *-*-* } 0 } diff --git a/libstdc++-v3/testsuite/20_util/move_only_function/call.cc b/libstdc++-v3/testsuite/20_util/move_only_function/call.cc index 217de374763..72c8118d716 100644 --- a/libstdc++-v3/testsuite/20_util/move_only_function/call.cc +++ b/libstdc++-v3/testsuite/20_util/move_only_function/call.cc @@ -204,13 +204,14 @@ test05() } struct Incomplete; +enum CompleteEnum : int; void test_params() { - std::move_only_function<void(Incomplete)> f1; - std::move_only_function<void(Incomplete&)> f2; - std::move_only_function<void(Incomplete&&)> f3; + std::move_only_function<void(Incomplete&)> f1; + std::move_only_function<void(Incomplete&&)> f2; + std::move_only_function<void(CompleteEnum)> f4; } int main() diff --git a/libstdc++-v3/testsuite/20_util/move_only_function/conv.cc b/libstdc++-v3/testsuite/20_util/move_only_function/conv.cc index 3da5e9e90a3..ef8bb37b232 100644 --- a/libstdc++-v3/testsuite/20_util/move_only_function/conv.cc +++ b/libstdc++-v3/testsuite/20_util/move_only_function/conv.cc @@ -2,6 +2,7 @@ // { dg-require-effective-target hosted } #include <functional> +#include <string_view> #include <testsuite_hooks.h> using std::move_only_function; @@ -15,6 +16,21 @@ static_assert( !std::is_constructible_v<std::move_only_function<void()&>, static_assert( !std::is_constructible_v<std::move_only_function<void() const>, std::move_only_function<void()>> ); +using FuncType = int(int); + +// Top level const qualifiers are ignored in function types, and decay +// is performed. +static_assert( std::is_same_v<std::move_only_function<void(int const)>, + std::move_only_function<void(int)>> ); +static_assert( std::is_same_v<std::move_only_function<void(int[2])>, + std::move_only_function<void(int*)>>); +static_assert( std::is_same_v<std::move_only_function<void(int[])>, + std::move_only_function<void(int*)>>); +static_assert( std::is_same_v<std::move_only_function<void(int const[5])>, + std::move_only_function<void(int const*)>>); +static_assert( std::is_same_v<std::move_only_function<void(FuncType)>, + std::move_only_function<void(FuncType*)>>); + // Non-trivial args, guarantess that type is not passed by copy struct CountedArg { @@ -177,6 +193,24 @@ test06() VERIFY( m1(c) == 2 ); } +void +test07() +{ + // Scalar types and small trivially move constructible types are passed + // by value to invoker. So int&& signature is not compatible for such types. + auto fi = [](CountedArg const& arg, int) noexcept { return arg.counter; }; + std::move_only_function<int(CountedArg, int) const noexcept> mi1(fi); + VERIFY( mi1(c, 0) == 1 ); + std::move_only_function<int(CountedArg, int&&) const noexcept> mi2(std::move(mi1)); + VERIFY( mi2(c, 0) == 2 ); + + auto fs = [](CountedArg const& arg, std::string_view) noexcept { return arg.counter; }; + std::move_only_function<int(CountedArg, std::string_view) const noexcept> ms1(fs); + VERIFY( ms1(c, "") == 1 ); + std::move_only_function<int(CountedArg, std::string_view&&) const noexcept> ms2(std::move(ms1)); + VERIFY( ms2(c, "") == 2 ); +} + int main() { test01(); @@ -185,4 +219,5 @@ int main() test04(); test05(); test06(); + test07(); } diff --git a/libstdc++-v3/testsuite/20_util/move_only_function/incomplete_neg.cc b/libstdc++-v3/testsuite/20_util/move_only_function/incomplete_neg.cc new file mode 100644 index 00000000000..d025c473e28 --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/move_only_function/incomplete_neg.cc @@ -0,0 +1,18 @@ +// { dg-do compile { target c++23 } } + +#include <functional> + +struct IncompleteClass; + +using T1 = std::move_only_function<int(IncompleteClass)>::result_type; // { dg-error "here" } +using T2 = std::move_only_function<int(int, IncompleteClass)>::result_type; // { dg-error "here" } + +enum Enum { + x = [] { + // Enum enumeration is incomplete here + using T3 = std::move_only_function<int(Enum)>::result_type; // { dg-error "here" } + return T3(1); + }() +}; + +// { dg-error "static assertion failed" "" { target *-*-* } 0 } -- 2.49.0