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 <ppa...@redhat.com>
---
Changes in v3:
- reworked pass_by_rref to pass_by_value, and simplified impl
- fixed some typos in the commit message

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          | 20 ++++++++++-
 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, 202 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..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<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 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

Reply via email to