libstdc++: Improve diagnostics for std::packaged_task invocable checks

Moving the static_assert that checks is_invocable_r_v into _Task_state
means it is checked when we instantiate that class template.

Replacing the __create_task_state function with a static member function
_Task_state::_S_create ensures we instantiate _Task_state and trigger
the static_assert immediately, not deep inside the implementation of
std::allocate_shared. This results in shorter diagnostics that don't
show deeply-nested template instantiations before the static_assert
failure.

Placing the static_assert at class scope also helps us to fail earlier
than waiting until when the _Task_state::_M_run virtual function is
instantiated. That also makes the diagnostics shorter and easier to read
(although for C++11 and C++14 modes the class-scope static_assert
doesn't check is_invocable_r, so dangling references aren't detected
until _M_run is instantiated).

libstdc++-v3/ChangeLog:

	* include/std/future (__future_base::_Task_state): Check
	invocable requirement here.
	(__future_base::_Task_state::_S_create): New static member
	function.
	(__future_base::_Task_state::_M_reset): Use _S_create.
	(__create_task_state): Remove.
	(packaged_task): Use _Task_state::_S_create instead of
	__create_task_state.
	* testsuite/30_threads/packaged_task/cons/dangling_ref.cc:
	Adjust dg-error patterns.
	* testsuite/30_threads/packaged_task/cons/lwg4154_neg.cc:
	Likewise.

Reviewed-by: Tomasz Kamiński <tkaminsk@redhat.com>

3 files changed, 38 insertions, 41 deletions

diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future
index b7ab233..0806900 100644
--- a/libstdc++-v3/include/std/future
+++ b/libstdc++-v3/include/std/future
@@ -1486,12 +1486,28 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     struct __future_base::_Task_state<_Fn, _Alloc, _Res(_Args...)> final
     : __future_base::_Task_state_base<_Res(_Args...)>
     {
+#ifdef __cpp_lib_is_invocable // C++ >= 17
+      static_assert(is_invocable_r_v<_Res, _Fn&, _Args...>);
+#else
+      static_assert(__is_invocable<_Fn&, _Args...>::value,
+		    "_Fn& is invocable with _Args...");
+#endif
+
       template<typename _Fn2>
 	_Task_state(_Fn2&& __fn, const _Alloc& __a)
 	: _Task_state_base<_Res(_Args...)>(__a),
 	  _M_impl(std::forward<_Fn2>(__fn), __a)
 	{ }
 
+      template<typename _Fn2>
+	static shared_ptr<_Task_state_base<_Res(_Args...)>>
+	_S_create(_Fn2&& __fn, const _Alloc& __a)
+	{
+	  return std::allocate_shared<_Task_state>(__a,
+						   std::forward<_Fn2>(__fn),
+						   __a);
+	}
+
     private:
       virtual void
       _M_run(_Args&&... __args)
@@ -1515,7 +1531,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }
 
       virtual shared_ptr<_Task_state_base<_Res(_Args...)>>
-      _M_reset();
+      _M_reset()
+      { return _S_create(std::move(_M_impl._M_fn), _M_impl); }
 
       struct _Impl : _Alloc
       {
@@ -1525,38 +1542,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	_Fn _M_fn;
       } _M_impl;
     };
-
-  template<typename _Signature, typename _Fn,
-	   typename _Alloc = std::allocator<int>>
-    shared_ptr<__future_base::_Task_state_base<_Signature>>
-    __create_task_state(_Fn&& __fn, const _Alloc& __a = _Alloc())
-    {
-      typedef typename decay<_Fn>::type _Fn2;
-      typedef __future_base::_Task_state<_Fn2, _Alloc, _Signature> _State;
-      return std::allocate_shared<_State>(__a, std::forward<_Fn>(__fn), __a);
-    }
-
-  template<typename _Fn, typename _Alloc, typename _Res, typename... _Args>
-    shared_ptr<__future_base::_Task_state_base<_Res(_Args...)>>
-    __future_base::_Task_state<_Fn, _Alloc, _Res(_Args...)>::_M_reset()
-    {
-      return __create_task_state<_Res(_Args...)>(std::move(_M_impl._M_fn),
-						 static_cast<_Alloc&>(_M_impl));
-    }
   /// @endcond
 
   /// packaged_task
   template<typename _Res, typename... _ArgTypes>
     class packaged_task<_Res(_ArgTypes...)>
     {
-      typedef __future_base::_Task_state_base<_Res(_ArgTypes...)> _State_type;
+      using _State_type = __future_base::_Task_state_base<_Res(_ArgTypes...)>;
       shared_ptr<_State_type>                   _M_state;
 
       // _GLIBCXX_RESOLVE_LIB_DEFECTS
       // 3039. Unnecessary decay in thread and packaged_task
       template<typename _Fn, typename _Fn2 = __remove_cvref_t<_Fn>>
-	using __not_same
-	  = typename enable_if<!is_same<packaged_task, _Fn2>::value>::type;
+	using __not_same = __enable_if_t<!is_same<packaged_task, _Fn2>::value>;
+
+      // _GLIBCXX_RESOLVE_LIB_DEFECTS
+      // 4154. The Mandates for std::packaged_task's constructor
+      // from a callable entity should consider decaying.
+      template<typename _Fn, typename _Alloc = std::allocator<int>>
+	using _Task_state
+	  = __future_base::_Task_state<__decay_t<_Fn>, _Alloc,
+				       _Res(_ArgTypes...)>;
 
     public:
       // Construction and destruction
@@ -1565,16 +1571,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       template<typename _Fn, typename = __not_same<_Fn>>
 	explicit
 	packaged_task(_Fn&& __fn)
-	: _M_state(
-	    __create_task_state<_Res(_ArgTypes...)>(std::forward<_Fn>(__fn)))
-	{
-#ifdef __cpp_lib_is_invocable // C++ >= 17
-	  // _GLIBCXX_RESOLVE_LIB_DEFECTS
-	  // 4154. The Mandates for std::packaged_task's constructor
-	  // from a callable entity should consider decaying
-	  static_assert(is_invocable_r_v<_Res, decay_t<_Fn>&, _ArgTypes...>);
-#endif
-	}
+	: _M_state(_Task_state<_Fn>::_S_create(std::forward<_Fn>(__fn), {}))
+	{ }
 
 #if __cplusplus < 201703L
       // _GLIBCXX_RESOLVE_LIB_DEFECTS
@@ -1583,8 +1581,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       // 2921. packaged_task and type-erased allocators
       template<typename _Fn, typename _Alloc, typename = __not_same<_Fn>>
 	packaged_task(allocator_arg_t, const _Alloc& __a, _Fn&& __fn)
-	: _M_state(__create_task_state<_Res(_ArgTypes...)>(
-		   std::forward<_Fn>(__fn), __a))
+	: _M_state(_Task_state<_Fn, _Alloc>::_S_create(std::forward<_Fn>(__fn),
+						       __a))
 	{ }
 
       // _GLIBCXX_RESOLVE_LIB_DEFECTS
diff --git a/libstdc++-v3/testsuite/30_threads/packaged_task/cons/dangling_ref.cc b/libstdc++-v3/testsuite/30_threads/packaged_task/cons/dangling_ref.cc
index 51c6ade..8cc3f78 100644
--- a/libstdc++-v3/testsuite/30_threads/packaged_task/cons/dangling_ref.cc
+++ b/libstdc++-v3/testsuite/30_threads/packaged_task/cons/dangling_ref.cc
@@ -8,6 +8,5 @@ int f();
 std::packaged_task<const int&()> task(f);
 // { dg-error "dangling reference" "" { target { c++14_down } } 0 }
 // { dg-error "reference to temporary" "" { target { c++14_down } } 0 }
-// { dg-error "no matching function" "" { target c++17 } 0 }
-// { dg-error "enable_if" "" { target c++17 } 0 }
 // { dg-error "static assertion failed" "" { target c++17 } 0 }
+// { dg-error "note: .*std::is_invocable_r" "" { target c++17 } 0 }
diff --git a/libstdc++-v3/testsuite/30_threads/packaged_task/cons/lwg4154_neg.cc b/libstdc++-v3/testsuite/30_threads/packaged_task/cons/lwg4154_neg.cc
index 6ba1bb1..b3413c2 100644
--- a/libstdc++-v3/testsuite/30_threads/packaged_task/cons/lwg4154_neg.cc
+++ b/libstdc++-v3/testsuite/30_threads/packaged_task/cons/lwg4154_neg.cc
@@ -12,16 +12,16 @@ struct F {
 
 // Mandates: is_invocable_r_v<R, decay_t<F>&, ArgTypes...> is true.
 const F f;
-std::packaged_task<void()> p(f); // { dg-error "here" "" { target c++17 } }
-// { dg-error "static assertion failed" "" { target c++17 } 0 }
-// { dg-error "invoke_r" "" { target *-*-* } 0 }
-// { dg-prune-output "enable_if<false" }
+std::packaged_task<void()> p(f); // { dg-error "here" }
+// { dg-error "static assertion failed" "" { target *-*-* } 0 }
+// { dg-error "note: .*std::is_invocable_r_v<void, " "" { target c++17 } 0 }
 
 // Only callable as rvalue
 struct Frv {
   int* operator()() && { return 0; }
 };
-std::packaged_task<int*()> p2(Frv{}); // { dg-error "here" "" { target c++17 } }
+std::packaged_task<int*()> p2(Frv{}); // { dg-error "here" }
+// { dg-error "note: .*std::is_invocable_r_v<int., " "" { target c++17 } 0 }
 
 // Only callable as non-const lvalue
 struct Fnc {