Fwd: Re: [move] You can do it better: implementing push_back and handling implicit conversions
[My original reply went directly to Ion, due to me being cc'ed in the original email. Hence forwarding... (pun intended)] -------- Original Message -------- Subject: Re: [move] You can do it better: implementing push_back and handling implicit conversions Date: Tue, 08 Mar 2011 17:23:58 -0800 From: Jeffrey Lee Hellrung, Jr. <jhellrung@ucla.edu> Reply-To: jhellrung@ucla.edu To: Ion Gaztañaga <igaztanaga@gmail.com> On 3/8/2011 1:57 PM, Ion Gaztañaga wrote:
Hi to all,
Are you a C++ expert? Do you like Boost.Move? I have a challenge for you.
During Boost.Move review, Jeffrey Lee Hellrung suggested adding "techniques to capture (implicitly created) rvalues with emulated rvalue references in C++03, e.g., for push_back, when you have knowledge of the types of objects you want to capture".
Yes, you know, I do seem to remember suggesting that...
I've worked on this problem these days and the solution is less than obvious (several compilers have has problems with ::boost::is_convertible and non-copyable types). I found a solution (attached test and required boost/move/move.hpp header) after several tries but it's complex (it needs several overloads and enable_if tricks) but I'm sure boosters will find a easier solution ;-)
Yes, a bit more complex than the solution I've been using for a while now (rough idea attached and explained below).
Current solution seems to work on these compilers: MSVC 7.1, 8.0, 9.0, Intel 11.0, GCC 4.3.4, 4.4, 4.4 c++0x mode, 4.5, 4.5 c++0x mode.
I've gotten your test cpp to compile and run with no assertion failures on MSVC9 (with a slight modification to assertions; see below), and I've used this same technique on gcc, but I don't work nearly as much on gcc, so it may be that some corner cases fail.
Problem: MSVC 10.0 (Visual 2010) with real rvalue references seems to have bugs and does not compile the test. I can't find a workaround, a future Service Pack might solve these errors, but maybe we should stick to emulation code in this compiler, unless someone could shed some light on this, of course.
Well...that sucks :(
Waiting your proposals,
Ion
Okay, here is the summary. Ion, I count 5 overloads of push_back in C++03 and 4 overloads in C++0x for your solution. I believe one can get by with just 3 overloads in C++03, and 1 in C++0x, but the path to "T construction" in my solution is a little bit different than yours. I delay any T construction until the placement new statement in priv_push_back. I don't know what your requirements are in general, but for push_back, I think this should be fine. The solution I sent you has maybe a slight disadvantage over yours in that conversion errors would likely end up pointing to priv_push_back, but if this is a concern, the template overloads of push_back can be restricted (further) with enable_if (I think). This has the effect that push_back'ing a conversion_source object x directly constructs the storage with x, rather than constructing a temporary and copying or constructing a temporary and moving. For push_back, it seems this is definitely desirable, and I'm having a hard time coming up with a situation where it is *not* desirable to delay the construction as long as possible. In any case, this explains the change in your assertions. I also added a Default value to the ConstructionType enum because...it was bugging me ;) The 3 overloads in C++03 are basically: // captures const lvalues and rvalues that are *not* T template< class U > typename boost::disable_if< is_same_sans_const_sans_reference<U,T> >::type push_back(const U& x) { ... as_lvalue(x) ... } // captures non-const lvalues and T lvalues template< class U > void push_back(U& x) { ... x ... } typedef typename add_reference_add_const< typename add_rvalue_reference<T>::type >::type rv_param_type; // captures T rvalues (if T is movable) void push_back(rv_param_type x) { ... x ... } In C++0x, a single overload is sufficient, I think: template< class U > void push_back(U&& x) { ... boost::forward<U>(x) ... } Let me know what you think of this framework. - Jeff
El 09/03/2011 2:25, Jeffrey Lee Hellrung, Jr. escribió:
Ion, I count 5 overloads of push_back in C++03 and 4 overloads in C++0x for your solution. I believe one can get by with just 3 overloads in C++03, and 1 in C++0x, but the path to "T construction" in my solution is a little bit different than yours. I delay any T construction until the placement new statement in priv_push_back. I don't know what your requirements are in general, but for push_back, I think this should be fine. The solution I sent you has maybe a slight disadvantage over yours in that conversion errors would likely end up pointing to priv_push_back, but if this is a concern, the template overloads of push_back can be restricted (further) with enable_if (I think).
I count 5 overloads for C++03 and 2 for C++0x (usual const T & and T&&). I don't know if emulation "placement construction" is right for convertible types, at least it should not occur in standard C++0x containers. std::list in C++0x can contain non-movable and non-copyable types, (some functions like resize(), require DefaultConstructible and emplace() requires EmplaceConstructible, but no MoveConstructible or CopyConstructible) and in this case insert(position, value) should fail for convertible types. I guess we should mimick that behaviour in C++03 containers with emulated move semantics. Best, Ion
On 3/9/2011 11:58 AM, Ion Gaztañaga wrote:
El 09/03/2011 2:25, Jeffrey Lee Hellrung, Jr. escribió:
Ion, I count 5 overloads of push_back in C++03 and 4 overloads in C++0x for your solution. I believe one can get by with just 3 overloads in C++03, and 1 in C++0x, but the path to "T construction" in my solution is a little bit different than yours. I delay any T construction until the placement new statement in priv_push_back. I don't know what your requirements are in general, but for push_back, I think this should be fine. The solution I sent you has maybe a slight disadvantage over yours in that conversion errors would likely end up pointing to priv_push_back, but if this is a concern, the template overloads of push_back can be restricted (further) with enable_if (I think).
I count 5 overloads for C++03 and 2 for C++0x (usual const T & and T&&).
Whoops! Yes, you're right.
I don't know if emulation "placement construction" is right for convertible types, at least it should not occur in standard C++0x containers.
I *think* I know what you mean by this, but just to clarify: you believe that push_back should behave as if it only accepted T const & and T&&...? I.e., the placement new should only copy constructor or move construct a T, not convert construct.
std::list in C++0x can contain non-movable and non-copyable types, (some functions like resize(), require DefaultConstructible and emplace() requires EmplaceConstructible, but no MoveConstructible or CopyConstructible)
...sure...
and in this case insert(position, value) should fail for convertible types.
By "should fail", you mean "should fail to compile, according to the standard", and here "value" is not an object of the std::list value_type, but some other type that's convertible to value_type, correct? Again, just to clarify.
I guess we should mimick that behaviour in C++03 containers with emulated move semantics.
::type
::type
Well that seems to be up to the author of the container. If you're looking to emulate the behavior of the std:: containers, including restricting to the same set of valid expressions, then yes, I guess you need to emulate push_back only receiving value_type const & and value_type&& parameters. In this case, in this example, in C++03, one can use 2 overloads of priv_push_back: template< class U > typename boost::enable_if< // strips const, &, and rv<> qualifiers is_same_sans_const_sans_reference<U,T> priv_push_back(U& x) { new(&storage_) T(x); } template< class U > typename boost::disable_if< // strips const, &, and rv<> qualifiers is_same_sans_const_sans_reference<U,T> priv_push_back(U& x) // something to force the instantiation of a move/copy constructor { new(&storage_) T(T(x)); } Alternatively, one could also put in some kind of static assertion in the templated push_back overloads that enforces the existence of a move/copy constructor whenever U isn't T (up to const and reference qualifiers). Yes, it's a bit more ugly, but if you were to document this as a general technique or recommendation, I would ultimately let the container author decide whether to allow "placement construction". - Jeff
El 09/03/2011 21:32, Jeffrey Lee Hellrung, Jr. escribió:
By "should fail", you mean "should fail to compile, according to the standard", and here "value" is not an object of the std::list value_type, but some other type that's convertible to value_type, correct? Again, just to clarify.
Right. The idea is also to have some macro to ease writing the same code for both C++03 and C++0x, e.g., priv_push_back receives a BOOST_FWD_REF param that can be forwarded both in C++03 and C++0x.
Yes, it's a bit more ugly, but if you were to document this as a general technique or recommendation, I would ultimately let the container author decide whether to allow "placement construction".
Yes, this is a good idea. Best, Ion
On 3/9/2011 1:41 PM, Ion Gaztañaga wrote:
El 09/03/2011 21:32, Jeffrey Lee Hellrung, Jr. escribió:
By "should fail", you mean "should fail to compile, according to the standard", and here "value" is not an object of the std::list value_type, but some other type that's convertible to value_type, correct? Again, just to clarify.
Right. The idea is also to have some macro to ease writing the same code for both C++03 and C++0x, e.g., priv_push_back receives a BOOST_FWD_REF param that can be forwarded both in C++03 and C++0x.
It seems reasonable for priv_push_back to have a common implementation between C++03 and C++0x. I believe the C++03 version should probably take it's parameter as U& rather than U const &, to preserve constness, so you might need to add an additional forwarding reference macro. I think I suggested this also in the Boost.Move review discussion. My lack of creativity has me using FWD2_REF( U ) expanding to U& in C++03 and U&& in C++0x. Are you also aiming to use macros to generate the various push_back overloads as well? That could be challenging to design a reasonable interface for, and I'm not necessarily opposed to the #ifndef BOOST_NO_RVALUE_REFERENCES / #else / #endif (as I'm not sure if a macro solution would look any better). - Jeff
El 09/03/2011 23:02, Jeffrey Lee Hellrung, Jr. escribió:
It seems reasonable for priv_push_back to have a common implementation between C++03 and C++0x. I believe the C++03 version should probably take it's parameter as U& rather than U const &, to preserve constness, so you might need to add an additional forwarding reference macro. I think I suggested this also in the Boost.Move review discussion. My lack of creativity has me using FWD2_REF( U ) expanding to U& in C++03 and U&& in C++0x.
So you think we could have a parameter that emulates "perfect forwarding" (lvalue ref, const lvalue ref or ::boost::rv)? It should be something that we could ::boost::forward until we finally construct the value (priv_push_back might not be the place where we call placement new in some containers, as we need to find the place, allocated, the node, etc.). Please, since you are more skilled than me in this forwarding issue, it would be nice if you could provide the code ;-)
Are you also aiming to use macros to generate the various push_back overloads as well? That could be challenging to design a reasonable interface for, and I'm not necessarily opposed to the #ifndef BOOST_NO_RVALUE_REFERENCES / #else / #endif (as I'm not sure if a macro solution would look any better).
I was thinking in something like: #define BOOST_MOVE_CONVERSION_AWARE_INSERTION_CATCH(TYPE, FWD_FUNCTION, RETURN_VALUE) In our test each parameter could be: TYPE = T FWD_FUNCTION = priv_push_back RETURN_VALUE = void We can have another macro that also takes an additional parameter before the forwarding, so that container users can implement something like: #define BOOST_MOVE_CONVERSION_AWARE_INSERTION_CATCH(TYPE, FWD_FUNCTION, RETURN_VALUE, BEFORE_PARAMETER) iterator insert(const_iterator, T &&x) iterator insert(const_iterator, const T &x) //Generate conversion-aware public insertion function BOOST_MOVE_CONVERSION_AWARE_INSERTION_CATCH_1(T, priv_insert, iterator, const_iterator) //Value will be forwarded here template<class U> iterator priv_insert(const_iterator, BOOST_FWD_REF(U) u) { return deeper_function(::boost::forward<U>(u)); } Best, Ion
participants (2)
-
Ion Gaztañaga -
Jeffrey Lee Hellrung, Jr.