https://gcc.gnu.org/g:901900bc37566c59b4eb62c1427f3150b800d8a0
commit r16-132-g901900bc37566c59b4eb62c1427f3150b800d8a0
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Thu Apr 24 21:55:16 2025 +0100
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 <tkami...@redhat.com>
Diff:
---
libstdc++-v3/include/std/future | 66 +++++++++++-----------
.../30_threads/packaged_task/cons/dangling_ref.cc | 3 +-
.../30_threads/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 b7ab233b85f0..080690064a91 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 51c6ade91c36..8cc3f78da9f0 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 6ba1bb1a53e9..b3413c2d11ac 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 {
