Opened 9 years ago
Closed 9 years ago
#9058 closed Feature Requests (fixed)
with_lock_guard function
Reported by: | Owned by: | viboes | |
---|---|---|---|
Milestone: | Boost 1.56.0 | Component: | thread |
Version: | Boost 1.54.0 | Severity: | Problem |
Keywords: | Cc: |
Description
There is example of with_lock_guard function in documentation. Generalized implementation:
template <class Lockable, class Function, class... Args> auto with_lock_guard( Lockable& m, BOOST_FWD_REF(Function) f, BOOST_FWD_REF(Args)... args ) -> decltype(f(args...)) { boost::lock_guard<Lockable> lock(m); return f(args...); }
I can create patch, to what file should I add this? Or create new one?
Attachments (5)
Change History (28)
comment:1 by , 9 years ago
follow-up: 3 comment:2 by , 9 years ago
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.
follow-ups: 5 6 comment:3 by , 9 years ago
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?
comment:4 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
by , 9 years ago
Attachment: | with_lock_guard.2.hpp added |
---|
update implementation: forward for compiler without c++11 variadic templates
follow-up: 7 comment:5 by , 9 years ago
Replying to viboes:
Why the non variadic version don't provides move semantics?
I simply don't think about this combination, fixed.
follow-up: 12 comment:6 by , 9 years ago
follow-up: 8 comment:7 by , 9 years ago
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?
follow-up: 9 comment:8 by , 9 years ago
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.
comment:9 by , 9 years ago
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 :(
comment:10 by , 9 years ago
The C++11 definition must be protected also with no rvalue references and no decltype.
comment:11 by , 9 years ago
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 (?)
comment:12 by , 9 years ago
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
comment:13 by , 9 years ago
Milestone: | To Be Determined → Boost 1.56.0 |
---|
follow-up: 15 comment:14 by , 9 years ago
Hi,
could you provide a complete patch to be added on next release?
Vicente
follow-up: 16 comment:15 by , 9 years ago
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
comment:16 by , 9 years ago
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
follow-up: 19 comment:17 by , 9 years ago
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.
comment:18 by , 9 years ago
Milestone: | Boost 1.56.0 → To Be Determined |
---|
follow-up: 20 comment:19 by , 9 years ago
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?
comment:20 by , 9 years ago
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.
comment:21 by , 9 years ago
Milestone: | To Be Determined → Boost 1.56.0 |
---|
comment:22 by , 9 years ago
comment:23 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
A new file please.