From: Tomasz Kamiński Date: Wed, 28 May 2025 09:16:22 +0000 (+0200) Subject: libstdc++: Pass small trivial types by value in polymorphic wrappers X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=08031b88e2a32cc334af540703c5332a6b716529;p=thirdparty%2Fgcc.git libstdc++: Pass small trivial types by value in polymorphic wrappers 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, they are passed by value to the invoker, when they are small (at most two pointers), trivially move constructible and trivially destructible. The latter guarantees that passing them by value has not user visible side effects. In particular, this extends 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 a consequence, the implementation imposes requirements on instantiation of move_only_function and copyable_function. To avoid producing the errors from the implementation details, a 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. Reviewed-by: Patrick Palka Reviewed-by: Jonathan Wakely Signed-off-by: Tomasz Kamiński --- 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..9db4ab7811a 100644 --- a/libstdc++-v3/include/bits/funcwrap.h +++ b/libstdc++-v3/include/bits/funcwrap.h @@ -199,7 +199,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION }; template - using __param_t = __conditional_t, _Tp, _Tp&&>; + consteval bool + __pass_by_value() + { + // n.b. sizeof(Incomplete&) is ill-formed for incomplete types, + // so we check is_reference_v first. + if constexpr (is_reference_v<_Tp> || is_scalar_v<_Tp>) + return true; + else + // n.b. we already asserted that types are complete in wrappers, + // avoid triggering additional errors from this function. + if constexpr (std::__is_complete_or_unbounded(__type_identity<_Tp>())) + if constexpr (sizeof(_Tp) <= 2 * sizeof(void*)) + return is_trivially_move_constructible_v<_Tp> + && is_trivially_destructible_v<_Tp>; + return false; + } + + template + using __param_t = __conditional_t<__pass_by_value<_Tp>(), _Tp, _Tp&&>; template 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 f1; - std::copyable_function f2; - std::copyable_function f3; + std::copyable_function f1; + std::copyable_function f2; + std::copyable_function 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 +#include #include using std::copyable_function; @@ -15,6 +16,21 @@ static_assert( !std::is_constructible_v, static_assert( !std::is_constructible_v, std::copyable_function> ); +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> ); +static_assert( std::is_same_v, + std::copyable_function>); +static_assert( std::is_same_v, + std::copyable_function>); +static_assert( std::is_same_v, + std::copyable_function>); +static_assert( std::is_same_v, + std::copyable_function>); + // 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 ci1(fi); + VERIFY( ci1(c, 0) == 1 ); + std::copyable_function ci2(ci1); + VERIFY( ci2(c, 0) == 2 ); + + auto fs = [](CountedArg const& arg, std::string_view) noexcept { return arg.counter; }; + std::copyable_function cs1(fs); + VERIFY( cs1(c, "") == 1 ); + std::copyable_function 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 + +struct IncompleteClass; + +using T1 = std::copyable_function::result_type; // { dg-error "here" } +using T2 = std::copyable_function::result_type; // { dg-error "here" } + +enum Enum { + x = [] { + // Enum enumeration is incomplete here + using T3 = std::copyable_function::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 f1(f); - std::function_ref f2(f); - // See PR120259, this should be supported. - // std::function_ref f3(f); + std::function_ref f1(f); + // See PR libstdc++/120259, this should be supported. + // std::function_ref f2(f); + std::function_ref 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 +#include #include 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> ); +static_assert( std::is_same_v, + std::function_ref>); +static_assert( std::is_same_v, + std::function_ref>); +static_assert( std::is_same_v, + std::function_ref>); +static_assert( std::is_same_v, + std::function_ref>); + // 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 ri1(fi); + VERIFY( ri1(c, 0) == 1 ); + std::function_ref ri2(ri1); + VERIFY( ri2(c, 0) == 2 ); + + auto fs = [](CountedArg const& arg, std::string_view) noexcept { return arg.counter; }; + std::function_ref rs1(fs); + VERIFY( rs1(c, "") == 1 ); + std::function_ref 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 + +struct IncompleteClass; + +int a1 = alignof(std::function_ref); // { dg-error "here" } +int a2 = alignof(std::function_ref); // { dg-error "here" } + +enum Enum { + x = [] { + // Enum enumeration is incomplete here + int a3 = alignof(std::function_ref); // { 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 f1; - std::move_only_function f2; - std::move_only_function f3; + std::move_only_function f1; + std::move_only_function f2; + std::move_only_function 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 +#include #include using std::move_only_function; @@ -15,6 +16,21 @@ static_assert( !std::is_constructible_v, static_assert( !std::is_constructible_v, std::move_only_function> ); +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> ); +static_assert( std::is_same_v, + std::move_only_function>); +static_assert( std::is_same_v, + std::move_only_function>); +static_assert( std::is_same_v, + std::move_only_function>); +static_assert( std::is_same_v, + std::move_only_function>); + // 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 mi1(fi); + VERIFY( mi1(c, 0) == 1 ); + std::move_only_function 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 ms1(fs); + VERIFY( ms1(c, "") == 1 ); + std::move_only_function 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 + +struct IncompleteClass; + +using T1 = std::move_only_function::result_type; // { dg-error "here" } +using T2 = std::move_only_function::result_type; // { dg-error "here" } + +enum Enum { + x = [] { + // Enum enumeration is incomplete here + using T3 = std::move_only_function::result_type; // { dg-error "here" } + return T3(1); + }() +}; + +// { dg-error "static assertion failed" "" { target *-*-* } 0 }