Le 10/01/12 13:11, Evan Wallace a écrit :
Should't the following thread_data(BOOST_RV_REF(F) f_): f(boost::move(f_))
use forward instead of move? thread_data(BOOST_RV_REF(F) f_): f(boost::forward<F>(f_))
Why the following (both) overloads are needed?
thread_data(BOOST_RV_REF(F) f_); thread_data(F const& f_);
When I submitted the patch, I thought that my handling of the forwarding was a bit awkward. As I will describe later in this email, I have submitted a new patch which uses the technique suggested by Jeffrey Lee Hellrung. That said, this discussion about the `thread_data` constructors is mostly irrelevant. `thread_data` is an internal type, and the code in the `thread` constructors already performs the lvalue/rvalue matching and calls the appropriate `thread_data` constructor.
Hrr, this is really a bad new. Could you create a Boost.Move ticket if the limitation is not documented and a ticket doesn't exists already for this issue? As the issue has been resolved by an alternative technique, this is probably not necessary.
Could you try to run the test on trunk related to move semantics with your version? I will do this later tonight.
Could you analyze in more detail the impact of using has_move_emulation_enabled? I investigated further, and while I still do not fully understand the impact, it seems that it is harmless to use it in the particular case where I am using it. In the name of consistency, I have changed the code to use has_move_emulation_enabled in version 1 of the patch.
Why the following (both) overloads are needed?
thread_data(BOOST_RV_REF(F) f_); thread_data(F const& f_); I don't know the context here, but I believe, in C++03, this would: capture-and-copy lvalues and rvalues; and capture-and-move explicitly created emulated rvalue references. Thus, true rvalues are captured as lvalues :( `thread_data` is an internal class. Its constructors are only ever called with explicitly created rvalues (which are forwarded from the rvalues that are matched in the constructor of `thread`), or lvalues. Thus the above behaviour is not problematic.
If you don't want to compromise, you could use a different set of overloads in C++03 and C++11: [snip] I haven't tested this, but it *should* capture-and-move both true rvalues and emulated rvalue references in C++03. And if it's an emulated rvalue reference, the second overload requires 1 fewer move construction. Thankyou for the idea!
It's a known limitation in Boost.Move, AFAIK. The above change should address this issue. It does. I have submitted a new version of the patch that uses those overloads in the constructors of `thread` and `packaged_task`.
[snip... explicit move() and copy() ...] I don't like this too much. Why doesn't boost::move works in this case? Could you give an example on which .copy() is needed? boost::move will not work on rvalues. `.copy()` is needed to allow a constructor to be matched from a const rvalue.
I don't know if this point will prevent the adoption of Boost.Move by Boost.Thread. I will comeback when I do some trials. See if the aforementioned solution I gave is acceptable. It is. Thankyou!
[I like the D-style template argument delimiters!] I have no confidence in my mail client whatsoever (you can see what it did to the formatting of my message), and I was trying to avoid the template being argument delimiters being seen as HTML tags and stripped. Note also - there was an error in my original posting, what I meant was `is_convertible!(T&, boost::rv!(T)&)`.
has_move_emulation_enabled<T> should be identical to boost::is_convertible<T&, boost::rv<T>& >, no? It isn't identical, but in the particular situation in which it is being used, I believe it has the same effect.
If the above is all accurate, then I wonder where the above 3 constructor overloads came from in the first place...? The above is all accurate. The 3 constructor overloads originally came from the constructors that all BOOST_COPYABLE_AND_MOVABLE types have. I thought that using the same overloads would also be appropriate for a template. I was wrong (for the reasons that you gave).
Hi, I have adapted your patch and many other tickets and committed the whole on trunk at revision 76543. * [@http://svn.boost.org/trac/boost/ticket/2741 #2741] Proposal to manage portable and non portable thread attributes. * [@http://svn.boost.org/trac/boost/ticket/6195 #6195] c++11 compliance: Provide the standard time related interface using Boost.Chrono. * [@http://svn.boost.org/trac/boost/ticket/6224 #6224] c++11 compliance: Add the use of standard noexcept on compilers supporting them. * [@http://svn.boost.org/trac/boost/ticket/6226 #6226] c++11 compliance: Add explicit bool conversion from locks. * [@http://svn.boost.org/trac/boost/ticket/6230 #6230] c++11 compliance: Follows the exception reporting mechanism as defined in the c++11. * [@http://svn.boost.org/trac/boost/ticket/6272 #6272] c++11 compliance: Add thread::id hash specialization. * [@http://svn.boost.org/trac/boost/ticket/6273 #6273] c++11 compliance: Add cv_status enum class and use it on the conditions wait functions. * [@http://svn.boost.org/trac/boost/ticket/6194 #6194] Adapt to Boost.Move. Fixed Bugs: * [@http://svn.boost.org/trac/boost/ticket/2575 #2575] Bug- Boost 1.36.0 on Itanium platform. * [@http://svn.boost.org/trac/boost/ticket/4921 #4921] BOOST_THREAD_USE_DLL and BOOST_THREAD_USE_LIB are crucial and need to be documented. * [@http://svn.boost.org/trac/boost/ticket/5013 #5013] documentation: boost::thread: pthreas_exit causes terminate(). * [@http://svn.boost.org/trac/boost/ticket/5351 #5351] interrupt a future get boost::unknown_exception. * [@http://svn.boost.org/trac/boost/ticket/5516 #5516] Upgrade lock is not acquired when previous upgrade lock releases if another read lock is present. * [@http://svn.boost.org/trac/boost/ticket/5990 #5990] shared_future<T>::get() has wrong return type. * [@http://svn.boost.org/trac/boost/ticket/6174 #6174] packaged_task doesn't correctly handle moving results. I have tested it on Mac gcc and clang, Windows MSVC-10, MinGw gcc-4.6.0 0x. I have some trouble yet on thread constructors. Hopping I have not introduced other regressions on other platforms. Best, Vicente