From patchwork Sat Jul 9 10:13:34 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 103965 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 4D49B1007D4 for ; Sat, 9 Jul 2011 20:13:59 +1000 (EST) Received: (qmail 4768 invoked by alias); 9 Jul 2011 10:13:55 -0000 Received: (qmail 4570 invoked by uid 22791); 9 Jul 2011 10:13:51 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, TW_DF X-Spam-Check-By: sourceware.org Received: from mail-pv0-f175.google.com (HELO mail-pv0-f175.google.com) (74.125.83.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 09 Jul 2011 10:13:35 +0000 Received: by pvf24 with SMTP id 24so1818665pvf.20 for ; Sat, 09 Jul 2011 03:13:34 -0700 (PDT) MIME-Version: 1.0 Received: by 10.142.199.19 with SMTP id w19mr881113wff.323.1310206414342; Sat, 09 Jul 2011 03:13:34 -0700 (PDT) Received: by 10.142.229.19 with HTTP; Sat, 9 Jul 2011 03:13:34 -0700 (PDT) Date: Sat, 9 Jul 2011 11:13:34 +0100 Message-ID: Subject: [v3] fix libstdc++/49668 - do not use std::bind for simple call wrappers From: Jonathan Wakely To: "libstdc++" , gcc-patches Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org std::bind forwards the bound arguments as lvalues, so we shouldn't be using it to implement the INVOKE(DECAY_COPY(f), DECAY_COPY(args)...) pseudo-expressions in the threads clauses. Using std::bind we can't invoke functions which require rvalue arguments. This patch adds a bind-like helper that can be used by call wrappers in , and instead of std::bind. The call wrapper returned by __bind_simple may not be copy constructible so _Async_state and _Deferred_state cannot use std::function to store it. PR libstdc++/49668 * include/std/functional (__bind_simple): Define. * include/std/future (_Task_setter): Parameterize by type of result pointer instead of state object. (_S_task_setter): Type deduction helper. (_Task_state): Use _S_task_setter and __bind_simple. (_Deferred_state, _Async_state): Store call wrapper directly not as std::function. Use _S_task_setter and __bind_simple. (_S_make_deferred_state, _S_make_async_state): Type deduction helpers. (async): Use new functions and __bind_simple. * include/std/mutex (call_once): Use __bind_simple. * include/std/thread (thread): Likewise. Remove unused headers. * src/thread.cc: Add header. * testsuite/30_threads/async/49668.cc: New. * testsuite/30_threads/call_once/49668.cc: New. * testsuite/30_threads/thread/cons/49668.cc: New. * testsuite/30_threads/thread/cons/moveable.cc: Remove unused bool. Tested x86_64-linux, committed to trunk. Index: include/std/functional =================================================================== --- include/std/functional (revision 176072) +++ include/std/functional (working copy) @@ -1499,6 +1499,77 @@ std::forward<_BoundArgs>(__args)...); } + template + struct _Bind_simple; + + template + struct _Bind_simple<_Callable(_Args...)> + { + typedef typename result_of<_Callable(_Args...)>::type result_type; + + template::type> + explicit + _Bind_simple(const _Callable& __callable, _Args2&&... __args) + : _M_bound(__callable, std::forward<_Args2>(__args)...) + { } + + template::type> + explicit + _Bind_simple(_Callable&& __callable, _Args2&&... __args) + : _M_bound(std::move(__callable), std::forward<_Args2>(__args)...) + { } + + _Bind_simple(const _Bind_simple&) = default; + _Bind_simple(_Bind_simple&&) = default; + + result_type + operator()() + { + typedef typename _Build_index_tuple::__type _Indices; + return _M_invoke(_Indices()); + } + + private: + + template + 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 + struct _Bind_simple_helper + { + typedef _Maybe_wrap_member_pointer::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 _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. Index: include/std/future =================================================================== --- include/std/future (revision 176072) +++ include/std/future (working copy) @@ -488,17 +488,42 @@ virtual void _M_run_deferred() { } }; - template + template class _Deferred_state; - template + template class _Async_state; template class _Task_state; - template + template + static std::shared_ptr<_State_base> + _S_make_deferred_state(_BoundFn&& __fn); + + template + static std::shared_ptr<_State_base> + _S_make_async_state(_BoundFn&& __fn); + + template struct _Task_setter; + + template + class _Task_setter_helper + { + typedef typename remove_reference<_BoundFn>::type::result_type __res; + public: + typedef _Task_setter<_Res_ptr, __res> __type; + }; + + template + static typename _Task_setter_helper<_Res_ptr, _BoundFn>::__type + _S_task_setter(_Res_ptr& __ptr, _BoundFn&& __call) + { + typedef _Task_setter_helper<_Res_ptr, _BoundFn> __helper_type; + typedef typename __helper_type::__type _Setter; + return _Setter{ __ptr, std::ref(__call) }; + } }; /// Partial specialization for reference types. @@ -1165,29 +1190,29 @@ } - template + template struct __future_base::_Task_setter { - typename _StateT::_Ptr_type operator()() + _Ptr_type operator()() { __try { - _M_state->_M_result->_M_set(_M_fn()); + _M_result->_M_set(_M_fn()); } __catch(...) { - _M_state->_M_result->_M_error = current_exception(); + _M_result->_M_error = current_exception(); } - return std::move(_M_state->_M_result); + return std::move(_M_result); } - _StateT* _M_state; + _Ptr_type& _M_result; std::function<_Res()> _M_fn; }; - template - struct __future_base::_Task_setter<_StateT, void> + template + struct __future_base::_Task_setter<_Ptr_type, void> { - typename _StateT::_Ptr_type operator()() + _Ptr_type operator()() { __try { @@ -1195,11 +1220,11 @@ } __catch(...) { - _M_state->_M_result->_M_error = current_exception(); + _M_result->_M_error = current_exception(); } - return std::move(_M_state->_M_result); + return std::move(_M_result); } - _StateT* _M_state; + _Ptr_type& _M_result; std::function _M_fn; }; @@ -1223,13 +1248,12 @@ _M_run(_Args... __args) { // bound arguments decay so wrap lvalue references - auto __bound = std::bind<_Res>(std::ref(_M_task), - _S_maybe_wrap_ref(std::forward<_Args>(__args))...); - _Task_setter<_Task_state> __setter{ this, std::move(__bound) }; + auto __boundfn = std::__bind_simple(std::ref(_M_task), + _S_maybe_wrap_ref(std::forward<_Args>(__args))...); + auto __setter = _S_task_setter(_M_result, std::move(__boundfn)); _M_set_result(std::move(__setter)); } - template friend class _Task_setter; typedef typename __future_base::_Ptr<_Result<_Res>>::type _Ptr_type; _Ptr_type _M_result; std::function<_Res(_Args...)> _M_task; @@ -1331,40 +1355,34 @@ : public true_type { }; - template + template class __future_base::_Deferred_state : public __future_base::_State_base { public: - typedef _Res _Res_type; - explicit - _Deferred_state(std::function<_Res()>&& __fn) + _Deferred_state(_BoundFn&& __fn) : _M_result(new _Result<_Res>()), _M_fn(std::move(__fn)) { } private: - template friend class _Task_setter; typedef typename __future_base::_Ptr<_Result<_Res>>::type _Ptr_type; _Ptr_type _M_result; - std::function<_Res()> _M_fn; + _BoundFn _M_fn; virtual void _M_run_deferred() { - _Task_setter<_Deferred_state> __setter{ this, _M_fn }; // safe to call multiple times so ignore failure - _M_set_result(std::move(__setter), true); + _M_set_result(_S_task_setter(_M_result, _M_fn), true); } }; - template + template class __future_base::_Async_state : public __future_base::_State_base { public: - typedef _Res _Res_type; - explicit - _Async_state(std::function<_Res()>&& __fn) + _Async_state(_BoundFn&& __fn) : _M_result(new _Result<_Res>()), _M_fn(std::move(__fn)), _M_thread(mem_fn(&_Async_state::_M_do_run), this) { } @@ -1374,17 +1392,34 @@ private: void _M_do_run() { - _Task_setter<_Async_state> __setter{ this, std::move(_M_fn) }; - _M_set_result(std::move(__setter)); + _M_set_result(_S_task_setter(_M_result, _M_fn)); } - template friend class _Task_setter; typedef typename __future_base::_Ptr<_Result<_Res>>::type _Ptr_type; _Ptr_type _M_result; - std::function<_Res()> _M_fn; + _BoundFn _M_fn; thread _M_thread; }; + template + inline std::shared_ptr<__future_base::_State_base> + __future_base::_S_make_deferred_state(_BoundFn&& __fn) + { + typedef typename remove_reference<_BoundFn>::type __fn_type; + typedef _Deferred_state<__fn_type> __state_type; + return std::make_shared<__state_type>(std::move(__fn)); + } + + template + inline std::shared_ptr<__future_base::_State_base> + __future_base::_S_make_async_state(_BoundFn&& __fn) + { + typedef typename remove_reference<_BoundFn>::type __fn_type; + typedef _Async_state<__fn_type> __state_type; + return std::make_shared<__state_type>(std::move(__fn)); + } + + /// async template future::type> @@ -1394,14 +1429,12 @@ std::shared_ptr<__future_base::_State_base> __state; if ((__policy & (launch::async|launch::deferred)) == launch::async) { - typedef typename __future_base::_Async_state _State; - __state = std::make_shared<_State>(std::bind( + __state = __future_base::_S_make_async_state(std::__bind_simple( std::forward<_Fn>(__fn), std::forward<_Args>(__args)...)); } else { - typedef typename __future_base::_Deferred_state _State; - __state = std::make_shared<_State>(std::bind( + __state = __future_base::_S_make_deferred_state(std::__bind_simple( std::forward<_Fn>(__fn), std::forward<_Args>(__args)...)); } return future(__state); Index: include/std/mutex =================================================================== --- include/std/mutex (revision 176072) +++ include/std/mutex (working copy) @@ -801,13 +801,13 @@ call_once(once_flag& __once, _Callable&& __f, _Args&&... __args) { #ifdef _GLIBCXX_HAVE_TLS - auto __bound_functor = std::bind(std::forward<_Callable>(__f), + auto __bound_functor = std::__bind_simple(std::forward<_Callable>(__f), std::forward<_Args>(__args)...); __once_callable = &__bound_functor; __once_call = &__once_call_impl; #else unique_lock __functor_lock(__get_once_mutex()); - __once_functor = std::bind(std::forward<_Callable>(__f), + __once_functor = std::__bind_simple(std::forward<_Callable>(__f), std::forward<_Args>(__args)...); __set_once_functor_lock_ptr(&__functor_lock); #endif Index: include/std/thread =================================================================== --- include/std/thread (revision 176072) +++ include/std/thread (working copy) @@ -38,8 +38,6 @@ #include #include #include -#include -#include #include #include #include @@ -132,7 +130,7 @@ explicit thread(_Callable&& __f, _Args&&... __args) { - _M_start_thread(_M_make_routine(std::bind( + _M_start_thread(_M_make_routine(std::__bind_simple( std::forward<_Callable>(__f), std::forward<_Args>(__args)...))); } Index: src/thread.cc =================================================================== --- src/thread.cc (revision 176072) +++ src/thread.cc (working copy) @@ -24,6 +24,7 @@ #include +#include #include #if defined(_GLIBCXX_USE_GET_NPROCS) Index: testsuite/30_threads/async/49668.cc =================================================================== --- testsuite/30_threads/async/49668.cc (revision 0) +++ testsuite/30_threads/async/49668.cc (revision 0) @@ -0,0 +1,61 @@ +// { dg-do run { target *-*-freebsd* *-*-netbsd* *-*-linux* *-*-solaris* *-*-cygwin *-*-darwin* alpha*-*-osf* mips-sgi-irix6* } } +// { dg-options " -std=gnu++0x -pthread" { target *-*-freebsd* *-*-netbsd* *-*-linux* alpha*-*-osf* mips-sgi-irix6* } } +// { dg-options " -std=gnu++0x -pthreads" { target *-*-solaris* } } +// { dg-options " -std=gnu++0x " { target *-*-cygwin *-*-darwin* } } +// { dg-require-cstdint "" } +// { dg-require-gthreads "" } + +// Copyright (C) 2011 Free Software Foundation, Inc. +// +// 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. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +#include +#include +#include + +struct moveable +{ + moveable() = default; + moveable(moveable&&) = default; + moveable(const moveable&) = delete; +}; + +using std::launch; +namespace ph = std::placeholders; + +typedef decltype(ph::_1) placeholder_type; + +bool f(moveable, placeholder_type) { return true; } + +void test01() +{ + auto fut = std::async(launch::async, f, moveable(), ph::_1); + bool test = fut.get(); + VERIFY( test ); +} + +void test02() +{ + auto fut = std::async(launch::deferred, f, moveable(), ph::_1); + bool test = fut.get(); + VERIFY( test ); +} + +int main() +{ + test01(); + test02(); +} Index: testsuite/30_threads/call_once/49668.cc =================================================================== --- testsuite/30_threads/call_once/49668.cc (revision 0) +++ testsuite/30_threads/call_once/49668.cc (revision 0) @@ -0,0 +1,51 @@ +// { dg-do run { target *-*-freebsd* *-*-netbsd* *-*-linux* *-*-solaris* *-*-cygwin *-*-darwin* alpha*-*-osf* mips-sgi-irix6* } } +// { dg-options " -std=gnu++0x -pthread" { target *-*-freebsd* *-*-netbsd* *-*-linux* alpha*-*-osf* mips-sgi-irix6* } } +// { dg-options " -std=gnu++0x -pthreads" { target *-*-solaris* } } +// { dg-options " -std=gnu++0x " { target *-*-cygwin *-*-darwin* } } +// { dg-require-cstdint "" } +// { dg-require-gthreads "" } + +// Copyright (C) 2011 Free Software Foundation, Inc. +// +// 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. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +#include +#include +#include + +struct moveable +{ + moveable() = default; + moveable(moveable&&) = default; + moveable(const moveable&) = delete; +}; + +typedef decltype(std::placeholders::_1) placeholder_type; + +void f(moveable, placeholder_type, bool& b) { b = true; } + +void test01() +{ + bool test = false; + std::once_flag once; + std::call_once(once, f, moveable(), std::placeholders::_1, std::ref(test)); + VERIFY( test ); +} + +int main() +{ + test01(); +} Index: testsuite/30_threads/thread/cons/49668.cc =================================================================== --- testsuite/30_threads/thread/cons/49668.cc (revision 0) +++ testsuite/30_threads/thread/cons/49668.cc (revision 0) @@ -0,0 +1,51 @@ +// { dg-do run { target *-*-freebsd* *-*-netbsd* *-*-linux* *-*-solaris* *-*-cygwin *-*-darwin* alpha*-*-osf* mips-sgi-irix6* } } +// { dg-options " -std=gnu++0x -pthread" { target *-*-freebsd* *-*-netbsd* *-*-linux* alpha*-*-osf* mips-sgi-irix6* } } +// { dg-options " -std=gnu++0x -pthreads" { target *-*-solaris* } } +// { dg-options " -std=gnu++0x " { target *-*-cygwin *-*-darwin* } } +// { dg-require-cstdint "" } +// { dg-require-gthreads "" } + +// Copyright (C) 2011 Free Software Foundation, Inc. +// +// 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. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +#include +#include +#include + +struct moveable +{ + moveable() = default; + moveable(moveable&&) = default; + moveable(const moveable&) = delete; +}; + +typedef decltype(std::placeholders::_1) placeholder_type; + +void f(moveable, placeholder_type, bool& b) { b = true; } + +void test01() +{ + bool test = false; + std::thread t(f, moveable(), std::placeholders::_1, std::ref(test)); + t.join(); + VERIFY( test ); +} + +int main() +{ + test01(); +} Index: testsuite/30_threads/thread/cons/moveable.cc =================================================================== --- testsuite/30_threads/thread/cons/moveable.cc (revision 176072) +++ testsuite/30_threads/thread/cons/moveable.cc (working copy) @@ -5,7 +5,7 @@ // { dg-require-cstdint "" } // { dg-require-gthreads "" } -// Copyright (C) 2009 Free Software Foundation, Inc. +// Copyright (C) 2009, 2010, 2011 Free Software Foundation, Inc. // // 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 @@ -27,8 +27,6 @@ #include #include -bool functor_was_called = false; - struct moveable { moveable() = default;