Re: [boost] [thread] Boost.Thread defines boost::move which conflicts with Boost.Move
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
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_)) Why the following (both) overloads are needed? thread_data(BOOST_RV_REF(F) f_); 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. 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?
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.
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 merge.
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.
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 run better. Could you analyze in more detail the impact of using has_move_emulation_enabled? Thanks a lot, Vicente -- View this message in context: http://boost.2283326.n4.nabble.com/thread-Boost-Thread-defines-boost-move-wh... Sent from the Boost - Dev mailing list archive at Nabble.com.
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 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.
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. [...]
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.
[I like the D-style template argument delimiters!] has_move_emulation_enabled<T> should be identical to boost::is_convertible< T&, boost::rv<T>& >, no? At least, I believe it was in previous versions of Boost.Move... I guess I should actually skim through the documentation and see what, if anything, has changed in Boost.Move! - Jeff
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
On Mon, Jan 9, 2012 at 2:17 PM, Vicente J. Botet Escriba < vicente.botet@wanadoo.fr> wrote: [...]
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?
Looking through the documentation, in short, no I can't find any mention of this limitation. Let me see if I understand it correctly, though. One has the following constructor overloads: template< class F > thread(BOOST_RV_REF( F ) f); template< class F > thread(BOOST_CATCH_CONST_RVALUE( F ) f); // what is BOOST_CATCH_CONST_RVALUE? template< class F > thread(F& f); and wishes thread((CopyableAndMovableFunctor())); to, ideally, bind the temporary functor to an emulated rvalue reference. But I gather the above won't even compile since 1) the template parameter F cannot be deduced in the first 2 constructor overloads (the compiler won't apply conversions); and 2) a temporary cannot bind to the F& overload. If the above is all accurate, then I wonder where the above 3 constructor overloads came from in the first place...? - Jeff
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). -- Evan Wallace
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
Hi Vicente, On Monday, 16. January 2012 19:51:21 Vicente J. Botet Escriba wrote:
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 would have liked separate smaller commits for those. One big one is harder to debug.
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.
Well, it seems my code is now broken with suspicious errors from Boost.TypeTraits: In file included from BOOST_ROOT/boost/chrono/duration.hpp:42:0, from BOOST_ROOT/boost/chrono/time_point.hpp:33, from BOOST_ROOT/boost/thread/locks.hpp:15, from BOOST_ROOT/boost/thread/pthread/mutex.hpp:12, from BOOST_ROOT/boost/thread/mutex.hpp:16, from BOOST_ROOT/boost/pool/detail/mutex.hpp:14, from BOOST_ROOT/boost/pool/poolfwd.hpp:24, from BOOST_ROOT/boost/pool/pool.hpp:27, from myfile.h:18, from myfile.cpp:10: BOOST_ROOT/boost/type_traits/common_type.hpp:121:34: error: ‘declval_b’ is not a type BOOST_ROOT/boost/type_traits/common_type.hpp:121:46: error: expected ‘,’ or ‘...’ before ‘?’ token BOOST_ROOT/boost/type_traits/common_type.hpp:121:73: error: ISO C++ forbids declaration of ‘BOOST_TYPEOF_TPL’ with no type [-fpermissive] BOOST_ROOT/boost/type_traits/common_type.hpp:121:73: error: expected ‘;’ at end of member declaration BOOST_ROOT/boost/type_traits/common_type.hpp:121:75: error: invalid use of template-name ‘boost::type’ without an argument list "myfile.cpp" compiles cleanly with 1.48.0 and trunk before 76543. Any ideas or do I have to build a testcase for this? Yours, Jürgen -- Dipl.-Math. Jürgen Hunold | IVE mbH Software-Entwickler | Lützerodestraße 10 Tel: +49 511 897668 33 | 30161 Hannover, Germany Fax: +49 511 897668 29 | http://www.ivembh.de juergen.hunold@ivembh.de | | Geschäftsführer: Sitz des Unternehmens: Hannover | Univ.-Prof. Dr.-Ing. Thomas Siefer Amtsgericht Hannover, HRB 56965 | PD Dr.-Ing. Alfons Radtke
Hi Vicente, On Tuesday, 17. January 2012 16:19:41 Jürgen Hunold wrote:
Hi Vicente,
Any ideas or do I have to build a testcase for this?
The testcase was easier than expected. BOOST_TYPEOF_SILENT is the culprit. Please find a testcase attached. error.log contains the full output. This code works with 1.48.0 and Boost.Trunk before your fixes. Yours, Jürgen -- Dipl.-Math. Jürgen Hunold | IVE mbH Software-Entwickler | Lützerodestraße 10 Tel: +49 511 897668 33 | 30161 Hannover, Germany Fax: +49 511 897668 29 | http://www.ivembh.de juergen.hunold@ivembh.de | | Geschäftsführer: Sitz des Unternehmens: Hannover | Univ.-Prof. Dr.-Ing. Thomas Siefer Amtsgericht Hannover, HRB 56965 | PD Dr.-Ing. Alfons Radtke
Le 17/01/12 16:19, Jürgen Hunold a écrit :
Hi Vicente,
On Monday, 16. January 2012 19:51:21 Vicente J. Botet Escriba wrote:
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 would have liked separate smaller commits for those. One big one is harder to debug. Hi,
sorry for the disturbance. Yes, I know that this was a too big commit, but as I was cummulated a lot of dev, it was quite complicated to commit them separately.
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. Well, it seems my code is now broken with suspicious errors from Boost.TypeTraits:
In file included from BOOST_ROOT/boost/chrono/duration.hpp:42:0, from BOOST_ROOT/boost/chrono/time_point.hpp:33, from BOOST_ROOT/boost/thread/locks.hpp:15, from BOOST_ROOT/boost/thread/pthread/mutex.hpp:12, from BOOST_ROOT/boost/thread/mutex.hpp:16, from BOOST_ROOT/boost/pool/detail/mutex.hpp:14, from BOOST_ROOT/boost/pool/poolfwd.hpp:24, from BOOST_ROOT/boost/pool/pool.hpp:27, from myfile.h:18, from myfile.cpp:10: BOOST_ROOT/boost/type_traits/common_type.hpp:121:34: error: ‘declval_b’ is not a type BOOST_ROOT/boost/type_traits/common_type.hpp:121:46: error: expected ‘,’ or ‘...’ before ‘?’ token BOOST_ROOT/boost/type_traits/common_type.hpp:121:73: error: ISO C++ forbids declaration of ‘BOOST_TYPEOF_TPL’ with no type [-fpermissive] BOOST_ROOT/boost/type_traits/common_type.hpp:121:73: error: expected ‘;’ at end of member declaration BOOST_ROOT/boost/type_traits/common_type.hpp:121:75: error: invalid use of template-name ‘boost::type’ without an argument list
"myfile.cpp" compiles cleanly with 1.48.0 and trunk before 76543.
Any ideas or do I have to build a testcase for this?
Do you know if the test pass for type_traits/common_type on your configuration? or Boost.Chrono? I suspect that there are some configurations that doesn't support it and I should have to use Boost.Chrono conditionally. BTW, which platform/compiler are you using? Please, could you make a ticket, I will try to introduce the conditional use of Boost.Chrono today. Best, Vicente
Le 17/01/12 19:42, Vicente J. Botet Escriba a écrit :
Le 17/01/12 16:19, Jürgen Hunold a écrit :
Hi Vicente,
On Monday, 16. January 2012 19:51:21 Vicente J. Botet Escriba wrote:
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 would have liked separate smaller commits for those. One big one is harder to debug. Hi,
sorry for the disturbance. Yes, I know that this was a too big commit, but as I was cummulated a lot of dev, it was quite complicated to commit them separately.
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. Well, it seems my code is now broken with suspicious errors from Boost.TypeTraits:
In file included from BOOST_ROOT/boost/chrono/duration.hpp:42:0, from BOOST_ROOT/boost/chrono/time_point.hpp:33, from BOOST_ROOT/boost/thread/locks.hpp:15, from BOOST_ROOT/boost/thread/pthread/mutex.hpp:12, from BOOST_ROOT/boost/thread/mutex.hpp:16, from BOOST_ROOT/boost/pool/detail/mutex.hpp:14, from BOOST_ROOT/boost/pool/poolfwd.hpp:24, from BOOST_ROOT/boost/pool/pool.hpp:27, from myfile.h:18, from myfile.cpp:10: BOOST_ROOT/boost/type_traits/common_type.hpp:121:34: error: ‘declval_b’ is not a type BOOST_ROOT/boost/type_traits/common_type.hpp:121:46: error: expected ‘,’ or ‘...’ before ‘?’ token BOOST_ROOT/boost/type_traits/common_type.hpp:121:73: error: ISO C++ forbids declaration of ‘BOOST_TYPEOF_TPL’ with no type [-fpermissive] BOOST_ROOT/boost/type_traits/common_type.hpp:121:73: error: expected ‘;’ at end of member declaration BOOST_ROOT/boost/type_traits/common_type.hpp:121:75: error: invalid use of template-name ‘boost::type’ without an argument list
"myfile.cpp" compiles cleanly with 1.48.0 and trunk before 76543.
Any ideas or do I have to build a testcase for this?
Do you know if the test pass for type_traits/common_type on your configuration? or Boost.Chrono?
I suspect that there are some configurations that doesn't support it and I should have to use Boost.Chrono conditionally. BTW, which platform/compiler are you using? Please, could you make a ticket, I will try to introduce the conditional use of Boost.Chrono today.
Hi, I have updated Boost.Thread (Committed revision 76570) so that the user can stay that Boost.Thread must not use Boost.Chrono. You need to define BOOST_THREAD_DONT_USE_CHRONO to this effect. Please could you update the trunk, try it and tell me if this is satisfactory for you? Best, Vicente
Hi Vicente. On Wednesday, 18. January 2012 01:14:35 Vicente J. Botet Escriba wrote:
Do you know if the test pass for type_traits/common_type on your configuration? or Boost.Chrono?
I suspect that there are some configurations that doesn't support it and I should have to use Boost.Chrono conditionally. BTW, which platform/compiler are you using? Please, could you make a ticket, I will try to introduce the conditional use of Boost.Chrono today.
The culprit seems to be that we compile with BOOST_TYPEOF_SILENT #defined. Please find the testcase from my second mail attached. I'm working on disabling that in our codebase as it was introduced to eliminate spurious warnings from Boost < 1.45.0
I have updated Boost.Thread (Committed revision 76570) so that the user can stay that Boost.Thread must not use Boost.Chrono. You need to define BOOST_THREAD_DONT_USE_CHRONO to this effect. Please could you update the trunk, try it and tell me if this is satisfactory for you?
Vicente, you are too fast for me ;-)) Removing BOOST_TYPEOF_SILENT already helped. But it seems I have to set BOOST_SYSTEM_NO_DEPRECATED because of: /home/hunold/src/devel/boost/boost/system/error_code.hpp:214: error: undefined reference to 'boost::system::generic_category()' /home/hunold/src/devel/boost/boost/system/error_code.hpp:215: error: undefined reference to 'boost::system::generic_category()' /home/hunold/src/devel/boost/boost/system/error_code.hpp:216: error: undefined reference to 'boost::system::system_category()' To test this, remove BOOST_TYPEOF_SILENT from my testcases Jamroot. That is quite unsatisfactory, but can be handled on our side. The dependency chain of Boost.Pool -> Boost.Thread (Mutex) -> Boost.Chrono / Boost.System makes me wonder if some separate header-only Boost.Mutex library might be advisable. Just my 0.02€ Sorry for the noise. Yours, Jürgen -- Dipl.-Math. Jürgen Hunold | IVE mbH Software-Entwickler | Lützerodestraße 10 Tel: +49 511 897668 33 | 30161 Hannover, Germany Fax: +49 511 897668 29 | http://www.ivembh.de juergen.hunold@ivembh.de | | Geschäftsführer: Sitz des Unternehmens: Hannover | Univ.-Prof. Dr.-Ing. Thomas Siefer Amtsgericht Hannover, HRB 56965 | PD Dr.-Ing. Alfons Radtke
participants (5)
-
Evan Wallace -
Jeffrey Lee Hellrung, Jr. -
Jürgen Hunold -
Vicente Botet -
Vicente J. Botet Escriba