Patchwork [v3] fix libstdc++/49668 - do not use std::bind for simple call wrappers

login
register
mail settings
Submitter Jonathan Wakely
Date July 9, 2011, 10:13 a.m.
Message ID <CAH6eHdRNeRR-JkUaqnF5re6UiO1ocAdcTZ3LZ0G1ZveQ9F-NYg@mail.gmail.com>
Download mbox | patch
Permalink /patch/103965/
State New
Headers show

Comments

Jonathan Wakely - July 9, 2011, 10:13 a.m.
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 <mutex>, <thread> and <future> 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.

Patch

Index: include/std/functional
===================================================================
--- include/std/functional	(revision 176072)
+++ include/std/functional	(working copy)
@@ -1499,6 +1499,77 @@ 
 			   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... _Args2, typename = typename
+               enable_if< sizeof...(_Args) == sizeof...(_Args2)>::type>
+        explicit
+        _Bind_simple(const _Callable& __callable, _Args2&&... __args)
+        : _M_bound(__callable, std::forward<_Args2>(__args)...)
+        { }
+
+      template<typename... _Args2, typename = typename
+               enable_if< sizeof...(_Args) == sizeof...(_Args2)>::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<sizeof...(_Args)>::__type _Indices;
+        return _M_invoke(_Indices());
+      }
+
+    private:
+
+      template<int... _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
+    {
+      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.
Index: include/std/future
===================================================================
--- include/std/future	(revision 176072)
+++ include/std/future	(working copy)
@@ -488,17 +488,42 @@ 
       virtual void _M_run_deferred() { }
     };
 
-    template<typename _Res>
+    template<typename _BoundFn, typename = typename _BoundFn::result_type>
       class _Deferred_state;
 
-    template<typename _Res>
+    template<typename _BoundFn, typename = typename _BoundFn::result_type>
       class _Async_state;
 
     template<typename _Signature>
       class _Task_state;
 
-    template<typename _StateT, typename _Res = typename _StateT::_Res_type>
+    template<typename _BoundFn>
+      static std::shared_ptr<_State_base>
+      _S_make_deferred_state(_BoundFn&& __fn);
+
+    template<typename _BoundFn>
+      static std::shared_ptr<_State_base>
+      _S_make_async_state(_BoundFn&& __fn);
+
+    template<typename _Res_ptr, typename _Res>
       struct _Task_setter;
+
+    template<typename _Res_ptr, typename _BoundFn>
+      class _Task_setter_helper
+      {
+	typedef typename remove_reference<_BoundFn>::type::result_type __res;
+      public:
+	typedef _Task_setter<_Res_ptr, __res> __type;
+      };
+
+    template<typename _Res_ptr, typename _BoundFn>
+      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<typename _StateT, typename _Res>
+  template<typename _Ptr_type, typename _Res>
     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<typename _StateT>
-    struct __future_base::_Task_setter<_StateT, void>
+  template<typename _Ptr_type>
+    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<void()>     _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<typename, typename> 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<typename _Res>
+  template<typename _BoundFn, typename _Res>
     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<typename, typename> 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<typename _Res>
+  template<typename _BoundFn, typename _Res>
     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<typename, typename> 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<typename _BoundFn>
+    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<typename _BoundFn>
+    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<typename _Fn, typename... _Args>
     future<typename result_of<_Fn(_Args...)>::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<result_type> _State;
-	  __state = std::make_shared<_State>(std::bind<result_type>(
+	  __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<result_type> _State;
-	  __state = std::make_shared<_State>(std::bind<result_type>(
+	  __state = __future_base::_S_make_deferred_state(std::__bind_simple(
               std::forward<_Fn>(__fn), std::forward<_Args>(__args)...));
 	}
       return future<result_type>(__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<void>(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<decltype(__bound_functor)>;
 #else
       unique_lock<mutex> __functor_lock(__get_once_mutex());
-      __once_functor = std::bind<void>(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 <chrono>
 #include <functional>
 #include <memory>
-#include <mutex>
-#include <condition_variable>
 #include <bits/functexcept.h>
 #include <bits/functional_hash.h>
 #include <bits/gthr.h>
@@ -132,7 +130,7 @@ 
       explicit 
       thread(_Callable&& __f, _Args&&... __args)
       {
-        _M_start_thread(_M_make_routine(std::bind<void>(
+        _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 <thread>
+#include <system_error>
 #include <cerrno>
 
 #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
+// <http://www.gnu.org/licenses/>.
+
+#include <future>
+#include <functional>
+#include <testsuite_hooks.h>
+
+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
+// <http://www.gnu.org/licenses/>.
+
+#include <mutex>
+#include <functional>
+#include <testsuite_hooks.h>
+
+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
+// <http://www.gnu.org/licenses/>.
+
+#include <thread>
+#include <functional>
+#include <testsuite_hooks.h>
+
+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 <utility>
 #include <testsuite_hooks.h>
 
-bool functor_was_called = false;
-
 struct moveable
 {
   moveable() = default;