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.
---

Tested x86_64-linux.

 libstdc++-v3/include/std/future               | 66 +++++++++----------
 .../packaged_task/cons/dangling_ref.cc        |  3 +-
 .../packaged_task/cons/lwg4154_neg.cc         | 10 +--
 3 files changed, 38 insertions(+), 41 deletions(-)

diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future
index b7ab233b85f..080690064a9 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 51c6ade91c3..8cc3f78da9f 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 6ba1bb1a53e..b3413c2d11a 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 {
-- 
2.49.0

Reply via email to