[PATCH] Add support for C++2a stop_token

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Add support for C++2a stop_token

Thomas Rodgers

0001-Add-support-for-C-2a-stop_token.patch (24K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add support for C++2a stop_token

Jonathan Wakely-3
On 23/10/19 12:50 -0700, Thomas Rodgers wrote:
>
>Thomas Rodgers writes:
>
>Let's try this again.

That's better, thanks :-)

>+ * include/Makefile.am: Add <stop_token> header.
>+        * include/Makefile.in: Regenerate.
>+ * include/std/stop_token: New file.
>+ * include/std/version (__cpp_lib_jthread): New value.
>+ * testsuite/30_threads/stop_token/1.cc: New test.
>+ * testsuite/30_threads/stop_token/2.cc: New test.
>+ * testsuite/30_threads/stop_token/stop_token.cc: New test.

ChangeLog entries should be provided separately from the patch (e.g.
inline in the makefile, or as a separte attachment) because they never
apply cleanly.

Also it looks like you have a mixture of tabs and space there, it
should be only tabs.

>diff --git a/libstdc++-v3/include/Makefile.in b/libstdc++-v3/include/Makefile.in
>index cd1e9df5482..9b4ab670315 100644
>--- a/libstdc++-v3/include/Makefile.in
>+++ b/libstdc++-v3/include/Makefile.in
>@@ -416,6 +416,7 @@ std_headers = \
> ${std_srcdir}/sstream \
> ${std_srcdir}/stack \
> ${std_srcdir}/stdexcept \
>+ ${std_srcdir}/stop_token \
> ${std_srcdir}/streambuf \
> ${std_srcdir}/string \
> ${std_srcdir}/string_view \

Generated files like Makefile.in don't need to be in the patch. I use
a git alias called 'filter' to filter them out:

!f(){ git $@ | filterdiff -x '*/ChangeLog' -x '*/Makefile.in' -x '*/configure' -x '*/config.h.in' -x '*/doc/html/*' | sed -e '/^diff.*ChangeLog/{N;d}' ; }; f

And then I create a patch with this alias:

!f(){ git filter show --diff-algorithm=histogram ${*:-HEAD} > /dev/shm/patch.txt; }; f


>diff --git a/libstdc++-v3/include/std/stop_token b/libstdc++-v3/include/std/stop_token
>new file mode 100644
>index 00000000000..b3655b85eae
>--- /dev/null
>+++ b/libstdc++-v3/include/std/stop_token
>@@ -0,0 +1,338 @@
>+// <stop_token> -*- C++ -*-
>+
>+// Copyright (C) 2008-2019 Free Software Foundation, Inc.

The copyright date should be just 2019.

>+//
>+// This file is part of the GNU ISO C++ Library.  This library is free
>+// software; you can redistribute it and/or modify it under the
>+// terms of the GNU General Public License as published by the
>+// Free Software Foundation; either version 3, or (at your option)
>+// any later version.
>+
>+// This library is distributed in the hope that it will be useful,
>+// but WITHOUT ANY WARRANTY; without even the implied warranty of
>+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+// GNU General Public License for more details.
>+
>+// Under Section 7 of GPL version 3, you are granted additional
>+// permissions described in the GCC Runtime Library Exception, version
>+// 3.1, as published by the Free Software Foundation.
>+
>+// You should have received a copy of the GNU General Public License and
>+// a copy of the GCC Runtime Library Exception along with this program;
>+// see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>+// <http://www.gnu.org/licenses/>.
>+
>+/** @file include/stop_token
>+ *  This is a Standard C++ Library header.
>+ */
>+
>+#ifndef _GLIBCXX_STOP_TOKEN
>+#define _GLIBCXX_STOP_TOKEN
>+

Please add:

#if __cplusplus > 201703L

here (and the corresponding #endif before the final #endif) so that
this file is empty when included pre-C++20.

>+#include <type_traits>
>+#include <memory>
>+#include <mutex>
>+#include <atomic>
>+
>+#define __cpp_lib_jthread 201907L
>+
>+namespace std _GLIBCXX_VISIBILITY(default)
>+{
>+  _GLIBCXX_BEGIN_NAMESPACE_VERSION

These BEGIN/END macros should not be indented (which has to be done
manually as editors always want to indent the content following the
opening brace of the namespace).

>+
>+  class stop_source;
>+  template<class _Callback>

s/class/typename/

>+  class stop_callback;
>+
>+  struct nostopstate_t { explicit nostopstate_t() = default; };
>+  inline constexpr nostopstate_t nostopstate();
>+
>+  class stop_token {

The opening brace should be on the next line please, in the same
column as "class".
(Same comment for all classes and structs in the patch).


>+  public:
>+    stop_token() noexcept = default;
>+
>+    stop_token(const stop_token& __other) noexcept
>+      : _M_state(__other._M_state)
>+    { }
>+
>+    stop_token(stop_token&& __other) noexcept
>+      : _M_state(std::move(__other._M_state))
>+    { }
>+
>+    ~stop_token() = default;
>+
>+    stop_token&
>+    operator=(const stop_token& __rhs) noexcept {
>+      _M_state = __rhs._M_state;
>+      return *this;
>+    }
>+
>+    stop_token&
>+    operator=(stop_token&& __rhs) noexcept {
>+      std::swap(_M_state, __rhs._M_state);
>+      return *this;
>+    }

This doesn't leave the RHS empty as required.

I think the copy/move constructors and copy/move assignment operators
can all be defined as = default and that will do the right thing.

>+
>+    [[nodiscard]]
>+    bool
>+    stop_possible() const noexcept
>+    {
>+      return static_cast<bool>(_M_state);
>+    }
>+
>+    [[nodiscard]]
>+    bool
>+    stop_requested() const noexcept
>+    {
>+      return stop_possible() && _M_state->_M_stop_requested();
>+    }
>+
>+  private:
>+    friend stop_source;
>+    template<typename _Callback>
>+    friend class stop_callback;
>+
>+    struct _Stop_cb {
>+      void(*_M_callback)(_Stop_cb*);
>+      _Stop_cb* _M_prev = nullptr;
>+      _Stop_cb* _M_next = nullptr;
>+
>+      template<typename _CB>

Please Use _Cb so it's not all uppercase (which is more likely to
conflict with libc macros).

>+      _Stop_cb(_CB&& __cb)
>+        : _M_callback(std::move(__cb))
>+      { }
>+
>+      static void
>+      _S_execute(_Stop_cb* __cb) noexcept
>+      {
>+        __cb->_M_callback(__cb);
>+      }
>+    };
>+
>+    struct _Stop_state_t {
>+      std::atomic<bool> _M_stopped;
>+      std::mutex _M_mtx;
>+      _Stop_cb* _M_head = nullptr;
>+
>+      _Stop_state_t()
>+        : _M_stopped{false}
>+      { }
>+
>+      bool
>+      _M_stop_requested()
>+      {
>+        return _M_stopped;
>+      }
>+
>+      bool
>+      _M_request_stop()
>+      {
>+        bool __stopped = false;
>+        if (_M_stopped.compare_exchange_strong(__stopped, true))
>+          {
>+            std::unique_lock<std::mutex> __lck{_M_mtx};
>+            while (auto __p = _M_head)
>+              {
>+                _M_head = __p->_M_next;
>+                _Stop_cb::_S_execute(__p);
>+              }
>+            return true;
>+          }
>+        return false;
>+      }
>+
>+      bool
>+      _M_register_callback(_Stop_cb* __cb)
>+      {
>+        std::unique_lock<std::mutex> __lck{_M_mtx};
>+        if (_M_stopped)
>+          return false;
>+
>+        __cb->_M_next = _M_head;
>+        _M_head->_M_prev = __cb;
>+        _M_head = __cb;
>+        return true;
>+      }
>+
>+      void
>+      _M_remove_callback(_Stop_cb* __cb)
>+      {
>+        std::unique_lock<std::mutex> __lck{_M_mtx};
>+        if (__cb == _M_head)
>+          {
>+            _M_head = _M_head->_M_next;
>+            _M_head->_M_prev = nullptr;
>+          }
>+        else
>+          {
>+            __cb->_M_prev->_M_next = __cb->_M_next;
>+            if (__cb->_M_next)
>+              {
>+                __cb->_M_next->_M_prev = __cb->_M_prev;
>+              }
>+          }
>+      }
>+    };
>+
>+    using _Stop_state = std::shared_ptr<_Stop_state_t>;
>+    _Stop_state _M_state;
>+
>+    explicit stop_token(_Stop_state __state)
>+      : _M_state{std::move(__state)}
>+    { }
>+  };
>+
>+  class stop_source {
>+    using _Stop_state_t = stop_token::_Stop_state_t;
>+    using _Stop_state = stop_token::_Stop_state;
>+
>+  public:
>+    stop_source()
>+      : _M_state(std::make_shared<_Stop_state_t>())
>+    { }
>+
>+    explicit stop_source(std::nostopstate_t) noexcept
>+    { }
>+
>+    stop_source(const stop_source& __other) noexcept
>+      : _M_state(__other._M_state)
>+    { }
>+
>+    stop_source(stop_source&& __other) noexcept
>+      : _M_state(std::move(__other._M_state))
>+    { }
>+
>+    stop_source&
>+    operator=(const stop_source& __rhs) noexcept
>+    {
>+      if (_M_state != __rhs._M_state)
>+        _M_state = __rhs._M_state;
>+      return *this;
>+    }
>+
>+    stop_source&
>+    operator=(stop_source&& __rhs) noexcept
>+    {
>+      std::swap(_M_state, __rhs._M_state);
>+      return *this;
>+    }

These can all be defaulted too.

>+    [[nodiscard]]
>+    bool
>+    stop_possible() const noexcept
>+    {
>+      return static_cast<bool>(_M_state);
>+    }
>+
>+    [[nodiscard]]
>+    bool
>+    stop_requested() const noexcept
>+    {
>+      return stop_possible() && _M_state->_M_stop_requested();
>+    }
>+
>+    bool
>+    request_stop() const noexcept
>+    {
>+      if (stop_possible())
>+        return _M_state->_M_request_stop();
>+      return false;
>+    }
>+
>+    [[nodiscard]]
>+    stop_token
>+    get_token() const noexcept
>+    {
>+      return stop_token{_M_state};
>+    }
>+
>+    void
>+    swap(stop_source& __other) noexcept
>+    {
>+      std::swap(_M_state, __other._M_state);
>+    }
>+
>+    [[nodiscard]]
>+    friend bool
>+    operator==(const stop_source& __a, const stop_source& __b) noexcept
>+    {
>+      return __a._M_state == __b._M_state;
>+    }
>+
>+    [[nodiscard]]
>+    friend bool
>+    operator!=(const stop_source& __a, const stop_source& __b) noexcept
>+    {
>+      return __a._M_state != __b._M_state;
>+    }
>+
>+  private:
>+    _Stop_state _M_state;
>+  };
>+
>+  template<typename _Callback>
>+    class [[nodiscard]] stop_callback
>+      : private stop_token::_Stop_cb {

Opening brace on the next line, in the same column as "class".

>+    using _Stop_cb = stop_token::_Stop_cb;

And then everything in the class body should be indented by another
two columns.

>+    using _Stop_state = stop_token::_Stop_state;
>+  public:
>+    using callback_type = _Callback;
>+
>+    template<typename _CB,

Change to _Cb here too.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add support for C++2a stop_token

Thomas Rodgers

The attached patch should be a complete implementation of C++20
stop_token, jthread, and changes to condition_variable_any which also
addresses the comments on the previous patch.


Jonathan Wakely writes:

> On 23/10/19 12:50 -0700, Thomas Rodgers wrote:
>>
>>Thomas Rodgers writes:
>>
>>Let's try this again.
>
> That's better, thanks :-)
>
>>+ * include/Makefile.am: Add <stop_token> header.
>>+        * include/Makefile.in: Regenerate.
>>+ * include/std/stop_token: New file.
>>+ * include/std/version (__cpp_lib_jthread): New value.
>>+ * testsuite/30_threads/stop_token/1.cc: New test.
>>+ * testsuite/30_threads/stop_token/2.cc: New test.
>>+ * testsuite/30_threads/stop_token/stop_token.cc: New test.
>
> ChangeLog entries should be provided separately from the patch (e.g.
> inline in the makefile, or as a separte attachment) because they never
> apply cleanly.
>
> Also it looks like you have a mixture of tabs and space there, it
> should be only tabs.
>
>>diff --git a/libstdc++-v3/include/Makefile.in b/libstdc++-v3/include/Makefile.in
>>index cd1e9df5482..9b4ab670315 100644
>>--- a/libstdc++-v3/include/Makefile.in
>>+++ b/libstdc++-v3/include/Makefile.in
>>@@ -416,6 +416,7 @@ std_headers = \
>> ${std_srcdir}/sstream \
>> ${std_srcdir}/stack \
>> ${std_srcdir}/stdexcept \
>>+ ${std_srcdir}/stop_token \
>> ${std_srcdir}/streambuf \
>> ${std_srcdir}/string \
>> ${std_srcdir}/string_view \
>
> Generated files like Makefile.in don't need to be in the patch. I use
> a git alias called 'filter' to filter them out:
>
> !f(){ git $@ | filterdiff -x '*/ChangeLog' -x '*/Makefile.in' -x '*/configure' -x '*/config.h.in' -x '*/doc/html/*' | sed -e '/^diff.*ChangeLog/{N;d}' ; }; f
>
> And then I create a patch with this alias:
>
> !f(){ git filter show --diff-algorithm=histogram ${*:-HEAD} > /dev/shm/patch.txt; }; f
>
>
>>diff --git a/libstdc++-v3/include/std/stop_token b/libstdc++-v3/include/std/stop_token
>>new file mode 100644
>>index 00000000000..b3655b85eae
>>--- /dev/null
>>+++ b/libstdc++-v3/include/std/stop_token
>>@@ -0,0 +1,338 @@
>>+// <stop_token> -*- C++ -*-
>>+
>>+// Copyright (C) 2008-2019 Free Software Foundation, Inc.
>
> The copyright date should be just 2019.
>
>>+//
>>+// This file is part of the GNU ISO C++ Library.  This library is free
>>+// software; you can redistribute it and/or modify it under the
>>+// terms of the GNU General Public License as published by the
>>+// Free Software Foundation; either version 3, or (at your option)
>>+// any later version.
>>+
>>+// This library is distributed in the hope that it will be useful,
>>+// but WITHOUT ANY WARRANTY; without even the implied warranty of
>>+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>+// GNU General Public License for more details.
>>+
>>+// Under Section 7 of GPL version 3, you are granted additional
>>+// permissions described in the GCC Runtime Library Exception, version
>>+// 3.1, as published by the Free Software Foundation.
>>+
>>+// You should have received a copy of the GNU General Public License and
>>+// a copy of the GCC Runtime Library Exception along with this program;
>>+// see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>>+// <http://www.gnu.org/licenses/>.
>>+
>>+/** @file include/stop_token
>>+ *  This is a Standard C++ Library header.
>>+ */
>>+
>>+#ifndef _GLIBCXX_STOP_TOKEN
>>+#define _GLIBCXX_STOP_TOKEN
>>+
>
> Please add:
>
> #if __cplusplus > 201703L
>
> here (and the corresponding #endif before the final #endif) so that
> this file is empty when included pre-C++20.
>
>>+#include <type_traits>
>>+#include <memory>
>>+#include <mutex>
>>+#include <atomic>
>>+
>>+#define __cpp_lib_jthread 201907L
>>+
>>+namespace std _GLIBCXX_VISIBILITY(default)
>>+{
>>+  _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> These BEGIN/END macros should not be indented (which has to be done
> manually as editors always want to indent the content following the
> opening brace of the namespace).
>
>>+
>>+  class stop_source;
>>+  template<class _Callback>
>
> s/class/typename/
>
>>+  class stop_callback;
>>+
>>+  struct nostopstate_t { explicit nostopstate_t() = default; };
>>+  inline constexpr nostopstate_t nostopstate();
>>+
>>+  class stop_token {
>
> The opening brace should be on the next line please, in the same
> column as "class".
> (Same comment for all classes and structs in the patch).
>
>
>>+  public:
>>+    stop_token() noexcept = default;
>>+
>>+    stop_token(const stop_token& __other) noexcept
>>+      : _M_state(__other._M_state)
>>+    { }
>>+
>>+    stop_token(stop_token&& __other) noexcept
>>+      : _M_state(std::move(__other._M_state))
>>+    { }
>>+
>>+    ~stop_token() = default;
>>+
>>+    stop_token&
>>+    operator=(const stop_token& __rhs) noexcept {
>>+      _M_state = __rhs._M_state;
>>+      return *this;
>>+    }
>>+
>>+    stop_token&
>>+    operator=(stop_token&& __rhs) noexcept {
>>+      std::swap(_M_state, __rhs._M_state);
>>+      return *this;
>>+    }
>
> This doesn't leave the RHS empty as required.
>
> I think the copy/move constructors and copy/move assignment operators
> can all be defined as = default and that will do the right thing.
>
>>+
>>+    [[nodiscard]]
>>+    bool
>>+    stop_possible() const noexcept
>>+    {
>>+      return static_cast<bool>(_M_state);
>>+    }
>>+
>>+    [[nodiscard]]
>>+    bool
>>+    stop_requested() const noexcept
>>+    {
>>+      return stop_possible() && _M_state->_M_stop_requested();
>>+    }
>>+
>>+  private:
>>+    friend stop_source;
>>+    template<typename _Callback>
>>+    friend class stop_callback;
>>+
>>+    struct _Stop_cb {
>>+      void(*_M_callback)(_Stop_cb*);
>>+      _Stop_cb* _M_prev = nullptr;
>>+      _Stop_cb* _M_next = nullptr;
>>+
>>+      template<typename _CB>
>
> Please Use _Cb so it's not all uppercase (which is more likely to
> conflict with libc macros).
>
>>+      _Stop_cb(_CB&& __cb)
>>+        : _M_callback(std::move(__cb))
>>+      { }
>>+
>>+      static void
>>+      _S_execute(_Stop_cb* __cb) noexcept
>>+      {
>>+        __cb->_M_callback(__cb);
>>+      }
>>+    };
>>+
>>+    struct _Stop_state_t {
>>+      std::atomic<bool> _M_stopped;
>>+      std::mutex _M_mtx;
>>+      _Stop_cb* _M_head = nullptr;
>>+
>>+      _Stop_state_t()
>>+        : _M_stopped{false}
>>+      { }
>>+
>>+      bool
>>+      _M_stop_requested()
>>+      {
>>+        return _M_stopped;
>>+      }
>>+
>>+      bool
>>+      _M_request_stop()
>>+      {
>>+        bool __stopped = false;
>>+        if (_M_stopped.compare_exchange_strong(__stopped, true))
>>+          {
>>+            std::unique_lock<std::mutex> __lck{_M_mtx};
>>+            while (auto __p = _M_head)
>>+              {
>>+                _M_head = __p->_M_next;
>>+                _Stop_cb::_S_execute(__p);
>>+              }
>>+            return true;
>>+          }
>>+        return false;
>>+      }
>>+
>>+      bool
>>+      _M_register_callback(_Stop_cb* __cb)
>>+      {
>>+        std::unique_lock<std::mutex> __lck{_M_mtx};
>>+        if (_M_stopped)
>>+          return false;
>>+
>>+        __cb->_M_next = _M_head;
>>+        _M_head->_M_prev = __cb;
>>+        _M_head = __cb;
>>+        return true;
>>+      }
>>+
>>+      void
>>+      _M_remove_callback(_Stop_cb* __cb)
>>+      {
>>+        std::unique_lock<std::mutex> __lck{_M_mtx};
>>+        if (__cb == _M_head)
>>+          {
>>+            _M_head = _M_head->_M_next;
>>+            _M_head->_M_prev = nullptr;
>>+          }
>>+        else
>>+          {
>>+            __cb->_M_prev->_M_next = __cb->_M_next;
>>+            if (__cb->_M_next)
>>+              {
>>+                __cb->_M_next->_M_prev = __cb->_M_prev;
>>+              }
>>+          }
>>+      }
>>+    };
>>+
>>+    using _Stop_state = std::shared_ptr<_Stop_state_t>;
>>+    _Stop_state _M_state;
>>+
>>+    explicit stop_token(_Stop_state __state)
>>+      : _M_state{std::move(__state)}
>>+    { }
>>+  };
>>+
>>+  class stop_source {
>>+    using _Stop_state_t = stop_token::_Stop_state_t;
>>+    using _Stop_state = stop_token::_Stop_state;
>>+
>>+  public:
>>+    stop_source()
>>+      : _M_state(std::make_shared<_Stop_state_t>())
>>+    { }
>>+
>>+    explicit stop_source(std::nostopstate_t) noexcept
>>+    { }
>>+
>>+    stop_source(const stop_source& __other) noexcept
>>+      : _M_state(__other._M_state)
>>+    { }
>>+
>>+    stop_source(stop_source&& __other) noexcept
>>+      : _M_state(std::move(__other._M_state))
>>+    { }
>>+
>>+    stop_source&
>>+    operator=(const stop_source& __rhs) noexcept
>>+    {
>>+      if (_M_state != __rhs._M_state)
>>+        _M_state = __rhs._M_state;
>>+      return *this;
>>+    }
>>+
>>+    stop_source&
>>+    operator=(stop_source&& __rhs) noexcept
>>+    {
>>+      std::swap(_M_state, __rhs._M_state);
>>+      return *this;
>>+    }
>
> These can all be defaulted too.
>
>>+    [[nodiscard]]
>>+    bool
>>+    stop_possible() const noexcept
>>+    {
>>+      return static_cast<bool>(_M_state);
>>+    }
>>+
>>+    [[nodiscard]]
>>+    bool
>>+    stop_requested() const noexcept
>>+    {
>>+      return stop_possible() && _M_state->_M_stop_requested();
>>+    }
>>+
>>+    bool
>>+    request_stop() const noexcept
>>+    {
>>+      if (stop_possible())
>>+        return _M_state->_M_request_stop();
>>+      return false;
>>+    }
>>+
>>+    [[nodiscard]]
>>+    stop_token
>>+    get_token() const noexcept
>>+    {
>>+      return stop_token{_M_state};
>>+    }
>>+
>>+    void
>>+    swap(stop_source& __other) noexcept
>>+    {
>>+      std::swap(_M_state, __other._M_state);
>>+    }
>>+
>>+    [[nodiscard]]
>>+    friend bool
>>+    operator==(const stop_source& __a, const stop_source& __b) noexcept
>>+    {
>>+      return __a._M_state == __b._M_state;
>>+    }
>>+
>>+    [[nodiscard]]
>>+    friend bool
>>+    operator!=(const stop_source& __a, const stop_source& __b) noexcept
>>+    {
>>+      return __a._M_state != __b._M_state;
>>+    }
>>+
>>+  private:
>>+    _Stop_state _M_state;
>>+  };
>>+
>>+  template<typename _Callback>
>>+    class [[nodiscard]] stop_callback
>>+      : private stop_token::_Stop_cb {
>
> Opening brace on the next line, in the same column as "class".
>
>>+    using _Stop_cb = stop_token::_Stop_cb;
>
> And then everything in the class body should be indented by another
> two columns.
>
>>+    using _Stop_state = stop_token::_Stop_state;
>>+  public:
>>+    using callback_type = _Callback;
>>+
>>+    template<typename _CB,
>
> Change to _Cb here too.


0001-Support-for-jthread-and-stop_token.patch (43K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add support for C++2a stop_token

Jonathan Wakely-3
On 15/11/19 14:38 +0000, Jonathan Wakely wrote:

>On 13/11/19 17:59 -0800, Thomas Rodgers wrote:
>>+  // TODO verify the standard requires this
>>+#if 0
>>+  // register another callback
>>+  bool cb3called{false};
>>+  std::stop_callback scb3{stok, [&]
>>+                                {
>>+                                  cb3called = true;
>>+                                }};
>>+  VERIFY(ssrc.stop_possible());
>>+  VERIFY(ssrc.stop_requested());
>>+  VERIFY(stok.stop_possible());
>>+  VERIFY(stok.stop_requested());
>>+  VERIFY(!cb1called);
>>+  VERIFY(cb2called);
>>+  VERIFY(cb3called);
>>+#endif
>
>The working draft definitely requires this:
>
>Effects: Initializes callback with std::forward<C>(cb). If st.stop_requested() is true, then
>std::forward<Callback>(callback)() is evaluated in the current thread before the constructor
>returns.
I've committed this fix to the nostopstate initializer, so it defines
a variable, instead of declaring a function. I've also added a test
that uses the stop_source(nostopstate_t) constructor, and a few other
tweaks to include the new header where it's needed.

Tested powerpc64le-linux, committed to trunk.


patch.txt (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add support for C++2a stop_token

Jonathan Wakely-3
On 15/11/19 14:40 +0000, Jonathan Wakely wrote:

>On 15/11/19 14:38 +0000, Jonathan Wakely wrote:
>>On 13/11/19 17:59 -0800, Thomas Rodgers wrote:
>>>+  // TODO verify the standard requires this
>>>+#if 0
>>>+  // register another callback
>>>+  bool cb3called{false};
>>>+  std::stop_callback scb3{stok, [&]
>>>+                                {
>>>+                                  cb3called = true;
>>>+                                }};
>>>+  VERIFY(ssrc.stop_possible());
>>>+  VERIFY(ssrc.stop_requested());
>>>+  VERIFY(stok.stop_possible());
>>>+  VERIFY(stok.stop_requested());
>>>+  VERIFY(!cb1called);
>>>+  VERIFY(cb2called);
>>>+  VERIFY(cb3called);
>>>+#endif
>>
>>The working draft definitely requires this:
>>
>>Effects: Initializes callback with std::forward<C>(cb). If st.stop_requested() is true, then
>>std::forward<Callback>(callback)() is evaluated in the current thread before the constructor
>>returns.
>
>I've committed this fix to the nostopstate initializer, so it defines
>a variable, instead of declaring a function. I've also added a test
>that uses the stop_source(nostopstate_t) constructor, and a few other
>tweaks to include the new header where it's needed.
This fixes some other issues I noticed, and should fix the failues for
the single-threaded multilib on AIX.

Tested powerpc64le-linux, committed to trunk.



patch.txt (19K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add support for C++2a stop_token

Jonathan Wakely-3
On 15/11/19 23:43 +0000, Jonathan Wakely wrote:

>On 15/11/19 14:40 +0000, Jonathan Wakely wrote:
>>On 15/11/19 14:38 +0000, Jonathan Wakely wrote:
>>>On 13/11/19 17:59 -0800, Thomas Rodgers wrote:
>>>>+  // TODO verify the standard requires this
>>>>+#if 0
>>>>+  // register another callback
>>>>+  bool cb3called{false};
>>>>+  std::stop_callback scb3{stok, [&]
>>>>+                                {
>>>>+                                  cb3called = true;
>>>>+                                }};
>>>>+  VERIFY(ssrc.stop_possible());
>>>>+  VERIFY(ssrc.stop_requested());
>>>>+  VERIFY(stok.stop_possible());
>>>>+  VERIFY(stok.stop_requested());
>>>>+  VERIFY(!cb1called);
>>>>+  VERIFY(cb2called);
>>>>+  VERIFY(cb3called);
>>>>+#endif
>>>
>>>The working draft definitely requires this:
>>>
>>>Effects: Initializes callback with std::forward<C>(cb). If st.stop_requested() is true, then
>>>std::forward<Callback>(callback)() is evaluated in the current thread before the constructor
>>>returns.
>>
>>I've committed this fix to the nostopstate initializer, so it defines
>>a variable, instead of declaring a function. I've also added a test
>>that uses the stop_source(nostopstate_t) constructor, and a few other
>>tweaks to include the new header where it's needed.
>
>This fixes some other issues I noticed, and should fix the failues for
>the single-threaded multilib on AIX.
And a couple more bugs fixed by this patch.

The std::jthread::get_id() function was missing a return statement.

The is_invocable check needs to be done using decayed types, as they'll
be forwarded to std::invoke as rvalues.

Also reduce header dependencies for the <thread> header. We don't need
to include <functional> for std::jthread because <bits/invoke.h> is
already included, which defines std::__invoke. We can also remove
<bits/functexcept.h> which isn't used at all. Finally, when
_GLIBCXX_HAS_GTHREADS is not defined there's no point including any
other headers, since we're not going to define anything in <thread>
anyway.

Tested powerpc64le-linux, committed to trunk.


patch.txt (12K) Download Attachment