Subject: Re: [boost] [thread] Boost.Thread defines boost::move which conflicts with Boost.Move
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2012-01-09 17:17:26
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_at_[hidden]>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.
>> Should't the following
>> thread_data(BOOST_RV_REF(F) f_):
>> use forward instead of move?
>> thread_data(BOOST_RV_REF(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.
>> 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
> 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.
>>> 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,
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk