Hi, I was recently trying out Boost.Move and a few issues worth sharing surfaced: 1. It would be helpful to have configuration macros to force emulation mode, even on C++11 compilers, as well as to disable move semantics altogether (that is, the conversion operators to boost::rv& should be disabled, and move/forward should return lvalue-references. BOOST_RV_REF and friends should remain intact, so that overloads remain unique.) In many cases, the higher level semantics of a program are expected to be identical under all three configurations, so having a quick way to switch between them is useful during testing/debugging. 2. Boost.Move is a little bit too opaque, as it stands. What's really missing are Boost.Move-aware type traits. Things like add_rvalue_reference are often necessary to calcuate return types of move-aware generic functions, and so on. Whether this belongs in Boost.TypeTraits or Boost.Move is a separate question. Likewise, there should be type traits to calculate the return types of boost::move and, in particular, boost::forward. On C++11, the return type of forward<T> conincides with add_rvalue_reference<T>, but not so in emulation mode, hence the necessity for this trait. 3. For some reason, the emulated boost::move is written so it doesn't accept temporaries. This doesn't play too nicely with forwarding: Unavoidably on C++03, there has to be made a choice whether forwarding functions use non-const or const-qualified references (bad wording, but you get the point), where the former accepts modifiable lvalues and rejects temporaries, and the latter does the opposite (well, sort of). Boost.Move chooses the latter. This means that while BOOST_FWD_REF parameters accept temporaries, these temporaries are bound as lvalues, which pretty much defeats the purpose of the library (note that these temporaries aren't eligible for copy-elision, since they're bound to a reference.) While there's not much you can do about it on the callee-side, callers could use boost::move on the temporaries to force them into rvalue-refs. Only that they can't. This creates a somewhat confusing situation where forwarding functions are able to take temporaries, but they won't move them, while the only things that can actually be moved are lvalues... Ultimately, this is what I suggest: First of all, have boost::move accept temporaries. Secondly, seriously consider changing the definition of BOOST_FWD_REF(T) from const T& to T&, as this would acheive two desirable, IMO, goals:: a. Forwarding functions would accept modifiable lvalues (and keep them modifiable). b. Forwarding functions would REJECT temporaries, UNLESS they're passed through boost::move, which assures that they're treated as rvalues. A few notes on the implementation: 4. There are many one-liner functions floating around that aren't declared inline. This is a big-deal for less-capable compilers, such as Sun and older versions of GCC. In fact, they simply won't inline boost::move calls without it. 5. boost::rv<T> unconditionally inherits from T, with the assumption that it would never get instanciated for non-class types, since it is only ever used as a reference. This is a false assumption in general. In the context of overload-resolution, the compiler is allowed, though not required, to instanciate the types of function parameters, even if the best overload can be determined without doing this. Consider this move-aware, but not very useful vector class: #include <boost/move/move.hpp> template <typename T> struct vector { void push_back(const T&) {} void push_back(BOOST_RV_REF(T)) {} }; int main() { vector<int> v; v.push_back(123); } The last line of main could potentially instanciate rv<int>, breaking the program. And it does. Sun falls right into this trap. It doesn't take much to make it break on GCC either, for the same reason. Fortunately, the fix is trivial: specialize boost::rv correctly for non-class types. All comments refer to the 1.48 release. I don't know what's going on on trunk. HTH
On Wed, Jan 11, 2012 at 9:29 AM, Dan Ivy <danivy.mail@gmail.com> wrote:
Hi,
I was recently trying out Boost.Move and a few issues worth sharing surfaced:
1. It would be helpful to have configuration macros to force emulation mode, even on C++11 compilers, as well as to disable move semantics altogether (that is, the conversion operators to boost::rv& should be disabled, and move/forward should return lvalue-references. BOOST_RV_REF and friends should remain intact, so that overloads remain unique.) In many cases, the higher level semantics of a program are expected to be identical under all three configurations, so having a quick way to switch between them is useful during testing/debugging.
It could be useful to force emulation mode, I suppose, but disabling move semantics altogether could easily break code that uses moveable-but-noncopyable types. So I question the utility of the latter, at least. 2. Boost.Move is a little bit too opaque, as it stands. What's really
missing are Boost.Move-aware type traits. Things like add_rvalue_reference are often necessary to calcuate return types of move-aware generic functions, and so on. Whether this belongs in Boost.TypeTraits or Boost.Move is a separate question. Likewise, there should be type traits to calculate the return types of boost::move and, in particular, boost::forward. On C++11, the return type of forward<T> conincides with add_rvalue_reference<T>, but not so in emulation mode, hence the necessity for this trait.
+1; perhaps Boost.TypeTraits can update the relevant metafunctions to account for boost::rv<T>&? I think we definitely should have add_reference add_rvalue_reference add_lvalue_reference remove_reference remove_rvalue_reference remove_lvalue_reference is_reference is_rvalue_reference is_lvalue_reference where boost::rv<T>& and (probably) boost::rv<T> const & are treated as T&&.
3. For some reason, the emulated boost::move is written so it doesn't accept temporaries. This doesn't play too nicely with forwarding: Unavoidably on C++03, there has to be made a choice whether forwarding functions use non-const or const-qualified references (bad wording, but you get the point), where the former accepts modifiable lvalues and rejects temporaries, and the latter does the opposite (well, sort of). Boost.Move chooses the latter. This means that while BOOST_FWD_REF parameters accept temporaries, these temporaries are bound as lvalues, which pretty much defeats the purpose of the library (note that these temporaries aren't eligible for copy-elision, since they're bound to a reference.)
I wouldn't use such strong language. It's "just" more limited than if true rvalue references were available. You can still pass explicitly created emulated rvalue references, and the library will allow you to move them. While
there's not much you can do about it on the callee-side, callers could use boost::move on the temporaries to force them into rvalue-refs. Only that they can't. This creates a somewhat confusing situation where forwarding functions are able to take temporaries, but they won't move them, while the only things that can actually be moved are lvalues...
Okay, when you phrase it that way, it is confusing :)
Ultimately, this is what I suggest: First of all, have boost::move accept temporaries.
Using a T const & parameter? I might prefer to remain safe to prevent accidentally moving lvalues-to-const. Secondly, seriously
consider changing the definition of BOOST_FWD_REF(T) from const T& to T&, as this would acheive two desirable, IMO, goals:: a. Forwarding functions would accept modifiable lvalues (and keep them modifiable). b. Forwarding functions would REJECT temporaries, UNLESS they're passed through boost::move, which assures that they're treated as rvalues.
This makes the use of the interface defined using BOOST_FWD_REF more onerous, *even* if true rvalue references are available! So I would consider this inferior to the current solution, even if the current solution is more limited. And, sometimes, you don't even know if the expression you're using as a parameter to a function of said interface is an rvalue or an lvalue (and sometimes it could be either, if this is all in a template). I sympathize with your goal, though. I've actually played around with various modifications of Boost.Move during its development process, and I made some suggestions for additions to Boost.Move which were ultimately rejected to keep the library simple (which is totally understandable). Two such suggestions were: - An additional macro BOOST_FWD2_REF( T ) which expanded to T& (in emulation mode) and would be used only to capture lvalues or explicitly created (emulated) rvalue references. This included the result of boost::forward. - An additional macro BOOST_MOVE( expr ) which expanded to an expression nearly functionally identical to expr except it would explicitly cast the result to an emulated rvalue reference if expr was an rvalue of moveable type.
A few notes on the implementation:
4. There are many one-liner functions floating around that aren't declared inline. This is a big-deal for less-capable compilers, such as Sun and older versions of GCC. In fact, they simply won't inline boost::move calls without it.
Sounds like you should file trac tickets and, possibly, patches. 5. boost::rv<T> unconditionally inherits from T, with the assumption
that it would never get instanciated for non-class types, since it is only ever used as a reference. This is a false assumption in general. In the context of overload-resolution, the compiler is allowed, though not required, to instanciate the types of function parameters, even if the best overload can be determined without doing this. Consider this move-aware, but not very useful vector class:
#include <boost/move/move.hpp>
template <typename T> struct vector { void push_back(const T&) {} void push_back(BOOST_RV_REF(T)) {} };
int main() { vector<int> v; v.push_back(123); }
The last line of main could potentially instanciate rv<int>, breaking the program. And it does. Sun falls right into this trap. It doesn't take much to make it break on GCC either, for the same reason. Fortunately, the fix is trivial: specialize boost::rv correctly for non-class types.
Hmmm...again, probably best to file a trac ticket, with patch if possible.
All comments refer to the 1.48 release. I don't know what's going on on trunk.
I haven't check recently, either. - Jeff
On Thu, Jan 12, 2012 at 8:40 AM, Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung@gmail.com> wrote:
On Wed, Jan 11, 2012 at 9:29 AM, Dan Ivy <danivy.mail@gmail.com> wrote:
Hi,
I was recently trying out Boost.Move and a few issues worth sharing surfaced:
1. It would be helpful to have configuration macros to force emulation mode, even on C++11 compilers, as well as to disable move semantics altogether (that is, the conversion operators to boost::rv& should be disabled, and move/forward should return lvalue-references. BOOST_RV_REF and friends should remain intact, so that overloads remain unique.) In many cases, the higher level semantics of a program are expected to be identical under all three configurations, so having a quick way to switch between them is useful during testing/debugging.
It could be useful to force emulation mode, I suppose, but disabling move semantics altogether could easily break code that uses moveable-but-noncopyable types. So I question the utility of the latter, at least.
Forcing emulation mode is useful for people that primarily use compilers where true rvalue-rferences can't be disabled, such as MSVC 10+. It wouldn't set a precedence, anyway. Boost.TypeOf, for example, already has such a macro, for similiar reasons, I suppose. As for disabling move-semantics, I don't feel too strongly about this, and personally never really needed it, yet, but I forsee it as coming in handy at times. You are correct that it will break some code, this is what I was refering to by "in many cases", but move-indifferent code is probably the more common one (after all, rvalue-refs are a relatively recent feature.) I don't think it's too harmful to just have it there, anyway.
2. Boost.Move is a little bit too opaque, as it stands. What's really
missing are Boost.Move-aware type traits. Things like add_rvalue_reference are often necessary to calcuate return types of move-aware generic functions, and so on. Whether this belongs in Boost.TypeTraits or Boost.Move is a separate question. Likewise, there should be type traits to calculate the return types of boost::move and, in particular, boost::forward. On C++11, the return type of forward<T> conincides with add_rvalue_reference<T>, but not so in emulation mode, hence the necessity for this trait.
+1; perhaps Boost.TypeTraits can update the relevant metafunctions to account for boost::rv<T>&? I think we definitely should have
add_reference add_rvalue_reference add_lvalue_reference remove_reference remove_rvalue_reference remove_lvalue_reference is_reference is_rvalue_reference is_lvalue_reference
where boost::rv<T>& and (probably) boost::rv<T> const & are treated as T&&.
One thing I forgot to bring up is how these traits shoud handle types without move emulation. While it would be nice for the result of add_rvalue_reference to be aligned with the return types of boost::move and boost::forward, that is, const T&, for non-emulated types (which also does the reference-collapsing trick), it wouldn't play very nicely with the rest of the traits. (i.e. add_rvalue_ref<T> is const T&, is is_rvalue_ref<const T&> true?) This is another good reason to provide separate traits to calculate the return types of move and forward.
3. For some reason, the emulated boost::move is written so it doesn't accept temporaries. This doesn't play too nicely with forwarding: Unavoidably on C++03, there has to be made a choice whether forwarding functions use non-const or const-qualified references (bad wording, but you get the point), where the former accepts modifiable lvalues and rejects temporaries, and the latter does the opposite (well, sort of). Boost.Move chooses the latter. This means that while BOOST_FWD_REF parameters accept temporaries, these temporaries are bound as lvalues, which pretty much defeats the purpose of the library (note that these temporaries aren't eligible for copy-elision, since they're bound to a reference.)
I wouldn't use such strong language. It's "just" more limited than if true rvalue references were available. You can still pass explicitly created emulated rvalue references, and the library will allow you to move them.
It's not "just" more limited. It's very likely, the way things are now, that someone would pass a "raw" (read un-moved) temporary to a forwarding function, unaware that it wouldn't be moved (because things "just work"). It's even more likely that having been aware of that, he would prefer passing the argument through move(), as ugly as it may be. This is how I see it, anyway. And, as said, boost::move doesn't accept temporaries, so you'll have to make a lvalue out of your temporary, and only then move it. Aside from being even uglier than using move() directly, how would you do that in initializer-lists, for example? (Well, you could use some move_temp, or move(temp()) functions, but, you know, how is this any better?)
While
there's not much you can do about it on the callee-side, callers could use boost::move on the temporaries to force them into rvalue-refs. Only that they can't. This creates a somewhat confusing situation where forwarding functions are able to take temporaries, but they won't move them, while the only things that can actually be moved are lvalues...
Okay, when you phrase it that way, it is confusing :)
Ultimately, this is what I suggest: First of all, have boost::move accept temporaries.
Using a T const & parameter? I might prefer to remain safe to prevent accidentally moving lvalues-to-const.
Agreed. I overlooked this.
Secondly, seriously
consider changing the definition of BOOST_FWD_REF(T) from const T& to T&, as this would acheive two desirable, IMO, goals:: a. Forwarding functions would accept modifiable lvalues (and keep them modifiable). b. Forwarding functions would REJECT temporaries, UNLESS they're passed through boost::move, which assures that they're treated as rvalues.
This makes the use of the interface defined using BOOST_FWD_REF more onerous, *even* if true rvalue references are available! So I would consider this inferior to the current solution, even if the current solution is more limited. And, sometimes, you don't even know if the expression you're using as a parameter to a function of said interface is an rvalue or an lvalue (and sometimes it could be either, if this is all in a template).
Yes it's uglier (but not THAT ugly). Tough luck, portability comes at a price. I'm not sure what you mean by "even if true rvalue references are avialable"? The change I propose only has to do with emulation. On C++11, FWD_REFs are still T&&, so if you're using C++11 exclusively, no harm done, you can pass temporaries as usual. The only difference is that once you switch to emulation mode, you'll get errors for those cases, instead of unknowingly regressing to copy-semantics. And, as you said yourself, some types are movable-but-non-copyable. So uncautious passing of temporaries can already compile fine on C++11 and break on emulation. As for the the second point, of not knowing the l/rvalue-ness of an expression, I agree. Again, overlooked.
I sympathize with your goal, though. I've actually played around with various modifications of Boost.Move during its development process, and I made some suggestions for additions to Boost.Move which were ultimately rejected to keep the library simple (which is totally understandable). Two such suggestions were:
- An additional macro BOOST_FWD2_REF( T ) which expanded to T& (in emulation mode) and would be used only to capture lvalues or explicitly created (emulated) rvalue references. This included the result of boost::forward.
I though about suggesting something like this, but came to the same conclusion that it overly-complicates stuff. You really don't want to have different types of forwarding functions, and having to know which is which. It's best to have a single one, however unpleasent the consequences are.
- An additional macro BOOST_MOVE( expr ) which expanded to an expression nearly functionally identical to expr except it would explicitly cast the result to an emulated rvalue reference if expr was an rvalue of moveable type.
+1; I see where you're going with this. I think that, given the issues you noted with boost::move taking a reference-to-const, and not knowing the r/l-ness of an expression ahead of time, it's a good solution. Why was it rejected?
A few notes on the implementation:
4. There are many one-liner functions floating around that aren't declared inline. This is a big-deal for less-capable compilers, such as Sun and older versions of GCC. In fact, they simply won't inline boost::move calls without it.
Sounds like you should file trac tickets and, possibly, patches.
5. boost::rv<T> unconditionally inherits from T, with the assumption
that it would never get instanciated for non-class types, since it is only ever used as a reference. This is a false assumption in general. In the context of overload-resolution, the compiler is allowed, though not required, to instanciate the types of function parameters, even if the best overload can be determined without doing this. Consider this move-aware, but not very useful vector class:
#include <boost/move/move.hpp>
template <typename T> struct vector { void push_back(const T&) {} void push_back(BOOST_RV_REF(T)) {} };
int main() { vector<int> v; v.push_back(123); }
The last line of main could potentially instanciate rv<int>, breaking the program. And it does. Sun falls right into this trap. It doesn't take much to make it break on GCC either, for the same reason. Fortunately, the fix is trivial: specialize boost::rv correctly for non-class types.
Hmmm...again, probably best to file a trac ticket, with patch if possible.
All comments refer to the 1.48 release. I don't know what's going on on trunk. .
I haven't check recently, either.
- Jeff
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
I will file the tickets sometime. Not sure I'm gonna have time to write patches, though
On Thu, Jan 12, 2012 at 5:23 AM, Dan Ivy <danivy.mail@gmail.com> wrote:
On Thu, Jan 12, 2012 at 8:40 AM, Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung@gmail.com> wrote:
On Wed, Jan 11, 2012 at 9:29 AM, Dan Ivy <danivy.mail@gmail.com> wrote:
Hi,
I was recently trying out Boost.Move and a few issues worth sharing surfaced:
1. It would be helpful to have configuration macros to force emulation mode, even on C++11 compilers, as well as to disable move semantics altogether (that is, the conversion operators to boost::rv& should be disabled, and move/forward should return lvalue-references.
BOOST_RV_REF
and friends should remain intact, so that overloads remain unique.) In many cases, the higher level semantics of a program are expected to be identical under all three configurations, so having a quick way to switch between them is useful during testing/debugging.
It could be useful to force emulation mode, I suppose, but disabling move semantics altogether could easily break code that uses moveable-but-noncopyable types. So I question the utility of the latter, at least.
Forcing emulation mode is useful for people that primarily use compilers where true rvalue-rferences can't be disabled, such as MSVC 10+. It wouldn't set a precedence, anyway. Boost.TypeOf, for example, already has such a macro, for similiar reasons, I suppose. As for disabling move-semantics, I don't feel too strongly about this, and personally never really needed it, yet, but I forsee it as coming in handy at times. You are correct that it will break some code, this is what I was refering to by "in many cases", but move-indifferent code is probably the more common one (after all, rvalue-refs are a relatively recent feature.) I don't think it's too harmful to just have it there, anyway.
I will see what Ion thinks concerning forcing emulation mode. Having thought about it a little more, it could help to test that code developed for C++11 also works in C++03, so I wouldn't be opposed to such a switch. And agreed regarding Boost.TypeOf. Regarding disabling move semantics, again, I fail to see the utility of such a global switch. Such a switch will change the semantics of the code on a global scale. What's wrong with using your own local switch to disable move semantics for just those classes you're interested in?
missing are Boost.Move-aware type traits. Things like add_rvalue_reference are often necessary to calcuate return types of move-aware generic functions, and so on. Whether this belongs in Boost.TypeTraits or Boost.Move is a separate question. Likewise, there should be type
2. Boost.Move is a little bit too opaque, as it stands. What's really traits to
calculate the return types of boost::move and, in particular, boost::forward. On C++11, the return type of forward<T> conincides with add_rvalue_reference<T>, but not so in emulation mode, hence the necessity for this trait.
+1; perhaps Boost.TypeTraits can update the relevant metafunctions to account for boost::rv<T>&? I think we definitely should have
add_reference add_rvalue_reference add_lvalue_reference remove_reference remove_rvalue_reference remove_lvalue_reference is_reference is_rvalue_reference is_lvalue_reference
where boost::rv<T>& and (probably) boost::rv<T> const & are treated as T&&.
One thing I forgot to bring up is how these traits shoud handle types without move emulation. While it would be nice for the result of add_rvalue_reference to be aligned with the return types of boost::move and boost::forward, that is, const T&, for non-emulated types (which also does the reference-collapsing trick), it wouldn't play very nicely with the rest of the traits. (i.e. add_rvalue_ref<T> is const T&, is is_rvalue_ref<const T&> true?) This is another good reason to provide separate traits to calculate the return types of move and forward.
Agreed. So add to the list result_of::move and result_of::forward.
3. For some reason, the emulated boost::move is written so it doesn't accept temporaries. This doesn't play too nicely with forwarding: Unavoidably on C++03, there has to be made a choice whether forwarding functions use non-const or const-qualified references (bad wording, but you get the point), where the former accepts modifiable lvalues and rejects temporaries, and the latter does the opposite (well, sort of). Boost.Move chooses the latter. This means that while BOOST_FWD_REF parameters accept temporaries, these temporaries are bound as lvalues, which pretty much defeats the purpose of the library (note that these temporaries aren't eligible for copy-elision, since they're bound to a reference.)
I wouldn't use such strong language. It's "just" more limited than if true rvalue references were available. You can still pass explicitly created emulated rvalue references, and the library will allow you to move them.
It's not "just" more limited. It's very likely, the way things are now, that someone would pass a "raw" (read un-moved) temporary to a forwarding function, unaware that it wouldn't be moved (because things "just work"). It's even more likely that having been aware of that, he would prefer passing the argument through move(), as ugly as it may be. This is how I see it, anyway. And, as said, boost::move doesn't accept temporaries, so you'll have to make a lvalue out of your temporary, and only then move it. Aside from being even uglier than using move() directly, how would you do that in initializer-lists, for example? (Well, you could use some move_temp, or move(temp()) functions, but, you know, how is this any better?)
All fair criticisms. However, I think this is the simplest solution. If BOOST_FWD_REF( T ) expanded to T&, it wouldn't bind to temporaries, making the call expression ugly, and unnecessarily so in the cases that true rvalue references are available or the type isn't even movable. In any event, the current forwarding solution is no worse than what we had before Boost.Move, so we aren't regressing in any way. If the caller really wanted to ensure their object got moved, there is a way to do that (bind the temporary and explicitly move it, or use a macro like BOOST_MOVE below); if the callee wanted to be particularly helpful, it would offer more overloads to accurately capture moveable temporaries (this can require up to 4 or more overloads and SFINAE, but it's possible). Let's not burden the caller of every call to a BOOST_FWD_REF( T ) parameter. [...]
Ultimately, this is what I suggest:
[...]
Secondly, seriously
consider changing the definition of BOOST_FWD_REF(T) from const T& to T&, as this would acheive two desirable, IMO, goals:: a. Forwarding functions would accept modifiable lvalues (and keep them modifiable). b. Forwarding functions would REJECT temporaries, UNLESS they're passed through boost::move, which assures that they're treated as rvalues.
This makes the use of the interface defined using BOOST_FWD_REF more onerous, *even* if true rvalue references are available! So I would consider this inferior to the current solution, even if the current solution is more limited. And, sometimes, you don't even know if the expression you're using as a parameter to a function of said interface is an rvalue or an lvalue (and sometimes it could be either, if this is all in a template).
Yes it's uglier (but not THAT ugly). Tough luck, portability comes at a price. I'm not sure what you mean by "even if true rvalue references are avialable"? The change I propose only has to do with emulation.
But the call expression doesn't change whether emulation is enabled or not. So you need to uglify the call expression solely to address emulation shortcomings. I don't think that's bad. Better to uglify the interface implementation. [...]
I sympathize with your goal, though. I've actually played around with various modifications of Boost.Move during its development process, and I made some suggestions for additions to Boost.Move which were ultimately rejected to keep the library simple (which is totally understandable). Two such suggestions were:
- An additional macro BOOST_FWD2_REF( T ) which expanded to T& (in emulation mode) and would be used only to capture lvalues or explicitly created (emulated) rvalue references. This included the result of boost::forward.
I though about suggesting something like this, but came to the same conclusion that it overly-complicates stuff. You really don't want to have different types of forwarding functions, and having to know which is which. It's best to have a single one, however unpleasent the consequences are.
In truth, I typically forego BOOST_FWD_REF altogether and just write different overload sets for "outward-looking" interfaces depending on BOOST_NO_RVALUE_REFERENCES. In which case BOOST_FWD2_REF is what I end up using more often.
- An additional macro BOOST_MOVE( expr ) which expanded to an expression
nearly functionally identical to expr except it would explicitly cast the result to an emulated rvalue reference if expr was an rvalue of moveable type.
+1; I see where you're going with this. I think that, given the issues you noted with boost::move taking a reference-to-const, and not knowing the r/l-ness of an expression ahead of time, it's a good solution. Why was it rejected?
I didn't push for it, merely suggested it. Perhaps Ion can address what he thought of the idea. Maybe he didn't see the utility of such a macro. - Jeff
On Thu, Jan 12, 2012 at 9:16 PM, Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung@gmail.com> wrote:
All fair criticisms. However, I think this is the simplest solution. If BOOST_FWD_REF( T ) expanded to T&, it wouldn't bind to temporaries, making the call expression ugly, and unnecessarily so in the cases that true rvalue references are available or the type isn't even movable. In any event, the current forwarding solution is no worse than what we had before Boost.Move, so we aren't regressing in any way. If the caller really wanted to ensure their object got moved, there is a way to do that (bind the temporary and explicitly move it, or use a macro like BOOST_MOVE below);
I'd hate to repeat myself again, so this is the last time, I promise :) I think we misunderstand each other, or at least I don't understand you completely. You keep talking about "cases where true rvalue-references are avaiable", but I don't see what you mean by that? The purpose of Boost.Move is to fill in the gaps when true rv-refs aren't available, so code using Boost.Move is, in general, unaware of whether or not true rv-refs are avaiable. I thoght you were talking about cases where the library writer uses Boost.Move for portability, but the end-user is using a C++11 compiler exclusively, and couldn't care less about these sort of things, and now he has to uglify his code just for the sake of Boost.Move's shenanigans. But, as I said, this is not the case. On C++11, BOOST_FWD_REF(T) is still T&& (&&!) and would happily accept, and move, temporaries. The only issue is about cases where protability matters. While I prefer uglier but movable temporaries, you prefer (I think) neater but possibly non-moveable ones. That's cool. The regression I was talking about is when the aforementioned C++11 enthusiastics suddenly need to compile their code on a C++03 compiler, I belive they would be upset that their program's efficency is compronised, without being given any hint about this during compilation (true, this is how it is right now. It doesn't mean that it HAS to be like that.), while you think that they'll be happy that their code compiles and runs. Again, both are valid opinions. if
the callee wanted to be particularly helpful, it would offer more overloads to accurately capture moveable temporaries (this can require up to 4 or more overloads and SFINAE, but it's possible).
Interesting. Could you provide a reference or an example?
[...]
In truth, I typically forego BOOST_FWD_REF altogether and just write different overload sets for "outward-looking" interfaces depending on BOOST_NO_RVALUE_REFERENCES. In which case BOOST_FWD2_REF is what I end up using more often.
So, it seems that you're already using the same thing I suggest...
On Thu, Jan 12, 2012 at 1:18 PM, Dan Ivy <danivy.mail@gmail.com> wrote:
On Thu, Jan 12, 2012 at 9:16 PM, Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung@gmail.com> wrote:
All fair criticisms. However, I think this is the simplest solution. If BOOST_FWD_REF( T ) expanded to T&, it wouldn't bind to temporaries, making the call expression ugly, and unnecessarily so in the cases that true rvalue references are available or the type isn't even movable. In any event, the current forwarding solution is no worse than what we had before Boost.Move, so we aren't regressing in any way. If the caller really wanted to ensure their object got moved, there is a way to do that (bind the temporary and explicitly move it, or use a macro like BOOST_MOVE below);
I'd hate to repeat myself again, so this is the last time, I promise :)
Sorry! I'm probably not explaining things clearly. I think we misunderstand each other, or at least I don't understand
you completely. You keep talking about "cases where true rvalue-references are avaiable", but I don't see what you mean by that?
[...]
While I prefer uglier but movable temporaries, you prefer (I think) neater but possibly non-moveable ones. That's cool. The regression I was talking about is when the aforementioned C++11 enthusiastics suddenly need to compile their code on a C++03 compiler, I belive they would be upset that their program's efficency is compronised, without being given any hint about this during compilation (true, this is how it is right now. It doesn't mean that it HAS to be like that.), while you think that they'll be happy that their code compiles and runs. Again, both are valid opinions.
Here's my position: - Ideally, client code written for both C++11 and C++03 should not have "ugly" workarounds for C++03 deficiencies, and that's basically what wrapping all function parameters in some BOOST_MOVE macro (or similar) would be, a workaround. - If you really want to guarantee C++11-level performance in C++03 and the interface can't otherwise guarantee this, and this necessitates some kind of workaround, then one workaround (wrapping in a BOOST_MOVE macro) is just about as bad as another (storing a temporary and explicitly moving). In any case, even if we did end up agreeing that wrapping function parameters in a BOOST_MOVE macro was desireable, this doesn't entirely preclude BOOST_FWD_REF( T ) from expanding to T const &; moveable temporaries would still be moved. The loss is that const-ness of lvalues is not preserved, but that isn't a common case. Ultimately, I believe the best solution is to just implement the interface in 2 different ways, depending on the presence or absence of true rvalue references. if
the callee wanted to be particularly helpful, it would offer more overloads to accurately capture moveable temporaries (this can require up to 4 or more overloads and SFINAE, but it's possible).
Interesting. Could you provide a reference or an example?
Here's the idea. In C++11, you can just get away with template< class T > void push_back(T&& x) { /*...*/ forward<T>(x) /*...*/ } while in C++03, the best simulation of the above behavior I've found is something like the following. // captures lvalue-references-to-non-const, lvalue references of moveable types, and emulated rvalue references template< class T > void push_back(T&); // if value_type is moveable, captures value_type temporaries or explicitly created emulated value_type rvalue_references // otherwise, captures value_type lvalues and rvalues typedef call_traits< typename add_rvalue_reference< value_type >::type
::param_type value_rvalue_param_type; void push_back(value_rvalue_param_type);
// captures any rvalue of moveable type (except of type value_type) typedef rv_sink< [*] > value_rv_sink_type; void push_back(rv_sink_type); // captures any non-moveable temporaries template< class T > typename boost::enable_if< [**] >::type push_back(T const &); For [*], see http://lists.boost.org/Archives/boost/2011/04/180064.php (there rv_sink is named genrv) (warning: it's a long read) For [**], this overload should be disabled if T is convertible to value_rv_sink_type or value_rvalue_param_type (thus preferring one of the previous overloads). I didn't want to complicate the presentation too much by writing that out in MPL gobbledeegook. Yes, this is a lot of boilerplate on the implementation side, but it should typically be paid back many times over via a simpler call interface. I wouldn't even consider the rv_sink overload to be strictly necessary if you want to relieve yourself of too much implementation burden.
[...]
In truth, I typically forego BOOST_FWD_REF altogether and just write different overload sets for "outward-looking" interfaces depending on BOOST_NO_RVALUE_REFERENCES. In which case BOOST_FWD2_REF is what I end up using more often.
So, it seems that you're already using the same thing I suggest...
Well, any function using BOOST_FWD2_REF in its parameter list is only passed parameters that are lvalues or the result of a boost::forward. So, I guess I'm using the same thing, but it doesn't sound like it's in the same context...? - Jeff
On Fri, Jan 13, 2012 at 1:04 AM, Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung@gmail.com> wrote:
Here's my position: - Ideally, client code written for both C++11 and C++03 should not have "ugly" workarounds for C++03 deficiencies, and that's basically what wrapping all function parameters in some BOOST_MOVE macro (or similar) would be, a workaround.
No argument here.
- If you really want to guarantee C++11-level performance in C++03 and the interface can't otherwise guarantee this, and this necessitates some kind of workaround, then one workaround (wrapping in a BOOST_MOVE macro) is just about as bad as another (storing a temporary and explicitly moving).
Allow me to disagree. I think one altenative is way better than the other, as it doesn't force you to spread your call expression over multiple statements, which is, aside from being obscure (BOOST_MOVE is ugly, but pretty straight-forward), is not applicable in some contexts (i,e, initializer lists).
In any case, even if we did end up agreeing that wrapping function parameters in a BOOST_MOVE macro was desireable, this doesn't entirely preclude BOOST_FWD_REF( T ) from expanding to T const &;
That's right, these are two separate things. moveable
temporaries would still be moved. The loss is that const-ness of lvalues is not preserved, but that isn't a common case.
Not too common, but far from being a corner case either.
[...]
Interesting. Could you provide a reference or an example?
Here's the idea. In C++11, you can just get away with [...]
Thanks for expanding. That's a clever trick :). I thought you were taking about something a bit different though, as this method doesn't scale to functions taking more than a single parameter (it's theoretically possible, but not practical), so it's not a general solution to forwarding. Btw, if that's all you're trying to do, won't something like this be a lot simpler? #include <iostream> #include <boost/type_traits/is_const.hpp> #include <boost/type_traits/is_convertible.hpp> #include <boost/utility/enable_if.hpp> template <typename T> struct movable {}; struct i_can_move : movable<i_can_move> {}; template <typename T> void what_is(T&) { if(!boost::is_const<T>::value) std::cout << "modifiable lvalue" << std::endl; else std::cout << "const lvalue" << std::endl; } template <typename T> typename boost::disable_if< boost::is_convertible<T, movable<T> >
::type what_is(const T&) { std::cout << "non-movable const-lvalue " "or rvalue" << std::endl; }
template <typename T> void what_is(const movable<T>&) { std::cout << "rvalue" << std::endl; // No indirect call, and we get to know what T is } int main() { { i_can_move x; const i_can_move y = x; what_is(x); what_is(y); what_is(i_can_move()); } std::cout << std::endl; { int x; const int y = 123; what_is(x); what_is(y); what_is(123); } }
[...]
So, it seems that you're already using the same thing I suggest...
Well, any function using BOOST_FWD2_REF in its parameter list is only passed parameters that are lvalues or the result of a boost::forward. So, I guess I'm using the same thing, but it doesn't sound like it's in the same context...?
But since we only have one BOOST_FWD_REF, I belive it should be applicable to any context (and you belive that it should be "optimized" for the most common one, point is clear, let's not open that again ;) )
On Fri, Jan 13, 2012 at 7:13 AM, Dan Ivy <danivy.mail@gmail.com> wrote:
On Fri, Jan 13, 2012 at 1:04 AM, Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung@gmail.com> wrote:
[...]
Interesting. Could you provide a reference or an example?
Here's the idea. In C++11, you can just get away with [...]
Thanks for expanding. That's a clever trick :). I thought you were taking about something a bit different though, as this method doesn't scale to functions taking more than a single parameter (it's theoretically possible, but not practical), so it's not a general solution to forwarding.
No, only really practical for 1 or (maybe) 2 function parameters :( Btw, if that's all you're trying to do, won't something like this be a
lot simpler?
[...snip inheriting from movable<T>...] Hmmm...indeed! Definitely a superior solution functionality-wise. The only drawback I can think of is that it can't be easily added to the existing Boost.Move move-enabling macros :( And I can't really think of a good way to hide sometimes inheriting from movable<T> (depending on the presence or absence of rvalue references) within some kind of macro framework. But this might be something I will have to look into further...
So, it seems that you're already using the same thing I suggest...
Well, any function using BOOST_FWD2_REF in its parameter list is only passed parameters that are lvalues or the result of a boost::forward. So, I guess I'm using the same thing, but it doesn't sound like it's in the same context...?
But since we only have one BOOST_FWD_REF, I belive it should be applicable to any context (and you belive that it should be "optimized" for the most common one, point is clear, let's not open that again ;) )
Hey, be my guest trying to convince Ion, he's the ultimate authority :) - Jeff
Dan Ivy wrote:
Ultimately, this is what I suggest: First of all, have boost::move accept temporaries. Secondly, seriously consider changing the definition of BOOST_FWD_REF(T) from const T& to T&, as this would acheive two desirable, IMO, goals:: a. Forwarding functions would accept modifiable lvalues (and keep them modifiable). b. Forwarding functions would REJECT temporaries, UNLESS they're passed through boost::move, which assures that they're treated as rvalues.
Especially your point "a" reveals a different understanding of "boost::forward" and "BOOST_FWD_REF" than I gained during the review. My understanding was that the sole purpose of "boost::forward" and "BOOST_FWD_REF" is to enable "Constructor Forwarding" (see <http://www.boost.org/doc/libs/1_48_0/doc/html/move/construct_forwarding.html>). So the purpose of "boost::forward" is not to emulate "std::forward" for more general use cases. The observation that Boost.Move doesn't play well enough with temporaries is important. It should at least be explained in the Boost.Move documentation. Whether it can/should be fixed or improved is a different question. Regards, Thomas
On Thu, Jan 12, 2012 at 2:50 PM, Thomas Klimpel <Thomas.Klimpel@synopsys.com> wrote:
Dan Ivy wrote:
Ultimately, this is what I suggest: First of all, have boost::move accept temporaries. Secondly, seriously consider changing the definition of BOOST_FWD_REF(T) from const T& to T&, as this would acheive two desirable, IMO, goals:: a. Forwarding functions would accept modifiable lvalues (and keep them modifiable). b. Forwarding functions would REJECT temporaries, UNLESS they're passed through boost::move, which assures that they're treated as rvalues.
Especially your point "a" reveals a different understanding of "boost::forward" and "BOOST_FWD_REF" than I gained during the review. My understanding was that the sole purpose of "boost::forward" and "BOOST_FWD_REF" is to enable "Constructor Forwarding" (see <http://www.boost.org/doc/libs/1_48_0/doc/html/move/construct_forwarding.html>). So the purpose of "boost::forward" is not to emulate "std::forward" for more general use cases.
I don't see it. How is "constructor forwarding" any different than "more general forwarding" in that respect, or, in any other respect?
Dan Ivy wrote:
"Constructor Forwarding" (see <http://www.boost.org/doc/libs/1_48_0/doc/html/move/construct_forwarding.html> ). So the purpose of "boost::forward" is not to emulate "std::forward" for more general use cases.
I don't see it. How is "constructor forwarding" any different than "more general forwarding" in that respect, or, in any other respect?
Well, the above documentation says "Fortunately, most constructors take arguments by value, by const-reference or by rvalue reference. If these limitations are accepted, ...". Because perfect emulation of "std::forward" is not possible, the above reasoning allows to exclude the case of "non-const reference" arguments. For an arbitrary function, excluding the case of "non-const reference" arguments would be a total no-go. Regards, Thomas
On Thu, Jan 12, 2012 at 5:13 PM, Thomas Klimpel <Thomas.Klimpel@synopsys.com> wrote:
Dan Ivy wrote:
"Constructor Forwarding" (see <http://www.boost.org/doc/libs/1_48_0/doc/html/move/construct_forwarding.html> ). So the purpose of "boost::forward" is not to emulate "std::forward" for more general use cases.
I don't see it. How is "constructor forwarding" any different than "more general forwarding" in that respect, or, in any other respect?
Well, the above documentation says "Fortunately, most constructors take arguments by value, by const-reference or by rvalue reference. If these limitations are accepted, ...".
Because perfect emulation of "std::forward" is not possible, the above reasoning allows to exclude the case of "non-const reference" arguments. For an arbitrary function, excluding the case of "non-const reference" arguments would be a total no-go.
Regards, Thomas
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Ok, I see."Constructor" forwarding in constrast to "perfect" forwarding. So "constructor" is an ajective describing the ""quality"" of forwarding and not the kind of forwarding. This wasn't immediatly obvious. FWIW I think it's a confusing term.
From: Dan Ivy
Ok, I see."Constructor" forwarding in constrast to "perfect" forwarding. So "constructor" is an ajective describing the ""quality"" of forwarding and not the kind of forwarding. This wasn't immediatly obvious. FWIW I think it's a confusing term.
No, "constructor" forwarding describes forwarding arguments of a object constructor function call specifically, as opposed to forwarding in the general case. It is only confusing because you are over thinking it. Regards, Luke
El 11/01/2012 18:29, Dan Ivy escribió:
Hi,
I was recently trying out Boost.Move and a few issues worth sharing surfaced:
Hi, sorry if my replies are not detailed enough, but lately I have no much time to read the mailing list and might have missed some points.
1. It would be helpful to have configuration macros to force emulation mode, even on C++11 compilers, as well as to disable move semantics altogether (that is, the conversion operators to boost::rv& should be disabled, and move/forward should return lvalue-references. BOOST_RV_REF and friends should remain intact, so that overloads remain unique.) In many cases, the higher level semantics of a program are expected to be identical under all three configurations, so having a quick way to switch between them is useful during testing/debugging.
Forcing emulation would be easy, if you think it's useful, please fill a ticket so that I can remember that for the new version. Disabling move semantics it's a bit extrange I don't think it should be added without a good reason.
2. Boost.Move is a little bit too opaque, as it stands. What's really missing are Boost.Move-aware type traits. Things like add_rvalue_reference are often necessary to calcuate return types of move-aware generic functions, and so on. Whether this belongs in Boost.TypeTraits or Boost.Move is a separate question. Likewise, there should be type traits to calculate the return types of boost::move and, in particular, boost::forward. On C++11, the return type of forward<T> conincides with add_rvalue_reference<T>, but not so in emulation mode, hence the necessity for this trait.
I agree. I added in trunk (and 1.49) some utilities (based on Jeffrey's code) in the detail namespace and started using them on some other libraries (intrusive, container): is_rvalue_reference add_rvalue_reference remove_rvalue_reference declval() They are experimental, any contribution/test/comment is welcome.
3. For some reason, the emulated boost::move is written so it doesn't accept temporaries. This doesn't play too nicely with forwarding:
The main use case I considered for BOOST_FWD_REF were constructors and emplace functions. I needed them extensively in Boost.Container so maybe I was biased.
4. There are many one-liner functions floating around that aren't declared inline. This is a big-deal for less-capable compilers, such as Sun and older versions of GCC. In fact, they simply won't inline boost::move calls without it.
Ok, thanks for the ticket.
5. boost::rv<T> unconditionally inherits from T, with the assumption that it would never get instanciated for non-class types, since it is only ever used as a reference. This is a false assumption in general. In the context of overload-resolution, the compiler is allowed, though not required, to instanciate the types of function parameters, even if the best overload can be determined without doing this. Consider this move-aware, but not very useful vector class:
Ok, thanks for the ticket. Ion
2012/1/13 Ion Gaztañaga <igaztanaga@gmail.com>:
El 11/01/2012 18:29, Dan Ivy escribió: Forcing emulation would be easy, if you think it's useful, please fill a ticket so that I can remember that for the new version. Disabling move semantics it's a bit extrange I don't think it should be added without a good reason.
Forcing move emulation is something I personally found useful, and it seems to be *almost* as simple adding a || defined (BOOST_MOVE_USE_EMULATION). I am pretty indifferent about disabling move semantics, it's just something I though would be useful, so feel free to discard it. The scenario I had in mind is something like this: You recently added move ctors to some of your classes, and not long after things start to go wrong. You suspect that the newly added move ctors have something to do with it (after all, move constructors are usually pretty delicate), so you disable move semantics and see if things go back to normal. If they do, you know where to start looking for errors. Move semantics are, for the most part, an optimization (I can hear Jeffrey grunts :) ), and it's your reponsibility to get this optimization right. Just like compilers let you disable optimizations (and compiler-writers probably do this once or twice when testing new stuff), it seemed logical to me to be able to disable moving. But that's just a thought.
participants (5)
-
Dan Ivy -
Ion Gaztañaga -
Jeffrey Lee Hellrung, Jr. -
Simonson, Lucanus J -
Thomas Klimpel