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 <ppalka@redhat.com>
Reviewed-by: Jonathan Wakely <jwakely@redhat.com>
Signed-off-by: Tomasz Kamiński <tkaminsk@redhat.com>

diff --git a/libstdc++-v3/include/bits/cpyfunc_impl.h b/libstdc++-v3/include/bits/cpyfunc_impl.h
index bc44cd3e313d4b7297a7ef4d04b1f3ad495a49a1..f1918ddf87a30a904b296ec4a732e9ad52c19fd1 100644 (file)
--- 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 1e19866035f159ff98067ab1bae5a407e58a9395..44c992281be21664bb697f8883dedfd3c30c30ee 100644 (file)
--- 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 cf261bcd4c8d67e8ca523ce0a198837e003129fb..9db4ab7811a88658c6e2c2fcb3342e496b363ebd 100644 (file)
--- a/libstdc++-v3/include/bits/funcwrap.h
+++ b/libstdc++-v3/include/bits/funcwrap.h
@@ -199,7 +199,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
      };
 
    template<typename _Tp>
-     using __param_t = __conditional_t<is_scalar_v<_Tp>, _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<typename _Tp>
+     using __param_t = __conditional_t<__pass_by_value<_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 1ceb910e815b1975e7e96ba7f1c96542309e5020..468e6855fc6b74e9a1b2ab595ddf81304721ccff 100644 (file)
--- 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 cf997577f6275e67d8c7ad25cc10cf835fe665e8..0ac5348f2fe19d68c666b86eb984191a63a3d5b7 100644 (file)
--- 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 e678e166172cd7bbf0cb9a37940dea0ac4da8f64..11c839b6932f6ebee27b14878969321877aa7058 100644 (file)
--- 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 (file)
index 0000000..21ddde0
--- /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 a91c6b4abb93d8a871d7723039c4e4a6ae7194c5..23253c33ee392c1ae7639bfc1401a8b3f7090b70 100644 (file)
--- 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 7dc7b8ceac000fd758e306c017a3cf6b5e9dd085..7606d265f98617da475d16345104c0eb5fb7f8cd 100644 (file)
--- 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 (file)
index 0000000..c8db1ee
--- /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 217de374763068edd4f1346fe20ffde961488139..72c8118d716eacffbfe7f988cc6d211d301bc6f5 100644 (file)
--- 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 3da5e9e90a3110256e49b5ac55ef566a3421e9d1..ef8bb37b2320c9859b8977291426fa8e59b52863 100644 (file)
--- 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 (file)
index 0000000..d025c47
--- /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 }