[Bug libstdc++/100612] New: std::jthread can't be initialized with a pointer to a member function taking a std::stop_token

[Bug libstdc++/100612] New: std::jthread can't be initialized with a pointer to a member function taking a std::stop_token

[jonathan.oconnor at protonmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100612

            Bug ID: 100612
           Summary: std::jthread can't be initialized with a pointer to a
                    member function taking a std::stop_token
           Product: gcc
           Version: 10.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jonathan.oconnor at protonmail dot com
  Target Milestone: ---

Created attachment 50817
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50817&action=edit
Sample source code showing error

[Bug libstdc++/100612] std::jthread can't be initialized with a pointer to a member function taking a std::stop_token

[jonathan.oconnor at protonmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100612

--- Comment #1 from Jonathan O'Connor <jonathan.oconnor at protonmail dot com> ---
The std::jthread constructor does not support taking a pointer to a member
function that has, as a first argument, a std::stop_token.

In C++20, the new jthread class can accept a std::stop_token to aid in stopping
a thread cooperatively. Unfortunately, the library code in gcc 10.2.0 and all
later versions (according to godbolt.org) do not allow a member function taking
a stop_token to be used to initialize a jthread.

The problem is due to the std::jthread::_S_create() method which can't handle
my desired case.

In a private email discussion with Nico Josuttis, his opinion was this should
be allowed by the standard:
  "well, my rough understanding is that as INVOKE is called
  (see 20.9.2 Requirements [func.require] in the Standard),
  you can always pass a member function with the object to call the member
  function for as next argument.
  That should work for both thread and jthread."

The fix should be relatively simple, involving a further if constexpr check for
the Callable being a member function. I'll try and make a patch, and append it
here.

#include <thread>

struct ThreadObj {
    void withoutStopToken() {}
    void withStopToken(std::stop_token st) {}
};

int main() {
    ThreadObj obj;
    // The following line causes an error. The other example compile fine.
    std::jthread t1{&ThreadObj::withStopToken, &obj};
    std::jthread t2{&ThreadObj::withoutStopToken, &obj};
    return 0;
}

[Bug libstdc++/100612] std::jthread can't be initialized with a pointer to a member function taking a std::stop_token

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100612

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I don't think this is valid.

The requirement for the constructor is is_­invocable_­v<decay_­t<F>,
stop_­token, decay_­t<Args>...> which means the stop_token is passed to INVOKE
before the object, so it calls INVOKE(f, token, &obj) which isn't valid.

[Bug libstdc++/100612] std::jthread can't be initialized with a pointer to a member function taking a std::stop_token

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100612

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The standard could support this by saying that if the function object is a
pointer to member function then the requirement is
is_­invocable_­v<decay_­t<F>, Arg0, stop_­token, decay_­t<ArgsN>...> where Are
is the first argument in the pack, and ArgsN is the rest of the pack. But
that's not what the standard says.

[Bug libstdc++/100612] std::jthread can't be initialized with a pointer to a member function taking a std::stop_token

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100612

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #3)
> Are is the first argument in the pack,


Sorry, that was meant to say Arg0 not Are

[Bug libstdc++/100612] std::jthread can't be initialized with a pointer to a member function taking a std::stop_token

[jonathan.oconnor at protonmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100612

--- Comment #5 from Jonathan O'Connor <jonathan.oconnor at protonmail dot com> ---
I was afraid you were going to say it's not a bug :-) That's why I reached out
to Nico, who was on the committee, and was one of the people who proposed
jthread.

My view, as a user, is that jthread should be a drop in replacement for thread,
with auto-joining and stop_token support.

I've just reread the descripton of jthread's constructor here:
https://en.cppreference.com/w/cpp/thread/jthread/jthread
and it does seem to explicitly exclude the behaviour I would like.

So, I have no problem with you closing this as not a bug.
Sorry for wasting your time. I guess, I'll have to write a committee proposal.

BTW, thanks for your all your work on g++ and the stdlib++. Much appreciated.

[Bug libstdc++/100612] std::jthread can't be initialized with a pointer to a member function taking a std::stop_token

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100612

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED
           Severity|normal                      |enhancement

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan O'Connor from comment #5)
> I was afraid you were going to say it's not a bug :-) That's why I reached
> out to Nico, who was on the committee, and was one of the people who
> proposed jthread.

Yes, I know, I was there when we reviewed it and voted it into the standard :-)

> My view, as a user, is that jthread should be a drop in replacement for
> thread, with auto-joining and stop_token support.

And it is. But "stop_token support" doesn't mean "can drop a stop_token at any
arbitrary position in the arguments list and expect it to work". For it to work
with a pointer-to-member we need to change the specification.


> I've just reread the descripton of jthread's constructor here:
> https://en.cppreference.com/w/cpp/thread/jthread/jthread
> and it does seem to explicitly exclude the behaviour I would like.

Right.

> So, I have no problem with you closing this as not a bug.
> Sorry for wasting your time. I guess, I'll have to write a committee
> proposal.

Please do.

> BTW, thanks for your all your work on g++ and the stdlib++. Much appreciated.

It's libstdc++ ;-)

FWIW this patch makes your code work, and I think this would make a reasonable
non-standard extension:

diff --git a/libstdc++-v3/include/std/thread b/libstdc++-v3/include/std/thread
index 886994c1320..ee4fec4a50c 100644
--- a/libstdc++-v3/include/std/thread
+++ b/libstdc++-v3/include/std/thread
@@ -99,6 +99,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION

 #ifdef __cpp_lib_jthread

+#ifndef __STRICT_ANSI__
+    template<typename _Callable, typename... _Args>
+      constexpr bool __pmf_callable_with_stop_token = false;
+
+    template<typename _Callable, typename _Obj, typename... _Args>
+      constexpr bool __pmf_callable_with_stop_token<_Callable, _Obj, _Args...>
+       = __and_<is_member_function_pointer<remove_reference_t<_Callable>>,
+                is_invocable<_Callable, _Obj, stop_token, _Args...>>::value;
+#endif
+
   /// A thread that can be requested to stop and automatically joined.
   class jthread
   {
@@ -211,6 +221,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       static thread
       _S_create(stop_source& __ssrc, _Callable&& __f, _Args&&... __args)
       {
+#ifndef __STRICT_ANSI__
+       if constexpr (__pmf_callable_with_stop_token<_Callable, _Args...>)
+         return _S_create2(__ssrc, std::forward<_Callable>(__f),
+                           std::forward<_Args>(__args)...);
+       else
+#endif
        if constexpr(is_invocable_v<decay_t<_Callable>, stop_token,
                                    decay_t<_Args>...>)
          return thread{std::forward<_Callable>(__f), __ssrc.get_token(),
@@ -226,6 +242,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
          }
       }

+#ifndef __STRICT_ANSI__
+    template<typename _Callable, typename _Obj, typename... _Args>
+      static thread
+      _S_create2(stop_source& __ssrc, _Callable&& __f, _Obj&& __obj,
+                _Args&&... __args)
+      {
+       return thread{std::forward<_Callable>(__f), std::forward<_Obj>(__obj),
+                     __ssrc.get_token(), std::forward<_Args>(__args)...};
+      }
+#endif
+
     stop_source _M_stop_source;
     thread _M_thread;
   };

I think I'll make this change in my own GCC fork, but not the gcc.gnu.org
version.

[Bug libstdc++/100612] std::jthread can't be initialized with a pointer to a member function taking a std::stop_token

[jonathan.oconnor at protonmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100612

--- Comment #7 from Jonathan O'Connor <jonathan.oconnor at protonmail dot com> ---
libstdc++. Woops, apologies!

I feel somewhat vindicated by your non __STRICT_ANSI__ change!
I'll now go away and write the proposal. Thanks for the encouragement.

[Bug libstdc++/100612] std::jthread can't be initialized with a pointer to a member function taking a std::stop_token

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100612

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
N.B. in C++20 you can call your member function using a lambda:

std::jthread t1{[&obj] (std::stop_token st) {
   obj.withStopToken(std::move(st)); }
};


Or generically, with perfect forwarding:

std::jthread t1{[&obj] (auto&&... args) {
   obj.withStopToken(std::forward<decltype(args)>(args)...); }
};

[Bug libstdc++/100612] std::jthread can't be initialized with a pointer to a member function taking a std::stop_token

[jonathan.oconnor at protonmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100612

--- Comment #9 from Jonathan O'Connor <jonathan.oconnor at protonmail dot com> ---
I'd already figured out the workaround using a static member function calling
the non-static member function. But the lambda version is nicer!

[Bug libstdc++/100612] std::jthread can't be initialized with a pointer to a member function taking a std::stop_token

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100612

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:34e9407b3b4298bd587e0df2e0047679019b66cf

commit r12-4081-g34e9407b3b4298bd587e0df2e0047679019b66cf
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu May 20 22:36:16 2021 +0100

    libstdc++: Make std::jthread support pointers to member functions [PR
100612]

    This adds a non-standard extension to support initializing a
    std::jthread with a pointer to a member function that expects a
    stop_token to be added to the arguments. That use case is not supported
    by C++20, because the stop_token would get added as the first argument,
    which is where the object argument needs to be to invoke a pointer to
    member function.

    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

    libstdc++-v3/ChangeLog:

            PR libstdc++/100612
            * include/std/thread (__pmf_expects_stop_token): New variable
            template to detect a pointer to member function that needs a
            stop_token to be added to the arguments.
            (jthread::__S_create): Use __pmf_expects_stop_token.
            (jthread::__S_create_pmf): New function.
            * testsuite/30_threads/jthread/100612.cc: New test.

[Bug libstdc++/100612] std::jthread can't be initialized with a pointer to a member function taking a std::stop_token

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100612

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Implemented as an extension for GCC 12.