This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: [PATCH] Replace __bind_simple with std::thread::__make_invoker
- From: Thomas Schwinge <thomas at codesourcery dot com>
- To: Jonathan Wakely <jwakely at redhat dot com>, <libstdc++ at gcc dot gnu dot org>, <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 13 Oct 2016 14:16:22 +0200
- Subject: Re: [PATCH] Replace __bind_simple with std::thread::__make_invoker
- Authentication-results: sourceware.org; auth=none
- References: <20161013103706.GA11280@redhat.com>
Hi!
Build failure after trunk r241093:
[...]/source-gcc/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc:121:12: error: ISO C++ forbids declaration of '_Bind_simple_helper' with no type [-fpermissive]
template _Bind_simple_helper<void (thread::*)(), reference_wrapper<thread>>::__type __bind_simple(void (thread::*&&)(), reference_wrapper<thread>&&);
^~~~~~~~~~~~~~~~~~~
[...]/source-gcc/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc:121:12: error: '_Bind_simple_helper' is not a template function
[...]/source-gcc/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc:121:31: error: expected ';' before '<' token
template _Bind_simple_helper<void (thread::*)(), reference_wrapper<thread>>::__type __bind_simple(void (thread::*&&)(), reference_wrapper<thread>&&);
^
For reference:
On Thu, 13 Oct 2016 11:37:06 +0100, Jonathan Wakely <jwakely@redhat.com> wrote:
> Some time ago I added __bind_simple, which is a simpler version of
> std::bind that doesn't support placeholders and nested bind
> expressions. It's only needed by a few concurrency components that do
> INVOKE(DECAY_COPY(f), DECAY_COPY(args)...) but those conponents have
> to include the whole of <functional> to get it.
>
> Now that std::call_once() doesn't use it, it's only needed in <thread>
> and <future> so I'm moving it into <thread>, renaming it to
> std::thread::__make_invoker, and adding some comments about its
> purpose.
>
> std::packaged_task shouldn't be using that function anyway, because it
> doesn't want the DECAY_COPY semantics and I was forced to wrap the
> arguments in reference_wrappers to prevent the decaying. Like
> std::call_once it can just use a lambda-expression instead.
>
> This allows us to remove <functional> from <thread> and <future>. That
> doesn't reduce the size of preprocessed source yet, because <memory>
> includes the whole of <functional> anyway, but I'll reduce that soon.
>
> * include/std/functional (_Bind_simple, _Bind_simple_helper)
> (__bind_simple): Remove.
> * include/std/future: Include <bits/invoke.h> instead of <functional>.
> (__future_base::_Task_state::_M_run)
> (__future_base::_Task_state::_M_run_delayed): Use lambda expressions
> instead of __bind_simple.
> (__future_base::_Task_state::_S_maybe_wrap_ref): Remove.
> (async): Use thread::__make_invoker instead of __bind_simple.
> * include/std/thread: Include <tuple> and <bits/invoke.h> instead of
> <functional>.
> (thread::_Invoker, thread::__make_invoker): Define helpers to do
> INVOKE(DECAY_COPY(f), DECAY_COPY(args)...).
>
> Tested powerpc64le-linux, committed to trunk.
>
> commit 13c071f6ee2e3edb177e6150f7a3903ecaf9de68
> Author: Jonathan Wakely <jwakely@redhat.com>
> Date: Wed Oct 12 17:15:46 2016 +0100
>
> Replace __bind_simple with std::thread::__make_invoker
>
> * include/std/functional (_Bind_simple, _Bind_simple_helper)
> (__bind_simple): Remove.
> * include/std/future: Include <bits/invoke.h> instead of <functional>.
> (__future_base::_Task_state::_M_run)
> (__future_base::_Task_state::_M_run_delayed): Use lambda expressions
> instead of __bind_simple.
> (__future_base::_Task_state::_S_maybe_wrap_ref): Remove.
> (async): Use thread::__make_invoker instead of __bind_simple.
> * include/std/thread: Include <tuple> and <bits/invoke.h> instead of
> <functional>.
> (thread::_Invoker, thread::__make_invoker): Define helpers to do
> INVOKE(DECAY_COPY(f), DECAY_COPY(args)...).
>
> diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional
> index 58134b7..3877033 100644
> --- a/libstdc++-v3/include/std/functional
> +++ b/libstdc++-v3/include/std/functional
> @@ -1300,69 +1300,6 @@ _GLIBCXX_MEM_FN_TRAITS(&&, false_type, true_type)
> std::forward<_BoundArgs>(__args)...);
> }
>
> - template<typename _Signature>
> - struct _Bind_simple;
> -
> - template<typename _Callable, typename... _Args>
> - struct _Bind_simple<_Callable(_Args...)>
> - {
> - typedef typename result_of<_Callable(_Args...)>::type result_type;
> -
> - template<typename _Tp, typename... _Up>
> - explicit
> - _Bind_simple(_Tp&& __f, _Up&&... __args)
> - : _M_bound(std::forward<_Tp>(__f), std::forward<_Up>(__args)...)
> - { }
> -
> - _Bind_simple(const _Bind_simple&) = default;
> - _Bind_simple(_Bind_simple&&) = default;
> -
> - result_type
> - operator()()
> - {
> - typedef typename _Build_index_tuple<sizeof...(_Args)>::__type _Indices;
> - return _M_invoke(_Indices());
> - }
> -
> - private:
> - template<std::size_t... _Indices>
> - typename result_of<_Callable(_Args...)>::type
> - _M_invoke(_Index_tuple<_Indices...>)
> - {
> - // std::bind always forwards bound arguments as lvalues,
> - // but this type can call functions which only accept rvalues.
> - return std::forward<_Callable>(std::get<0>(_M_bound))(
> - std::forward<_Args>(std::get<_Indices+1>(_M_bound))...);
> - }
> -
> - std::tuple<_Callable, _Args...> _M_bound;
> - };
> -
> - template<typename _Func, typename... _BoundArgs>
> - struct _Bind_simple_helper
> - : _Bind_check_arity<typename decay<_Func>::type, _BoundArgs...>
> - {
> - typedef _Maybe_wrap_member_pointer<typename decay<_Func>::type>
> - __maybe_type;
> - typedef typename __maybe_type::type __func_type;
> - typedef _Bind_simple<__func_type(typename decay<_BoundArgs>::type...)>
> - __type;
> - };
> -
> - // Simplified version of std::bind for internal use, without support for
> - // unbound arguments, placeholders or nested bind expressions.
> - template<typename _Callable, typename... _Args>
> - typename _Bind_simple_helper<_Callable, _Args...>::__type
> - __bind_simple(_Callable&& __callable, _Args&&... __args)
> - {
> - typedef _Bind_simple_helper<_Callable, _Args...> __helper_type;
> - typedef typename __helper_type::__maybe_type __maybe_type;
> - typedef typename __helper_type::__type __result_type;
> - return __result_type(
> - __maybe_type::__do_wrap( std::forward<_Callable>(__callable)),
> - std::forward<_Args>(__args)...);
> - }
> -
> /**
> * @brief Exception class thrown when class template function's
> * operator() is called with an empty target.
> diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future
> index aa866c5..fffbdbb 100644
> --- a/libstdc++-v3/include/std/future
> +++ b/libstdc++-v3/include/std/future
> @@ -35,7 +35,6 @@
> # include <bits/c++0x_warning.h>
> #else
>
> -#include <functional>
> #include <mutex>
> #include <thread>
> #include <condition_variable>
> @@ -43,6 +42,7 @@
> #include <atomic>
> #include <bits/atomic_futex.h>
> #include <bits/functexcept.h>
> +#include <bits/invoke.h>
> #include <bits/unique_ptr.h>
> #include <bits/shared_ptr.h>
> #include <bits/uses_allocator.h>
> @@ -1403,18 +1403,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> virtual void
> _M_run(_Args&&... __args)
> {
> - // bound arguments decay so wrap lvalue references
> - auto __boundfn = std::__bind_simple(std::ref(_M_impl._M_fn),
> - _S_maybe_wrap_ref(std::forward<_Args>(__args))...);
> + auto __boundfn = [&] () -> typename result_of<_Fn(_Args&&...)>::type {
> + return std::__invoke(_M_impl._M_fn, std::forward<_Args>(__args)...);
> + };
> this->_M_set_result(_S_task_setter(this->_M_result, __boundfn));
> }
>
> virtual void
> _M_run_delayed(_Args&&... __args, weak_ptr<_State_base> __self)
> {
> - // bound arguments decay so wrap lvalue references
> - auto __boundfn = std::__bind_simple(std::ref(_M_impl._M_fn),
> - _S_maybe_wrap_ref(std::forward<_Args>(__args))...);
> + auto __boundfn = [&] () -> typename result_of<_Fn(_Args&&...)>::type {
> + return std::__invoke(_M_impl._M_fn, std::forward<_Args>(__args)...);
> + };
> this->_M_set_delayed_result(_S_task_setter(this->_M_result, __boundfn),
> std::move(__self));
> }
> @@ -1422,17 +1422,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> virtual shared_ptr<_Task_state_base<_Res(_Args...)>>
> _M_reset();
>
> - template<typename _Tp>
> - static reference_wrapper<_Tp>
> - _S_maybe_wrap_ref(_Tp& __t)
> - { return std::ref(__t); }
> -
> - template<typename _Tp>
> - static
> - typename enable_if<!is_lvalue_reference<_Tp>::value, _Tp>::type&&
> - _S_maybe_wrap_ref(_Tp&& __t)
> - { return std::forward<_Tp>(__t); }
> -
> struct _Impl : _Alloc
> {
> template<typename _Fn2>
> @@ -1713,8 +1702,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> {
> __try
> {
> - __state = __future_base::_S_make_async_state(std::__bind_simple(
> - std::forward<_Fn>(__fn), std::forward<_Args>(__args)...));
> + __state = __future_base::_S_make_async_state(
> + std::thread::__make_invoker(std::forward<_Fn>(__fn),
> + std::forward<_Args>(__args)...)
> + );
> }
> #if __cpp_exceptions
> catch(const system_error& __e)
> @@ -1727,8 +1718,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> }
> if (!__state)
> {
> - __state = __future_base::_S_make_deferred_state(std::__bind_simple(
> - std::forward<_Fn>(__fn), std::forward<_Args>(__args)...));
> + __state = __future_base::_S_make_deferred_state(
> + std::thread::__make_invoker(std::forward<_Fn>(__fn),
> + std::forward<_Args>(__args)...));
> }
> return future<__async_result_of<_Fn, _Args...>>(__state);
> }
> diff --git a/libstdc++-v3/include/std/thread b/libstdc++-v3/include/std/thread
> index 7e185d3..7a3c407 100644
> --- a/libstdc++-v3/include/std/thread
> +++ b/libstdc++-v3/include/std/thread
> @@ -36,11 +36,12 @@
> #else
>
> #include <chrono>
> -#include <functional>
> #include <memory>
> +#include <tuple>
> #include <cerrno>
> #include <bits/functexcept.h>
> #include <bits/functional_hash.h>
> +#include <bits/invoke.h>
> #include <bits/gthr.h>
>
> #if defined(_GLIBCXX_HAS_GTHREADS) && defined(_GLIBCXX_USE_C99_STDINT_TR1)
> @@ -122,8 +123,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> auto __depend = nullptr;
> #endif
> _M_start_thread(_S_make_state(
> - std::__bind_simple(std::forward<_Callable>(__f),
> - std::forward<_Args>(__args)...)),
> + __make_invoker(std::forward<_Callable>(__f),
> + std::forward<_Args>(__args)...)),
> __depend);
> }
>
> @@ -212,6 +213,55 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> void
> _M_start_thread(__shared_base_type);
> #endif
> +
> + private:
> + // A call wrapper that does INVOKE(forwarded tuple elements...)
> + template<typename _Tuple>
> + struct _Invoker
> + {
> + _Tuple _M_t;
> +
> + template<size_t _Index>
> + static __tuple_element_t<_Index, _Tuple>&&
> + _S_declval();
> +
> + template<size_t... _Ind>
> + auto
> + _M_invoke(_Index_tuple<_Ind...>)
> + noexcept(noexcept(std::__invoke(_S_declval<_Ind>()...)))
> + -> decltype(std::__invoke(_S_declval<_Ind>()...))
> + { return std::__invoke(std::get<_Ind>(std::move(_M_t))...); }
> +
> + using _Indices
> + = typename _Build_index_tuple<tuple_size<_Tuple>::value>::__type;
> +
> + using result_type
> + = decltype(std::declval<_Invoker>()._M_invoke(_Indices()));
> +
> + result_type
> + operator()()
> + noexcept(noexcept(std::declval<_Invoker>()._M_invoke(_Indices())))
> + { return _M_invoke(_Indices()); }
> +
> + };
> +
> + // Alias for _Invoker<tuple<DECAY_COPY(_Tp)...>>
> + template<typename... _Tp>
> + using __invoker_type
> + = _Invoker<decltype(std::make_tuple(std::declval<_Tp>()...))>;
> +
> + public:
> + // Returns a call wrapper that does
> + // INVOKE(DECAY_COPY(__callable), DECAY_COPY(__args)).
> + template<typename _Callable, typename... _Args>
> + static __invoker_type<_Callable, _Args...>
> + __make_invoker(_Callable&& __callable, _Args&&... __args)
> + {
> + return { {
> + std::make_tuple(std::forward<_Callable>(__callable),
> + std::forward<_Args>(__args)...)
> + } };
> + }
> };
>
> inline void
Grüße
Thomas