Boost C++ Libraries: Ticket #9058: with_lock_guard function

viboes — Wed, 28 Aug 2013 20:39:59 GMT

A new file please.

attachment set

ruslan_baratov@… — Sat, 31 Aug 2013 07:01:55 GMT

attachment → with_lock_guard.hpp

implementation

attachment set

ruslan_baratov@… — Sat, 31 Aug 2013 07:02:15 GMT

attachment → with_lock_guard_example.cpp

example

attachment set

ruslan_baratov@… — Sat, 31 Aug 2013 07:02:30 GMT

attachment → with_lock_guard_usage.cpp

usage

ruslan_baratov@… — Sat, 31 Aug 2013 07:05:14 GMT

version for compilers without c++11 variadic templates support is limited to 4 arguments and not working with lambdas (boost::result_of limitation). Tested on clang 3.2/5.0, gcc 4.7.3, msvc 2012.

viboes — Sat, 31 Aug 2013 08:55:47 GMT

Replying to ruslan_baratov@…:

version for compilers without c++11 variadic templates support is limited to 4 arguments and not working with lambdas (boost::result_of limitation). Tested on clang 3.2/5.0, gcc 4.7.3, msvc 2012.

Thanks for the patch.

Why the non variadic version don't provides move semantics?

Could you add a patch for the documentation?

owner, status changed

viboes — Sat, 31 Aug 2013 10:10:02 GMT

owner changed from Anthony Williams to viboes
status new → assigned

attachment set

ruslan_baratov@… — Sat, 31 Aug 2013 11:42:45 GMT

attachment → with_lock_guard.2.hpp

update implementation: forward for compiler without c++11 variadic templates

attachment set

ruslan_baratov@… — Sat, 31 Aug 2013 11:43:05 GMT

attachment → with_lock_guard_usage.2.cpp

usage update

ruslan_baratov@… — Sat, 31 Aug 2013 11:44:13 GMT

Replying to viboes:

Why the non variadic version don't provides move semantics?

I simply don't think about this combination, fixed.

ruslan_baratov@… — Sat, 31 Aug 2013 11:45:43 GMT

Replying to viboes:

Could you add a patch for the documentation?

I take a look.

viboes — Sat, 31 Aug 2013 11:54:05 GMT

Replying to ruslan_baratov@…:

Replying to viboes:

Why the non variadic version don't provides move semantics?

I simply don't think about this combination, fixed.

Func could also be Movable.

Could you add some test with movable only types?

ruslan_baratov@… — Sat, 31 Aug 2013 12:21:22 GMT

Replying to viboes:

Func could also be Movable.

Not working for g++ without c++11 support (can't bind simple function):

with_lock_guard_usage.cpp:42:44: error:
no matching function for call to 'with_lock_guard(boost::mutex&, void (&)())'

clang without c++11 support refuse to work with boost::bind

boost/bind/mem_fn_template.hpp:151:29: error:
call to pointer to member function of type 'int (int &)' drops 'const' qualifier

Could you add some test with movable only types?

Ok.

viboes — Sat, 31 Aug 2013 13:06:45 GMT

Replying to ruslan_baratov@…:

Replying to viboes:

Func could also be Movable.

Not working for g++ without c++11 support (can't bind simple function):
with_lock_guard_usage.cpp:42:44: error:
no matching function for call to 'with_lock_guard(boost::mutex&, void (&)())'
clang without c++11 support refuse to work with boost::bind
boost/bind/mem_fn_template.hpp:151:29: error:
call to pointer to member function of type 'int (int &)' drops 'const' qualifier

I didn't said that it would be simple ;-)

You would need to add some specific overloads when rvalue references are not supported by the compiler :(

viboes — Sun, 01 Sep 2013 07:06:07 GMT

The C++11 definition must be protected also with no rvalue references and no decltype.

ruslan_baratov@… — Sun, 01 Sep 2013 10:56:44 GMT

Updated version on github:

  github.com/ruslo/thread/commit/ea30f0dc6b1fe7951d1d0dff88fd03e946429669

For function without c++11 variadic templates, Func parameter is now movable:

template <class Lockable, class Func, class Arg>
typename boost::result_of<Func(Arg)>::type with_lock_guard(
    Lockable& m,
    BOOST_FWD_REF(Func) func,
    BOOST_FWD_REF(Arg) arg
) {
  boost::lock_guard<Lockable> lock(m);
  return func(
      boost::forward<Arg>(arg)
  );
}

overloaded version for function pointers:

template <class Lockable, class Func, class Arg1, class Arg2>
typename boost::result_of<
    typename boost::add_pointer<Func>::type(Arg1, Arg2)
>::type with_lock_guard(
    Lockable& m,
    Func* func,
    BOOST_FWD_REF(Arg1) arg1,
    BOOST_FWD_REF(Arg2) arg2
) {
  BOOST_STATIC_ASSERT(boost::is_function<Func>::value);
  boost::lock_guard<Lockable> lock(m);
  return func(
      boost::forward<Arg1>(arg1),
      boost::forward<Arg2>(arg2)
  );
}

In this version boost::bind which call non-const class method still not working (for variadic template version is OK). It's because BOOST_FWD_REF(Func) = const Func&. I can't figure out how to do a workaround in not ugly way. I can make version with copy-by-value for types with T::result_type defined if this is appropriate.

If used with lambda, BOOST_RESULT_OF_USE_DECLTYPE may need to be defined, see this bug: https://svn.boost.org/trac/boost/ticket/7311

I've added test:

  test_with_lock_guard_bind.cpp
  test_with_lock_guard_lambda.cpp
  test_with_lock_guard_move.cpp
  test_with_lock_guard_simple.cpp

Replying to viboes:

The C++11 definition must be protected also with no rvalue references and no decltype.

I've add BOOST_NO_CXX11_DECLTYPE check; do BOOST_NO_CXX11_RVALUE_REFERENCES check really needed? I thought it automatically checked in BOOST_FWD_REF (?)

ruslan_baratov@… — Mon, 02 Sep 2013 20:12:21 GMT

Replying to ruslan_baratov@…:

Replying to viboes:

Could you add a patch for the documentation?

I take a look.

Documentation for with_lock_guard added to mutex_concept.qbk, examples updated in sync_tutorial.qbk.

github.com/ruslo/thread/commit/425efc8ab1f87af30f16368ea105f82c41e6639d

milestone changed

viboes — Mon, 23 Sep 2013 16:48:55 GMT

milestone To Be Determined → Boost 1.56.0

viboes — Tue, 15 Oct 2013 22:23:36 GMT

Hi,

could you provide a complete patch to be added on next release?

Vicente

ruslan_baratov@… — Thu, 17 Oct 2013 12:55:15 GMT

Replying to viboes:

could you provide a complete patch to be added on next release?

Ok, patch can be found here:

github.com/ruslo/thread/commit/0d16fb0b97bd5218d4b694d574ccbd5cc668be9b

viboes — Thu, 17 Oct 2013 17:46:34 GMT

Replying to ruslan_baratov@…:

Replying to viboes:

could you provide a complete patch to be added on next release?

Ok, patch can be found here:
github.com/ruslo/thread/commit/0d16fb0b97bd5218d4b694d574ccbd5cc668be9b

Thanks, Vicente

viboes — Tue, 28 Jan 2014 21:17:51 GMT

Hi,

I have started to integrate the patch, but I have some remarks.

The patch doesn't allows to pass class member functions. It will take some time to take in account this feature that makes it uniform to the other callable interfaces as thread constructor, packaged_task constructor and async (at least for C++11 compilers). I will come back to you as soon as I have something working.

Sorry for the delay.

milestone changed

viboes — Tue, 28 Jan 2014 21:17:59 GMT

milestone Boost 1.56.0 → To Be Determined

ruslan_baratov@… — Wed, 29 Jan 2014 16:02:45 GMT

Replying to viboes:

Hi,

I have started to integrate the patch, but I have some remarks.

The patch doesn't allows to pass class member functions. It will take some time to take in account this feature that makes it uniform to the other callable interfaces as thread constructor, packaged_task constructor and async (at least for C++11 compilers). I will come back to you as soon as I have something working.

Sorry for the delay.

Hi Vicente!

Looks like 'invoke' function fits perfectly for this job. I've found implementation in 'thread/detail/invoke.hpp' but I don't know what is the current ready-status of this file. Can I use it?

viboes — Wed, 29 Jan 2014 18:38:44 GMT

Replying to ruslan_baratov@…:

Replying to viboes:

Hi,

I have started to integrate the patch, but I have some remarks.

The patch doesn't allows to pass class member functions. It will take some time to take in account this feature that makes it uniform to the other callable interfaces as thread constructor, packaged_task constructor and async (at least for C++11 compilers). I will come back to you as soon as I have something working.

Sorry for the delay.

Hi Vicente!

Looks like 'invoke' function fits perfectly for this job. I've found implementation in 'thread/detail/invoke.hpp' but I don't know what is the current ready-status of this file. Can I use it?

Yes, the idea is to use invoke, but I have had some regressions recently with invoke on C++11 on some tests associated to async. When I disable the C++ implementation these test work.

I don't know yet if I will deliver all the new features associated to futures as executors, ... in boost 1.56 :(

I need to add unit test for the invoke function.

Maybe you could give it a try and report to me any issues privately.

milestone changed

viboes — Sun, 09 Feb 2014 02:56:03 GMT

milestone To Be Determined → Boost 1.56.0

Pulled on develop https://github.com/boostorg/thread/commit/b1ac899f23f915c19dfd3e8be9dd8b94b43ef55d and https://github.com/boostorg/thread/commit/80634ee51e3a9fd39d1daae6d9431588f783a6ea

viboes — Sat, 15 Feb 2014 16:21:31 GMT

https://github.com/boostorg/thread/commit/750c849b0f0dff79a289111955260a4147ac7f59

status changed; resolution set

viboes — Sat, 15 Feb 2014 16:21:39 GMT

status assigned → closed
resolution → fixed