Le 09/01/12 22:31, Jeffrey Lee Hellrung, Jr. a écrit :
On Mon, Jan 9, 2012 at 7:29 AM, Vicente Botet<vicente.botet@wanadoo.fr>wrote:
Mankarse wrote
On Thu, 05 Jan 2012 07:00:27 +0100, Vicente J. Botet Escriba wrote:>Mankarse please, could you post here the patch for the boost.thread>class so that I can start the integration of what you have already done? I have attached a patch to the ticket (https://svn.boost.org/trac/boost/ticket/6194). This patch is fairly comprehensive in terms of code, but it does not include any tests or documentation. In the course of writing the patch, I created some simple tests, but I have not yet translated these into Boost.Test testcases, or included them in the patch.
Thanks Evan,
I will try to merge it on my own repository and see how it behaves. I have added some tests to check the move semantics. They are in directory libs/thread/test/threads and libs/thread/test/sync. Maybe you could check them and suggest for others.
BTw,
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_))
I don't think so. I haven't brushed up on the latest changes to Boost.Move since being officially added to Boost, but prior incarnations paired boost::forward with BOOST_FWD_REF. I think boost::move is correct when using BOOST_RV_REF.
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 :( If you don't want to compromise, you could use a different set of overloads in C++03 and C++11:
#ifndef BOOST_NO_RVALUE_REFERENCES template< class F> thread_data(F&& f); // and use boost::forward<F>(f) #else // #ifndef BOOST_NO_RVALUE_REFERENCES template< class F> thread_data(F f); // and use boost::move(f) template< class F> thread_data(boost::rv<F>& f); // and use boost::move(f) #endif // #ifndef BOOST_NO_RVALUE_REFERENCES
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. I have not used Boost.Move yet. I will take this in account while applying the patch.
I have tested the basic functionality on a number of compilers, but there are probably still some bugs in the patched code. The patch contains a number of known problems that may be worth discussing:1) Explicit move() and copy() calls:boost::thread and boost::packaged_task both have templated constructors that take an rvalue reference to a functor (which they move or copy as appropriate). Templates parameters cannot be deduced to a type that is only reachable through an conversion operator. Template constructors can also never be explicitly instansiated. (As far as I know.) This means that the overloads that are used in the definitions of BOOST_COPYABLE_AND_MOVABLE types (BOOST_RV_REF(type), BOOST_CATCH_CONST_RLVALUE(type), type&), are not matched for rvalue objects (in C++03 mode). Example: //In C++11 the `CopyableAndMovableFunctor` would be //moved into the thread. //In C++03 this does not compile. boost::thread((CopyableAndMovableFunctor()))
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?
It's a known limitation in Boost.Move, AFAIK. The above change should address this issue.
Could you point where in the doc it is described or to the Trac ticket?
Here, no overload would be matched, because the `type` in the overloads is templated. To solve this problem I modified Boost.Move to add two member functions - `copy()` and `move()` - which forward to the rv const& and rv& conversion operators. With these, the above example can be rewritten as: //In both C++11 and C++03 the `CopyableAndMovableFunctor` //will be moved or copied into the thread as appropriate. boost::thread(CopyableAndMovableFunctor().move()), boost::thread(CopyableAndMovableFunctor().copy()) I can imagine the names `copy` and `move` being quite common, so this might cause unfortunate conflicts. Is this an acceptable change to Boost.Move? Is there another way to get the functionality that does not require this?
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?
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 acceptable if the user interface doesn't changes.
Thanks for all your comments, Vicente