Subject: Re: [boost] [thread] Boost.Thread defines boost::move which conflicts with Boost.Move
From: Vicente Botet (vicente.botet_at_[hidden])
Date: 2012-01-09 10:29:26
> 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.
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.
Should't the following
use forward instead of move?
Why the following (both) overloads are needed?
thread_data(F const& f_);
> 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.Â Â
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
> 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()),Â Â
> 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.
> 2) SFINAE requiredBoost.Move requires SFINAE, and by extension
> Boost.Thread now also requires SFINAE. Previously, Boost.Thread did
> not require SFINAE. Is this an acceptable regression, or should more
> work be put into maintaining compatibility with compilers that do not
> support SFINAE?
A lot of libraries are requiring SFINAE now. Could you create a Boost.Move
ticket if the limitation is not documented and a ticket doesn't exists
already for this issue?
We could have a specific code for compilers that don't support SFINAE. I
will see what I can do while merging your patch.
> 3) Not using has_move_emulation_enabledI did not notice the
> `has_move_emulation_enabled` metafunction, and instead used
> `is_convertible!(T&, boost::rv!(T))`. This may mean that some
> situations that `has_move_emulation_enabled` accounts for will not be
> accounted for by the code in the patch. I have not yet evaluated
> whether simply replacing all the occurrences will work. Any thoughts?
I have no idea. I suspect that I will need to look for this in Boost.Move.
> 4) Not using Vicente's patches for #6174 or #5990By the time that I
> was aware of these patches, I had already independently fixed the
> issues. I have not looked at either patch in detail, but they each go
> about fixing the issues in a significantly different way from my
> patch. Which code should be used in each of these cases?
Yes, I know that some of my commits could be in conflict with the Boost.Move
adoption, but I wanted to fix as much as possible as I was not sure the
adoption will be in release 1.49. I will take a lot of care while doing the
> Where should I go from here? I could do the integration into the svn
> trunk version, write the documentation, or update the tests to the new
> version. Are there any other tasks that I can help out with? I will of
> course keep the patch updated with fixes for any bugs that I find in
Could you try to run the test on trunk related to move semantics with your
version? To do that just copy
the directories threads and sync from libs/thread/test and the Jamfile
whithout changing the sources and do
bjam threads sync
I will commit this evening more updates on trunk so that these tests should
Could you analyze in more detail the impact of using
Thanks a lot,
-- View this message in context: http://boost.2283326.n4.nabble.com/thread-Boost-Thread-defines-boost-move-which-conflicts-with-Boost-Move-tp4249499p4278761.html Sent from the Boost - Dev mailing list archive at Nabble.com.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk