|
Boost : |
Subject: Re: [boost] [thread] Boost.Thread defines boost::move which conflicts with Boost.Move
From: Evan Wallace (onlyone_at_[hidden])
Date: 2012-01-09 09:07:07
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 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()))
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?
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?
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?
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?
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
it.
Evan Wallace
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk