On Tue, Mar 11, 2025 at 10:22 PM Jonathan Wakely <jwak...@redhat.com> wrote:
> LWG 4154 (approved in Wrocław, November 2024) fixed the Mandates:
> precondition for std::packaged_task::packaged_task(F&&) to match what
> the implementation actually requires. We already gave a diagnostic in
> the right cases as required by the issue resolution, so strictly
> speaking we don't need to do anything. But the current diagnostic comes
> from inside the implementation of std::__invoke_r and could be more
> user-friendly.
>
> For C++17 (when std::is_invocable_r_v is available) add a static_assert
> to the constructor, so the error is clear:
>
> .../include/c++/15.0.1/future: In instantiation of
> 'std::packaged_task<_Res(_ArgTypes ...)>::packaged_task(_Fn&&) [with _Fn =
> const F&; <template-parameter-2-2> = void; _Res = void; _ArgTypes = {}]':
> lwg4154_neg.cc:15:31: required from here
> 15 | std::packaged_task<void()> p(f); // { dg-error "here" "" { target
> c++17 } }
> | ^
> .../include/c++/15.0.1/future:1575:25: error: static assertion failed
> 1575 | static_assert(is_invocable_r_v<_Res, decay_t<_Fn>&,
> _ArgTypes...>);
> |
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Also add a test to confirm we get a diagnostic as the standard requires.
>
> libstdc++-v3/ChangeLog:
>
> * include/std/future (packaged_task::packaged_task(F&&)): Add
> static_assert.
> * testsuite/30_threads/packaged_task/cons/dangling_ref.cc: Add
> dg-error for new static assertion.
> * testsuite/30_threads/packaged_task/cons/lwg4154_neg.cc: New
> test.
> ---
>
> Tested x86_64-linux.
>
> libstdc++-v3/include/std/future | 9 ++++-
> .../packaged_task/cons/dangling_ref.cc | 1 +
> .../packaged_task/cons/lwg4154_neg.cc | 36 +++++++++++++++++++
> 3 files changed, 45 insertions(+), 1 deletion(-)
> create mode 100644
> libstdc++-v3/testsuite/30_threads/packaged_task/cons/lwg4154_neg.cc
>
> diff --git a/libstdc++-v3/include/std/future
> b/libstdc++-v3/include/std/future
> index 2a855f262a0..b7ab233b85f 100644
> --- a/libstdc++-v3/include/std/future
> +++ b/libstdc++-v3/include/std/future
> @@ -1567,7 +1567,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 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
> + }
>
> #if __cplusplus < 201703L
> // _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 225b65fe6a7..51c6ade91c3 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
> @@ -10,3 +10,4 @@ std::packaged_task<const int&()> task(f);
> // { 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 }
> 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
> new file mode 100644
> index 00000000000..57472a3d798
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/30_threads/packaged_task/cons/lwg4154_neg.cc
> @@ -0,0 +1,36 @@
> +// { dg-do compile { target c++11 } }
> +
> +// LWG 4154. The Mandates for std::packaged_task's constructor from
> +// a callable entity should consider decaying
> +
> +#include <future>
> +
> +struct F {
> + void operator()() & = delete;
> + void operator()() const & { }
> +};
> +
> +// 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" }
>
Could you add a test for functor that is only rvalue-callable (only
operator() &&).
There was discussion about this case on reflector, so I think it would be
good to document that they are not supported.
> +
> +namespace good
> +{
> + struct F {
> + void operator()() const & = delete;
> + void operator()() & { }
> + };
> +
> + // In C++11/14/17/20 std::packaged_task::packaged_task<F>(F&&)
> incorrectly
> + // required that the callable passed to the constructor can be invoked.
> + // If the type of the parameter is const F& we might not be able to
> invoke
> + // the parameter, but that's OK because we store and invoke a non-const
> F.
> + // So this should work without errors:
> + const F f;
> + std::packaged_task<void()> p(f);
> +}
> +
> +
> --
> 2.48.1
>
>
