[PATCH] PR libstdc++/91456 make INVOKE<R> work with uncopyable prvalues

[PATCH] PR libstdc++/91456 make INVOKE<R> work with uncopyable prvalues

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 1468 bytes --]

In C++17 a function can return a prvalue of a type that cannot be moved
or copied. The current implementation of std::is_invocable_r uses
std::is_convertible to test the conversion to R required by INVOKE<R>.
That fails for non-copyable prvalues, because std::is_convertible is
defined in terms of std::declval which uses std::add_rvalue_reference.
In C++17 conversion from R to R involves no copies and so is not the
same as conversion from R&& to R.

This commit changes std::is_invocable_r to check the conversion without
using std::is_convertible.

std::function also contains a similar check using std::is_convertible,
which can be fixed by simply reusing std::is_invocable_r (but because
std::is_invocable_r is not defined for C++11 it uses the underlying
std::__is_invocable_impl trait directly).

	PR libstdc++/91456
	* include/bits/std_function.h (__check_func_return_type): Remove.
	(function::_Callable): Use std::__is_invocable_impl instead of
	__check_func_return_type.
	* include/std/type_traits (__is_invocable_impl): Add another defaulted
	template parameter. Define a separate partial specialization for
	INVOKE and INVOKE<void>. For INVOKE<R> replace is_convertible check
	with a check that models delayed temporary materialization.
	* testsuite/20_util/function/91456.cc: New test.
	* testsuite/20_util/is_invocable/91456.cc: New test.

Tested x86_64-linux, committed to trunk.

I might backport this to gcc-9-branch too, after some time on trunk.


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 8037 bytes --]

commit a9ae95a61b2d8b5ccbbaff1f9bd0b3ed70c600ed
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Aug 15 15:46:39 2019 +0100

    PR libstdc++/91456 make INVOKE<R> work with uncopyable prvalues
    
    In C++17 a function can return a prvalue of a type that cannot be moved
    or copied. The current implementation of std::is_invocable_r uses
    std::is_convertible to test the conversion to R required by INVOKE<R>.
    That fails for non-copyable prvalues, because std::is_convertible is
    defined in terms of std::declval which uses std::add_rvalue_reference.
    In C++17 conversion from R to R involves no copies and so is not the
    same as conversion from R&& to R.
    
    This commit changes std::is_invocable_r to check the conversion without
    using std::is_convertible.
    
    std::function also contains a similar check using std::is_convertible,
    which can be fixed by simply reusing std::is_invocable_r (but because
    std::is_invocable_r is not defined for C++11 it uses the underlying
    std::__is_invocable_impl trait directly).
    
            PR libstdc++/91456
            * include/bits/std_function.h (__check_func_return_type): Remove.
            (function::_Callable): Use std::__is_invocable_impl instead of
            __check_func_return_type.
            * include/std/type_traits (__is_invocable_impl): Add another defaulted
            template parameter. Define a separate partial specialization for
            INVOKE and INVOKE<void>. For INVOKE<R> replace is_convertible check
            with a check that models delayed temporary materialization.
            * testsuite/20_util/function/91456.cc: New test.
            * testsuite/20_util/is_invocable/91456.cc: New test.

diff --git a/libstdc++-v3/include/bits/std_function.h b/libstdc++-v3/include/bits/std_function.h
index 5733bf5f3f9..42f87873d55 100644
--- a/libstdc++-v3/include/bits/std_function.h
+++ b/libstdc++-v3/include/bits/std_function.h
@@ -293,10 +293,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }
     };
 
-  template<typename _From, typename _To>
-    using __check_func_return_type
-      = __or_<is_void<_To>, is_same<_From, _To>, is_convertible<_From, _To>>;
-
   /**
    *  @brief Primary class template for std::function.
    *  @ingroup functors
@@ -309,8 +305,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       private _Function_base
     {
       template<typename _Func,
-	       typename _Res2 = typename result_of<_Func&(_ArgTypes...)>::type>
-	struct _Callable : __check_func_return_type<_Res2, _Res> { };
+	       typename _Res2 = __invoke_result<_Func&, _ArgTypes...>>
+	struct _Callable
+	: __is_invocable_impl<_Res2, _Res>::type
+	{ };
 
       // Used so the return type convertibility checks aren't done when
       // performing overload resolution for copy construction/assignment.
diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
index d3f853d4ce2..44db2cade5d 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -2883,14 +2883,49 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   // __is_invocable (std::is_invocable for C++11)
 
-  template<typename _Result, typename _Ret, typename = void>
+  // The primary template is used for invalid INVOKE expressions.
+  template<typename _Result, typename _Ret,
+	   bool = is_void<_Ret>::value, typename = void>
     struct __is_invocable_impl : false_type { };
 
+  // Used for valid INVOKE and INVOKE<void> expressions.
   template<typename _Result, typename _Ret>
-    struct __is_invocable_impl<_Result, _Ret, __void_t<typename _Result::type>>
-    : __or_<is_void<_Ret>, is_convertible<typename _Result::type, _Ret>>::type
+    struct __is_invocable_impl<_Result, _Ret,
+			       /* is_void<_Ret> = */ true,
+			       __void_t<typename _Result::type>>
+    : true_type
     { };
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wctor-dtor-privacy"
+  // Used for INVOKE<R> expressions to check the implicit conversion to R.
+  template<typename _Result, typename _Ret>
+    struct __is_invocable_impl<_Result, _Ret,
+			       /* is_void<_Ret> = */ false,
+			       __void_t<typename _Result::type>>
+    {
+    private:
+      // The type of the INVOKE expression.
+      // Unlike declval, this doesn't add_rvalue_reference.
+      static typename _Result::type _S_get();
+
+      template<typename _Tp>
+	static void _S_conv(_Tp);
+
+      // This overload is viable if INVOKE(f, args...) can convert to _Tp.
+      template<typename _Tp, typename = decltype(_S_conv<_Tp>(_S_get()))>
+	static true_type
+	_S_test(int);
+
+      template<typename _Tp>
+	static false_type
+	_S_test(...);
+
+    public:
+      using type = decltype(_S_test<_Ret>(1));
+    };
+#pragma GCC diagnostic pop
+
   template<typename _Fn, typename... _ArgTypes>
     struct __is_invocable
     : __is_invocable_impl<__invoke_result<_Fn, _ArgTypes...>, void>::type
diff --git a/libstdc++-v3/testsuite/20_util/function/91456.cc b/libstdc++-v3/testsuite/20_util/function/91456.cc
new file mode 100644
index 00000000000..a2d412d7bec
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/function/91456.cc
@@ -0,0 +1,37 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++17" }
+// { dg-do compile { target c++17 } }
+
+#include <functional>
+
+struct Immovable {
+  Immovable() = default;
+  Immovable(const Immovable&) = delete;
+  Immovable& operator=(const Immovable&) = delete;
+};
+
+Immovable get() { return {}; }
+const Immovable i = get();                      // OK
+std::function<const Immovable()> f{&get};       // fails
+const Immovable i2 = f();
+
+const Immovable cget() { return {}; }
+Immovable ci = cget();                          // OK
+std::function<Immovable()> cf{&cget};           // fails
+Immovable ci2 = cf();
diff --git a/libstdc++-v3/testsuite/20_util/is_invocable/91456.cc b/libstdc++-v3/testsuite/20_util/is_invocable/91456.cc
new file mode 100644
index 00000000000..d510d221a7d
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/is_invocable/91456.cc
@@ -0,0 +1,34 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++17" }
+// { dg-do compile { target c++17 } }
+
+#include <type_traits>
+
+#include <functional>
+
+struct Immovable {
+  Immovable() = default;
+  Immovable(const Immovable&) = delete;
+  Immovable& operator=(const Immovable&) = delete;
+};
+
+Immovable get() { return {}; }
+const Immovable i = get();                      // OK
+std::function<const Immovable()> f{&get};       // fails
+const Immovable i2 = f();

Re: [PATCH] PR libstdc++/91456 make INVOKE<R> work with uncopyable prvalues

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 1624 bytes --]

On 15/08/19 17:04 +0100, Jonathan Wakely wrote:
>In C++17 a function can return a prvalue of a type that cannot be moved
>or copied. The current implementation of std::is_invocable_r uses
>std::is_convertible to test the conversion to R required by INVOKE<R>.
>That fails for non-copyable prvalues, because std::is_convertible is
>defined in terms of std::declval which uses std::add_rvalue_reference.
>In C++17 conversion from R to R involves no copies and so is not the
>same as conversion from R&& to R.
>
>This commit changes std::is_invocable_r to check the conversion without
>using std::is_convertible.
>
>std::function also contains a similar check using std::is_convertible,
>which can be fixed by simply reusing std::is_invocable_r (but because
>std::is_invocable_r is not defined for C++11 it uses the underlying
>std::__is_invocable_impl trait directly).
>
>	PR libstdc++/91456
>	* include/bits/std_function.h (__check_func_return_type): Remove.
>	(function::_Callable): Use std::__is_invocable_impl instead of
>	__check_func_return_type.
>	* include/std/type_traits (__is_invocable_impl): Add another defaulted
>	template parameter. Define a separate partial specialization for
>	INVOKE and INVOKE<void>. For INVOKE<R> replace is_convertible check
>	with a check that models delayed temporary materialization.
>	* testsuite/20_util/function/91456.cc: New test.
>	* testsuite/20_util/is_invocable/91456.cc: New test.

With some minor changes to __is_convertible_helper we could make that
usable by both std::is_convertible and __is_invokable_impl.

I don't plan to commit this now but might do at a later date.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 4166 bytes --]

diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
index 44db2cade5d..4df3fee4c77 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -1491,20 +1491,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
            bool = __or_<is_void<_From>, is_function<_To>,
                         is_array<_To>>::value>
     struct __is_convertible_helper
-    {
-      typedef typename is_void<_To>::type type;
-    };
+    : public is_void<_To>::type
+    { };
 
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wctor-dtor-privacy"
   template<typename _From, typename _To>
     class __is_convertible_helper<_From, _To, false>
     {
+      // Unlike declval, this doesn't add_rvalue_reference.
+      template<typename _From1>
+	static _From1 __declval();
+
       template<typename _To1>
 	static void __test_aux(_To1) noexcept;
 
       template<typename _From1, typename _To1,
-	       typename = decltype(__test_aux<_To1>(std::declval<_From1>()))>
+	       typename = decltype(__test_aux<_To1>(__declval<_From1>()))>
 	static true_type
 	__test(int);
 
@@ -1513,14 +1516,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	__test(...);
 
     public:
-      typedef decltype(__test<_From, _To>(0)) type;
+      using type = decltype(__test<_From, _To>(0));
     };
 #pragma GCC diagnostic pop
 
+  template<typename _Tp> struct add_rvalue_reference;
+
   /// is_convertible
   template<typename _From, typename _To>
     struct is_convertible
-    : public __is_convertible_helper<_From, _To>::type
+    : public __is_convertible_helper<typename add_rvalue_reference<_From>::type,
+				     _To>::type
     { };
 
   template<typename _From, typename _To,
@@ -1535,12 +1541,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template<typename _From, typename _To>
     class __is_nt_convertible_helper<_From, _To, false>
     {
+      // Unlike declval, this doesn't add_rvalue_reference.
+      template<typename _From1>
+	static _From1 __declval();
+
       template<typename _To1>
 	static void __test_aux(_To1) noexcept;
 
       template<typename _From1, typename _To1>
 	static
-	__bool_constant<noexcept(__test_aux<_To1>(std::declval<_From1>()))>
+	__bool_constant<noexcept(__test_aux<_To1>(__declval<_From1>()))>
 	__test(int);
 
       template<typename, typename>
@@ -1555,14 +1565,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // is_nothrow_convertible for C++11
   template<typename _From, typename _To>
     struct __is_nothrow_convertible
-    : public __is_nt_convertible_helper<_From, _To>::type
+    : __is_nt_convertible_helper<typename add_rvalue_reference<_From>::type,
+				 _To>::type
     { };
 
 #if __cplusplus > 201703L
   /// is_nothrow_convertible
   template<typename _From, typename _To>
     struct is_nothrow_convertible
-    : public __is_nt_convertible_helper<_From, _To>::type
+    : __is_nt_convertible_helper<typename add_rvalue_reference<_From>::type,
+				 _To>::type
     { };
 
   /// is_nothrow_convertible_v
@@ -2896,35 +2908,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     : true_type
     { };
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wctor-dtor-privacy"
   // Used for INVOKE<R> expressions to check the implicit conversion to R.
   template<typename _Result, typename _Ret>
     struct __is_invocable_impl<_Result, _Ret,
 			       /* is_void<_Ret> = */ false,
 			       __void_t<typename _Result::type>>
-    {
-    private:
-      // The type of the INVOKE expression.
-      // Unlike declval, this doesn't add_rvalue_reference.
-      static typename _Result::type _S_get();
-
-      template<typename _Tp>
-	static void _S_conv(_Tp);
-
-      // This overload is viable if INVOKE(f, args...) can convert to _Tp.
-      template<typename _Tp, typename = decltype(_S_conv<_Tp>(_S_get()))>
-	static true_type
-	_S_test(int);
-
-      template<typename _Tp>
-	static false_type
-	_S_test(...);
-
-    public:
-      using type = decltype(_S_test<_Ret>(1));
-    };
-#pragma GCC diagnostic pop
+    : __is_convertible_helper<typename _Result::type, _Ret>
+    { };
 
   template<typename _Fn, typename... _ArgTypes>
     struct __is_invocable