[variant] Basic rvalue and C++11 features support

Hi, There is a patch in ticket #7620 that adds some rvalue assignment operators and rvalue constructors for Boost.Variant library. I saw no activity from authors of Boost.Variant Eric Friedman and Itay Maman for a long time, so if there will be no objections, I'll commit patch to trunk in the middle of November. Any suggestions and comments are welcomed. -- Best regards, Antony Polukhin

Le 01/11/12 18:57, Antony Polukhin a écrit :
Hi,
There is a patch in ticket #7620 that adds some rvalue assignment operators and rvalue constructors for Boost.Variant library.
I saw no activity from authors of Boost.Variant Eric Friedman and Itay Maman for a long time, so if there will be no objections, I'll commit patch to trunk in the middle of November.
Any suggestions and comments are welcomed.
Hi, I have not analyzed completely the patch but I see that Boost.Move is not used to emulate move semantics on C++98. Is there a deep reason to don't use it? Best, Vicente

on Thu Nov 01 2012, "Vicente J. Botet Escriba" <vicente.botet-AT-wanadoo.fr> wrote:
Le 01/11/12 18:57, Antony Polukhin a écrit :
Hi,
There is a patch in ticket #7620 that adds some rvalue assignment operators and rvalue constructors for Boost.Variant library.
I saw no activity from authors of Boost.Variant Eric Friedman and Itay Maman for a long time, so if there will be no objections, I'll commit patch to trunk in the middle of November.
Any suggestions and comments are welcomed.
Hi,
I have not analyzed completely the patch but I see that Boost.Move is not used to emulate move semantics on C++98. Is there a deep reason to don't use it?
+1 See https://svn.boost.org/trac/boost/ticket/7601 -- Dave Abrahams BoostPro Computing Software Development Training http://www.boostpro.com Clang/LLVM/EDG Compilers C++ Boost

----- Original Message -----
From: Dave Abrahams <dave@boostpro.com> To: boost@lists.boost.org Cc: Sent: Thursday, November 1, 2012 2:53 PM Subject: Re: [boost] [variant] Basic rvalue and C++11 features support
on Thu Nov 01 2012, "Vicente J. Botet Escriba" <vicente.botet-AT-wanadoo.fr> wrote:
Le 01/11/12 18:57, Antony Polukhin a écrit :
Hi,
There is a patch in ticket #7620 that adds some rvalue assignment operators and rvalue constructors for Boost.Variant library.
I saw no activity from authors of Boost.Variant Eric Friedman and Itay Maman for a long time, so if there will be no objections, I'll commit patch to trunk in the middle of November.
Any suggestions and comments are welcomed.
Hi,
I have not analyzed completely the patch but I see that Boost.Move is not used to emulate move semantics on C++98. Is there a deep reason to don't use it?
Interesting, as this will compile: struct foo { foo() {} boost::variant<int, float> x; }; int main() { foo a; a = foo(); } But if it were to use Boost.Move, this will no longer compile, breaking barkwards compatibility.

2012/11/2 paul Fultz <pfultz2@yahoo.com>:
From: Dave Abrahams <dave@boostpro.com>
Hi,
I have not analyzed completely the patch but I see that Boost.Move is not used to emulate move semantics on C++98. Is there a deep reason to don't use it?
...
But if it were to use Boost.Move, this will no longer compile, breaking barkwards compatibility.
Yes, main reason for not using Boost.Move's macrosses is compatibility. To be honest, I did not know about the issue mentioned by Paul Fultz. I was just scared with a huge amount of workarounds for compilers with broken constructor template orderings and MSVC6 specific hacks. Using some of the Boost.Move macrosses would definitely break compilation on some old/buggy compilers. Compilers that do support rvalues have no such bugs (or at least all of thouse compilers are being tested in regression tests), so allowing move optimizations only for C++11 compilers seemed reasonable. About ticket #7601. Using ONLY boost::move function (and NO macrosses) can give some performance gains without risk to break compatibility for C++03 compilers. For example, after applying patch #7620 there are some places which could benefit if users type uses Boost.Move (for example `move_into` visitor has code like T(::boost::detail::variant::move(operand))). I did not used Boost.Move in #7620 for simplicity of debugging: I did not wanted to mix possible bugs of #7620 patch with possible bugs of #7601. -- Best regards, Antony Polukhin

Le 02/11/12 08:36, Antony Polukhin a écrit :
2012/11/2 paul Fultz <pfultz2@yahoo.com>:
From: Dave Abrahams <dave@boostpro.com>
Hi,
I have not analyzed completely the patch but I see that Boost.Move is not used to emulate move semantics on C++98. Is there a deep reason to don't use it? +1 See https://svn.boost.org/trac/boost/ticket/7601 ... But if it were to use Boost.Move, this will no longer compile, breaking barkwards compatibility. Yes, main reason for not using Boost.Move's macrosses is compatibility. To be honest, I did not know about the issue mentioned by Paul Fultz. I was just scared with a huge amount of workarounds for compilers with broken constructor template orderings and MSVC6 specific hacks. Using some of the Boost.Move macrosses would definitely break compilation on some old/buggy compilers.
Compilers that do support rvalues have no such bugs (or at least all of thouse compilers are being tested in regression tests), so allowing move optimizations only for C++11 compilers seemed reasonable.
About ticket #7601. Using ONLY boost::move function (and NO macrosses) can give some performance gains without risk to break compatibility for C++03 compilers. For example, after applying patch #7620 there are some places which could benefit if users type uses Boost.Move (for example `move_into` visitor has code like T(::boost::detail::variant::move(operand))). I did not used Boost.Move in #7620 for simplicity of debugging: I did not wanted to mix possible bugs of #7620 patch with possible bugs of #7601.
Hi, I have some experience making portable move semantics for Boost.Thread which has also its own bugged move semantics emulation. I've added my own macros that use either the Boost.Move macros or the equivalent for the legacy emulation. This is a transitory step as IMO the legacy should be deprecated soon. While I agree that the use of macros is not ideal, don't have a correct c++03 implementation that is close to the C++11 one is a shame. However making a portable implementation is sometimes a headache, as there is not a C++03 move semantic emulation for general purpose classes as tuple, bind, shared_ptr, ... and other TR1. I really think that it is worth the effort and I hope that the Boost community will finish by providing a C++portable move semantic emulation if we don't want that the libraries finish as is the case of Boost.Variant, Boost.Fusion, ... providing only C++11 move semantics. Best, Vicente

on Fri Nov 02 2012, paul Fultz <pfultz2-AT-yahoo.com> wrote:
----- Original Message -----
From: Dave Abrahams <dave@boostpro.com> To: boost@lists.boost.org Cc: Sent: Thursday, November 1, 2012 2:53 PM Subject: Re: [boost] [variant] Basic rvalue and C++11 features support
on Thu Nov 01 2012, "Vicente J. Botet Escriba" <vicente.botet-AT-wanadoo.fr> wrote:
Le 01/11/12 18:57, Antony Polukhin a écrit :
Hi,
There is a patch in ticket #7620 that adds some rvalue assignment operators and rvalue constructors for Boost.Variant library.
I saw no activity from authors of Boost.Variant Eric Friedman and Itay Maman for a long time, so if there will be no objections, I'll commit patch to trunk in the middle of November.
Any suggestions and comments are welcomed.
Hi,
I have not analyzed completely the patch but I see that Boost.Move is not used to emulate move semantics on C++98. Is there a deep reason to don't use it?
Interesting, as this will compile:
struct foo { foo() {} boost::variant<int, float> x; };
int main() { foo a; a = foo(); }
But if it were to use Boost.Move, this will no longer compile, breaking barkwards compatibility.
Oh, *that* old problem again! I agree that it's searious. I guess Boost.Move is not-very-usable in that case. I wonder if Boost.Move could use whatever technique boost::variant does? -- Dave Abrahams BoostPro Computing Software Development Training http://www.boostpro.com Clang/LLVM/EDG Compilers C++ Boost

Patch from ticket #7620 was applied, passed tests and was merged to release branch. Next step is ticket #7712. It adds `template <class T> variant(T&&)` constructor to Boost.Variant for compilers with rvalue references support. If no one objects, I'll commit it in a week. P.S.: patch contains usage of deprecated macro, that would be fixed before applying it. -- Best regards, Antony Polukhin

Hi, I just looked at the current state of variant and noticed that the implementation of recursive_variant's move ctor seems to be not as optimal as I hoped. The current implementation as written is: recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand) : p_(new T( detail::variant::move(operand.get()) )) { } Unless I am mistaken, I think the heap allocation is not needed and the target should simply grab the pointer and mark the source pointer as 0 (with additional checks for 0 in the dtor): template <typename T> recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand) : p_(operand.release()) { } template <typename T> T* recursive_wrapper<T>::release() { T* ptr = p_; p_ = 0; return ptr; } template <typename T> recursive_wrapper<T>::~recursive_wrapper() { if (p_) boost::checked_delete(p_); } The tests are running just fine (tested with gcc, msvc) with this (more optimal) implementation. I also run the Spirit test suite (Spirit makes heavy use of variant). Thoughts? I can commit this patch if it is acceptable. Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com

On 06/01/2013 01:40, Joel de Guzman wrote:
Hi,
I just looked at the current state of variant and noticed that the implementation of recursive_variant's move ctor seems to be not as optimal as I hoped. The current implementation as written is:
recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand) : p_(new T( detail::variant::move(operand.get()) )) { }
Unless I am mistaken, I think the heap allocation is not needed and the target should simply grab the pointer and mark the source pointer as 0 (with additional checks for 0 in the dtor):
(...)
template <typename T> recursive_wrapper<T>::~recursive_wrapper() { if (p_) boost::checked_delete(p_); , }
Hi, Maybe I missed something but why would you test for p_ in the destructor ? Isn't boost::checked_delete< T >( 0 ) valid ? MAT.

On 1/7/13 2:04 AM, Mathieu Champlon wrote:
On 06/01/2013 01:40, Joel de Guzman wrote:
template <typename T> recursive_wrapper<T>::~recursive_wrapper() { if (p_) boost::checked_delete(p_); , }
Hi,
Maybe I missed something but why would you test for p_ in the destructor ? Isn't boost::checked_delete< T >( 0 ) valid ?
You are right. The check is not needed. Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com

In message <50EA0D46.8080100@boost-consulting.com>, Joel de Guzman <djowel@gmail.com> writes
On 1/7/13 2:04 AM, Mathieu Champlon wrote:
On 06/01/2013 01:40, Joel de Guzman wrote:
template <typename T> recursive_wrapper<T>::~recursive_wrapper() { if (p_) boost::checked_delete(p_); , }
Hi,
Maybe I missed something but why would you test for p_ in the destructor ? Isn't boost::checked_delete< T >( 0 ) valid ?
You are right. The check is not needed.
But is there an expected performance benefit? Regards, -- Alec Ross

Joel de Guzman wrote:
Hi,
I just looked at the current state of variant and noticed that the implementation of recursive_variant's move ctor seems to be not as optimal as I hoped. The current implementation as written is:
recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand) : p_(new T( detail::variant::move(operand.get()) )) { }
Unless I am mistaken, I think the heap allocation is not needed and the target should simply grab the pointer and mark the source pointer as 0 (with additional checks for 0 in the dtor):
template <typename T> recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand) : p_(operand.release()) { }
template <typename T> T* recursive_wrapper<T>::release() { T* ptr = p_; p_ = 0; return ptr; }
template <typename T> recursive_wrapper<T>::~recursive_wrapper() { if (p_) boost::checked_delete(p_); }
The tests are running just fine (tested with gcc, msvc) with this (more optimal) implementation. I also run the Spirit test suite (Spirit makes heavy use of variant).
Thoughts? I can commit this patch if it is acceptable.
A recursive_wrapper is not a pointer. It's a value-like wrapper that is assumed to always contain a valid object. The move constructor should leave the moved-from recursive_wrapper in a valid state, which precludes nullifying it. That is, unless you suggest adding an "empty" state to recursive_wrapper, which doesn't sound like a very good idea. -- Paul Smith -- View this message in context: http://boost.2283326.n4.nabble.com/variant-Basic-rvalue-and-C-11-features-su... Sent from the Boost - Dev mailing list archive at Nabble.com.

On 1/8/13 4:14 AM, Paul Smith wrote:
Joel de Guzman wrote:
Hi,
I just looked at the current state of variant and noticed that the implementation of recursive_variant's move ctor seems to be not as optimal as I hoped. The current implementation as written is:
recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand) : p_(new T( detail::variant::move(operand.get()) )) { }
Unless I am mistaken, I think the heap allocation is not needed and the target should simply grab the pointer and mark the source pointer as 0 (with additional checks for 0 in the dtor):
template <typename T> recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand) : p_(operand.release()) { }
template <typename T> T* recursive_wrapper<T>::release() { T* ptr = p_; p_ = 0; return ptr; }
template <typename T> recursive_wrapper<T>::~recursive_wrapper() { if (p_) boost::checked_delete(p_); }
The tests are running just fine (tested with gcc, msvc) with this (more optimal) implementation. I also run the Spirit test suite (Spirit makes heavy use of variant).
Thoughts? I can commit this patch if it is acceptable.
A recursive_wrapper is not a pointer. It's a value-like wrapper that is assumed to always contain a valid object. The move constructor should leave the moved-from recursive_wrapper in a valid state, which precludes nullifying it. That is, unless you suggest adding an "empty" state to recursive_wrapper, which doesn't sound like a very good idea.
I disagree. That state will happen only when copying rvalues which will immediately be destructed anyway. What danger do you see in that situation? Example: recursive_wrapper<foo> bar() {...} // function returning recursive_wrapper recursive_wrapper<foo> foo(bar()); // copy Under no circumstances will anyone get to see that "empty" state. Do you see something that I don't? Without this move optimization (as it currently is), it is very inefficient especially with big structures (e.g. tuples and fusion adapted structs). Without this optimization, such temporary copies will end up with two heap allocations and unnecessary copying of the structures, instead of one heap allocation and a simple pointer swap. That would mean the missed optimization in the order of magnitudes with applications that use variant heavily (e.g. Spirit). Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com

On 1/8/13 4:14 AM, Paul Smith wrote:
Joel de Guzman wrote:
Hi,
I just looked at the current state of variant and noticed that the implementation of recursive_variant's move ctor seems to be not as optimal as I hoped. The current implementation as written is:
recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand) : p_(new T( detail::variant::move(operand.get()) )) { }
Unless I am mistaken, I think the heap allocation is not needed and the target should simply grab the pointer and mark the source pointer as 0 (with additional checks for 0 in the dtor):
template <typename T> recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand) : p_(operand.release()) { }
template <typename T> T* recursive_wrapper<T>::release() { T* ptr = p_; p_ = 0; return ptr; }
template <typename T> recursive_wrapper<T>::~recursive_wrapper() { if (p_) boost::checked_delete(p_); }
The tests are running just fine (tested with gcc, msvc) with this (more optimal) implementation. I also run the Spirit test suite (Spirit makes heavy use of variant).
Thoughts? I can commit this patch if it is acceptable.
A recursive_wrapper is not a pointer. It's a value-like wrapper that is assumed to always contain a valid object. The move constructor should leave the moved-from recursive_wrapper in a valid state, which precludes nullifying it. That is, unless you suggest adding an "empty" state to recursive_wrapper, which doesn't sound like a very good idea.
I disagree. That state will happen only when copying rvalues which will immediately be destructed anyway. What danger do you see in that situation? Example:
recursive_wrapper<foo> bar() {...} // function returning recursive_wrapper
recursive_wrapper<foo> foo(bar()); // copy
Under no circumstances will anyone get to see that "empty" state. Do you see something that I don't?
Without this move optimization (as it currently is), it is very inefficient especially with big structures (e.g. tuples and fusion adapted structs). Without this optimization, such temporary copies will end up with two heap allocations and unnecessary copying of the structures, instead of one heap allocation and a simple pointer swap. That would mean the missed optimization in the order of magnitudes with applications that use variant heavily (e.g. Spirit).
I agree 100% with Joel. Move construction means move construction - i.e. the source object is by definition left in a zombie state. No harm done. What's the point in having a move constructor which essentially is equivalent to a copy constructor in the first place? Regards Hartmut --------------- http://boost-spirit.com http://stellar.cct.lsu.edu

On Tue, Jan 8, 2013 at 2:49 AM, Hartmut Kaiser <hartmut.kaiser@gmail.com> wrote:
On 1/8/13 4:14 AM, Paul Smith wrote:
A recursive_wrapper is not a pointer. It's a value-like wrapper that is assumed to always contain a valid object. The move constructor should leave the moved-from recursive_wrapper in a valid state, which precludes nullifying it. That is, unless you suggest adding an "empty" state to recursive_wrapper, which doesn't sound like a very good idea.
I disagree. That state will happen only when copying rvalues which will immediately be destructed anyway. What danger do you see in that situation? Example:
recursive_wrapper<foo> bar() {...} // function returning recursive_wrapper
recursive_wrapper<foo> foo(bar()); // copy
Under no circumstances will anyone get to see that "empty" state. Do you see something that I don't?
Without this move optimization (as it currently is), it is very inefficient especially with big structures (e.g. tuples and fusion adapted structs). Without this optimization, such temporary copies will end up with two heap allocations and unnecessary copying of the structures, instead of one heap allocation and a simple pointer swap. That would mean the missed optimization in the order of magnitudes with applications that use variant heavily (e.g. Spirit).
I agree 100% with Joel. Move construction means move construction - i.e. the source object is by definition left in a zombie state. No harm done. What's the point in having a move constructor which essentially is equivalent to a copy constructor in the first place?
Because it's not equivalent to a copy constructor. I can mutate the source object, just not break it. The move-ctor of std::vector is much more efficient than the copy-ctor, even though it leaves the source as a completely valid vector. Even in the recursive_wrapper case, the move-ctor is still (potentially) more efficient than the copy-ctor.
Regards Hartmut --------------- http://boost-spirit.com http://stellar.cct.lsu.edu
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Paul Smith

On Tue, Jan 8, 2013 at 2:49 AM, Hartmut Kaiser <hartmut.kaiser@gmail.com> wrote:
On 1/8/13 4:14 AM, Paul Smith wrote:
A recursive_wrapper is not a pointer. It's a value-like wrapper that is assumed to always contain a valid object. The move constructor should leave the moved-from recursive_wrapper in a valid state, which precludes nullifying it. That is, unless you suggest adding an "empty" state to recursive_wrapper, which doesn't sound like a very good idea.
I disagree. That state will happen only when copying rvalues which will immediately be destructed anyway. What danger do you see in that situation? Example:
recursive_wrapper<foo> bar() {...} // function returning recursive_wrapper
recursive_wrapper<foo> foo(bar()); // copy
Under no circumstances will anyone get to see that "empty" state. Do you see something that I don't?
Without this move optimization (as it currently is), it is very inefficient especially with big structures (e.g. tuples and fusion adapted structs). Without this optimization, such temporary copies will end up with two heap allocations and unnecessary copying of the structures, instead of one heap allocation and a simple pointer swap. That would mean the missed optimization in the order of magnitudes with applications that use variant heavily (e.g. Spirit).
I agree 100% with Joel. Move construction means move construction - i.e. the source object is by definition left in a zombie state. No harm done. What's the point in having a move constructor which essentially is equivalent to a copy constructor in the first place?
Because it's not equivalent to a copy constructor. I can mutate the source object, just not break it.
I did not suggest breaking the object. Setting the pointer to zero still leaves the object in valid state, no? And if not, it is easy enough to make it a valid state by changing the implementation.
The move-ctor of std::vector is much more efficient than the copy-ctor, even though it leaves the source as a completely valid vector. Even in the recursive_wrapper case, the move-ctor is still (potentially) more efficient than the copy-ctor.
The only thing the standard requires wrt a moved-from object is that they are left in a valid (although unspecified) state. Thus simply nulling out the pointer in the reference_wrapper should do the trick (just as Joel proposed). See for instance: 17.6.5.15 - [lib.types.movedfrom] <quote> Objects of types defined in the C++ standard library may be moved from (12.8). Move operations may be explicitly specified or implicitly generated. Unless otherwise specified, such moved-from objects shall be placed in a valid but unspecified state. </quote> Regards Hartmut --------------- http://boost-spirit.com http://stellar.cct.lsu.edu

On Tue, Jan 8, 2013 at 4:18 PM, Hartmut Kaiser <hartmut.kaiser@gmail.com> wrote:
On Tue, Jan 8, 2013 at 2:49 AM, Hartmut Kaiser <hartmut.kaiser@gmail.com> wrote:
On 1/8/13 4:14 AM, Paul Smith wrote:
A recursive_wrapper is not a pointer. It's a value-like wrapper that is assumed to always contain a valid object. The move constructor should leave the moved-from recursive_wrapper in a valid state, which precludes nullifying it. That is, unless you suggest adding an "empty" state to recursive_wrapper, which doesn't sound like a very good idea.
I disagree. That state will happen only when copying rvalues which will immediately be destructed anyway. What danger do you see in that situation? Example:
recursive_wrapper<foo> bar() {...} // function returning recursive_wrapper
recursive_wrapper<foo> foo(bar()); // copy
Under no circumstances will anyone get to see that "empty" state. Do you see something that I don't?
Without this move optimization (as it currently is), it is very inefficient especially with big structures (e.g. tuples and fusion adapted structs). Without this optimization, such temporary copies will end up with two heap allocations and unnecessary copying of the structures, instead of one heap allocation and a simple pointer swap. That would mean the missed optimization in the order of magnitudes with applications that use variant heavily (e.g. Spirit).
I agree 100% with Joel. Move construction means move construction - i.e. the source object is by definition left in a zombie state. No harm done. What's the point in having a move constructor which essentially is equivalent to a copy constructor in the first place?
Because it's not equivalent to a copy constructor. I can mutate the source object, just not break it.
I did not suggest breaking the object. Setting the pointer to zero still leaves the object in valid state, no?
No.
And if not, it is easy enough to make it a valid state by changing the implementation.
Sure, it's easy enough to add an empty state. The problem is that now everything that uses recursive_wrapper must take into account that it may not contain a live value.
The move-ctor of std::vector is much more efficient than the copy-ctor, even though it leaves the source as a completely valid vector. Even in the recursive_wrapper case, the move-ctor is still (potentially) more efficient than the copy-ctor.
The only thing the standard requires wrt a moved-from object is that they are left in a valid (although unspecified) state. Thus simply nulling out the pointer in the reference_wrapper should do the trick (just as Joel proposed).
See for instance: 17.6.5.15 - [lib.types.movedfrom]
<quote> Objects of types defined in the C++ standard library may be moved from (12.8). Move operations may be explicitly specified or implicitly generated. Unless otherwise specified, such moved-from objects shall be placed in a valid but unspecified state. </quote>
I think you misinterpret the term "valid state" in this context. A valid state is one that doesn't break the invariants of the object. A recursive_wrapper is assumed to always contain an instance, hence clearing it's pointer is not a valid state. A "valid but unspecified state" simply means *any* valid state (e.g. it can contain a different value). Btw, these requirements are for the library types, not client types used in conjunction with them. For the equivalent requirements on client types see tables 20 and 22 which reiterate these requirements using clearer wording.
Regards Hartmut --------------- http://boost-spirit.com http://stellar.cct.lsu.edu
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Paul Smith

Paul Smith <pl.smith.mail@gmail.com> writes:
On Tue, Jan 8, 2013 at 4:18 PM, Hartmut Kaiser <hartmut.kaiser@gmail.com> wrote:
On Tue, Jan 8, 2013 at 2:49 AM, Hartmut Kaiser <hartmut.kaiser@gmail.com> wrote:
On 1/8/13 4:14 AM, Paul Smith wrote:
A recursive_wrapper is not a pointer. It's a value-like wrapper that is assumed to always contain a valid object. The move constructor should leave the moved-from recursive_wrapper in a valid state, which precludes nullifying it. That is, unless you suggest adding an "empty" state to recursive_wrapper, which doesn't sound like a very good idea.
I disagree. That state will happen only when copying rvalues which will immediately be destructed anyway. What danger do you see in that situation? Example:
recursive_wrapper<foo> bar() {...} // function returning recursive_wrapper
recursive_wrapper<foo> foo(bar()); // copy
Under no circumstances will anyone get to see that "empty" state. Do you see something that I don't?
Without this move optimization (as it currently is), it is very inefficient especially with big structures (e.g. tuples and fusion adapted structs). Without this optimization, such temporary copies will end up with two heap allocations and unnecessary copying of the structures, instead of one heap allocation and a simple pointer swap. That would mean the missed optimization in the order of magnitudes with applications that use variant heavily (e.g. Spirit).
I agree 100% with Joel. Move construction means move construction - i.e. the source object is by definition left in a zombie state. No harm done. What's the point in having a move constructor which essentially is equivalent to a copy constructor in the first place?
Because it's not equivalent to a copy constructor. I can mutate the source object, just not break it.
I did not suggest breaking the object. Setting the pointer to zero still leaves the object in valid state, no?
No.
And if not, it is easy enough to make it a valid state by changing the implementation.
Sure, it's easy enough to add an empty state. The problem is that now everything that uses recursive_wrapper must take into account that it may not contain a live value.
The move-ctor of std::vector is much more efficient than the copy-ctor, even though it leaves the source as a completely valid vector. Even in the recursive_wrapper case, the move-ctor is still (potentially) more efficient than the copy-ctor.
The only thing the standard requires wrt a moved-from object is that they are left in a valid (although unspecified) state. Thus simply nulling out the pointer in the reference_wrapper should do the trick (just as Joel proposed).
See for instance: 17.6.5.15 - [lib.types.movedfrom]
<quote> Objects of types defined in the C++ standard library may be moved from (12.8). Move operations may be explicitly specified or implicitly generated. Unless otherwise specified, such moved-from objects shall be placed in a valid but unspecified state. </quote>
I think you misinterpret the term "valid state" in this context. A valid state is one that doesn't break the invariants of the object. A recursive_wrapper is assumed to always contain an instance, hence clearing it's pointer is not a valid state. A "valid but unspecified state" simply means *any* valid state (e.g. it can contain a different value). Btw, these requirements are for the library types, not client types used in conjunction with them. For the equivalent requirements on client types see tables 20 and 22 which reiterate these requirements using clearer wording.
Isn't the difference that there a two different concepts here that the standard fails to distinguish? IMO, it should be possible to treat an rvalue destructively and leave it in an invalid state when it is guaranteed the value cannot be accessed anymore afterwards, e.g. the move operation was implicitly generated. (Of course in that case the destructor of the class should be aware of this.) The only situation, I am aware of, where a value is still accessible after being moved is when it is forcibly converted to an T&& through std::move or a cast ("explictliy specified" in 12.8), so this shouldn't be permissible for some types. e.g. struct X { X(X&& x) { /* treat x destructively */ } }; X x; X x2{move(x)}; // require that x "can be moved". not // correct for reference_wrapper X x3{X()}; // works for reference_wrapper I think this is a defect of the standard library definition because you really want to implement some types that way and it should be possible to do it.

Philipp Moeller wrote:
The only situation, I am aware of, where a value is still accessible after being moved is when it is forcibly converted to an T&& through std::move or a cast ("explictliy specified" in 12.8), so this shouldn't be permissible for some types. ... I think this is a defect of the standard library definition because you really want to implement some types that way and it should be possible to do it.
No, you really don't want to implement a type this way. It would break* the default implementation of std::swap and most of the rest of the standard library. Moving from lvalues is an important use case. * Where "break" is a shorthand for "reduce performance significantly".

On 8 January 2013 08:18, Hartmut Kaiser <hartmut.kaiser@gmail.com> wrote:
The only thing the standard requires wrt a moved-from object is that they are left in a valid (although unspecified) state. Thus simply nulling out the pointer in the reference_wrapper should do the trick (just as Joel proposed).
See for instance: 17.6.5.15 - [lib.types.movedfrom]
<quote> Objects of types defined in the C++ standard library may be moved from (12.8). Move operations may be explicitly specified or implicitly generated. Unless otherwise specified, such moved-from objects shall be placed in a valid but unspecified state. </quote>
If I remember correctly, that section was added at the 11th hour so to speak (at the Madrid meeting, days before ratifying the standard) so we didn't have to go through the entire library figuring out and documenting which operations were and were not legal to call on moved-from standard library objects. Other than destroyable and (possibly) assignable, it really is up to the author of the class to define what operations are allowed on moved-from objects. In other words, authors are allowed to make "not in the moved-from state" as a precondition for operations they don't want to allow in that state. A nulled-out pointer in the reference wrapper meets this requirement. -- Nevin ":-)" Liber <mailto:nevin@eviloverlord.com> (847) 691-1404

On Tue, Jan 8, 2013 at 7:28 PM, Nevin Liber <nevin@eviloverlord.com> wrote:
On 8 January 2013 08:18, Hartmut Kaiser <hartmut.kaiser@gmail.com> wrote:
The only thing the standard requires wrt a moved-from object is that they are left in a valid (although unspecified) state. Thus simply nulling out the pointer in the reference_wrapper should do the trick (just as Joel proposed).
See for instance: 17.6.5.15 - [lib.types.movedfrom]
<quote> Objects of types defined in the C++ standard library may be moved from (12.8). Move operations may be explicitly specified or implicitly generated. Unless otherwise specified, such moved-from objects shall be placed in a valid but unspecified state. </quote>
If I remember correctly, that section was added at the 11th hour so to speak (at the Madrid meeting, days before ratifying the standard) so we didn't have to go through the entire library figuring out and documenting which operations were and were not legal to call on moved-from standard library objects.
Other than destroyable and (possibly) assignable, it really is up to the author of the class to define what operations are allowed on moved-from objects. In other words, authors are allowed to make "not in the moved-from state" as a precondition for operations they don't want to allow in that state.
A nulled-out pointer in the reference wrapper meets this requirement. -- Nevin ":-)" Liber <mailto:nevin@eviloverlord.com> (847) 691-1404
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Boost.Variant defines operator< for variants, so they are easily usable as keys for associative containers. Table 20 says that moved-from objects "must still meet the requirements of the library component that is using it. The operations listed in those requirements must work as specified whether the object has been moved from or not." Obviously, it's required that keys be comparable. But trying to compare two recursive variants that hold the same recusrive_wrapper type is UB if one them is null. Hence, recursive variants are no longer valid key types for associative containers (technically). -- Paul Smith

On Tue, Jan 8, 2013 at 7:48 PM, Peter Dimov <lists@pdimov.com> wrote:
Paul Smith wrote:
But trying to compare two recursive variants that hold the same recusrive_wrapper type is UB if one them is null.
You can make it work though, can't you? Null values just need to compare less than any other.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Sure. My point was that "destructible and assignable" are not always enough as far as the library goes. -- Paul Smith

Paul Smith wrote:
Sure. My point was that "destructible and assignable" are not always enough as far as the library goes.
Yes, I agree. To expand on that, the requirements defined by the standard library such as CopyConstructible, LessThanComparable, and so on, do not have preconditions. So if a movable type claims to conform to these requirements, it can't impose a precondition that the operations only work on values that haven't been moved from. And the same holds for requirements defined by libraries other than the standard library.

On Tue, Jan 8, 2013 at 8:09 PM, Peter Dimov <lists@pdimov.com> wrote:
Paul Smith wrote:
Sure. My point was that "destructible and assignable" are not always enough as far as the library goes.
Yes, I agree. To expand on that, the requirements defined by the standard library such as CopyConstructible, LessThanComparable, and so on, do not have preconditions. So if a movable type claims to conform to these requirements, it can't impose a precondition that the operations only work on values that haven't been moved from. And the same holds for requirements defined by libraries other than the standard library.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
That's exactly my point. If you were doing something like this (i.e. nullifying rec_wrap) in your own code, I wouldn't be all that concerned because you should know the requirements from your own types. But variant is a generic library, which is used by different people that have different requirements. Once we decide that moved-from recursive_variants need to be less-than comparable for the STL's sake, we essentially admit that the moved-from state *is* visible. And why stop here? Why just guarantee comparability? Different users might have other equally legitimate requirements just as well. The only way to make everyone happy is to guarantee that a moved-from recursive variant is a valid variant. -- Paul Smith

On Tue, Jan 8, 2013 at 8:48 PM, Peter Dimov <lists@pdimov.com> wrote:
Paul Smith wrote:
The only way to make everyone happy is to guarantee that a moved-from recursive variant is a valid variant.
Well, the performance hit from `new T` is making a lot of people unhappy, so technically, this is not true. :-)
Unfortunately, that's probably not the worst thing about it. The fact that it can also throw is the one that has the more subtle and far reaching consequences.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Paul Smith

On 1/9/13 11:03 PM, Paul Smith wrote:
On Tue, Jan 8, 2013 at 8:48 PM, Peter Dimov <lists@pdimov.com> wrote:
Paul Smith wrote:
The only way to make everyone happy is to guarantee that a moved-from recursive variant is a valid variant.
Well, the performance hit from `new T` is making a lot of people unhappy, so technically, this is not true. :-)
Unfortunately, that's probably not the worst thing about it. The fact that it can also throw is the one that has the more subtle and far reaching consequences.
And that is utterly disappointing. This "conservative" move makes all proxy-like objects with pointer ownership very inefficient! While we are advocating pass by value! So... can anyone finally give a good definition of what "valid" means? Nevin gives a compelling counter argument with NaN in that a NaN is a valid state for a floating point value yet you can't do any operations on it. I would think that such a state can be acceptable for an already moved-from object. Well, because, it just makes sense! And IMO, it would be foolish to let this optimization opportunity pass. Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com

On Jan 10, 2013, at 6:29 PM, Joel de Guzman <djowel@gmail.com> wrote:
On 1/9/13 11:03 PM, Paul Smith wrote:
On Tue, Jan 8, 2013 at 8:48 PM, Peter Dimov <lists@pdimov.com> wrote:
Paul Smith wrote:
The only way to make everyone happy is to guarantee that a moved-from recursive variant is a valid variant.
Well, the performance hit from `new T` is making a lot of people unhappy, so technically, this is not true. :-)
Unfortunately, that's probably not the worst thing about it. The fact that it can also throw is the one that has the more subtle and far reaching consequences.
And that is utterly disappointing. This "conservative" move makes all proxy-like objects with pointer ownership very inefficient! While we are advocating pass by value!
So... can anyone finally give a good definition of what "valid" means?
Validity depends upon the class in question. A smart pointer can be null and still valid. In your case, the class has the semantics that it always references a value. Changing that, to permit a null state, may be acceptable or it may have unacceptable side effects. Perhaps a new type, with the null state, can be added and used in some of the current contexts. That your tests passed after your changes suggests that Spirit might well use such a type without problem. ___ Rob

On 01/10/13 21:38, Rob Stewart wrote:
On Jan 10, 2013, at 6:29 PM, Joel de Guzman <djowel@gmail.com> wrote:
On 1/9/13 11:03 PM, Paul Smith wrote:
On Tue, Jan 8, 2013 at 8:48 PM, Peter Dimov <lists@pdimov.com> wrote:
Paul Smith wrote:
The only way to make everyone happy is to guarantee that a moved-from recursive variant is a valid variant.
Well, the performance hit from `new T` is making a lot of people unhappy, so technically, this is not true. :-)
Unfortunately, that's probably not the worst thing about it. The fact that it can also throw is the one that has the more subtle and far reaching consequences.
And that is utterly disappointing. This "conservative" move makes all proxy-like objects with pointer ownership very inefficient! While we are advocating pass by value!
So... can anyone finally give a good definition of what "valid" means?
Validity depends upon the class in question. A smart pointer can be null and still valid. In your case, the class has the semantics that it always references a value. Changing that, to permit a null state, may be acceptable or it may have unacceptable side effects. Perhaps a new type, with the null state, can be added and used in some of the current contexts.
The "truly tagged" variant here: http://svn.boost.org/svn/boost/sandbox/variadic_templates/boost/composite_st... has such a null state. When constructed with no arguments, it's in this null state. It's "truly tagged" because there can be duplicate bounded types distinguished only by the tag value. IOW, although: variant<T1,T1> is not valid, one_of_maybe<int,T1,T1> is. The one thing bad about this is that to get a T1 out, you *must* supply the value. For example: get<0> to get the 1st T1, iff the tag is 0 and get<1> to get the 2nd T1, iff the tag is 1 otherwise, if the tag is -1, no values have been put into the one_of_maybe, it's in the null state. However, that's also a good thing because there has been a problem with variants needing duplicate types: http://sourceforge.net/mailarchive/message.php?msg_id=24003772 The other bad thing about one_of_maybe is it's slower to compile than boost variant. I'm unsure why but I'd guess it's because there's more template meta-programming involved :( Hope the above gives some ideas about how to solve this problem. -regards, Larry
That your tests passed after your changes suggests that Spirit might well use such a type without problem.
___ Rob
_______________________________________________ Unsubscribe & other changes:

On 1/11/13 2:00 PM, Larry Evans wrote:
On 01/10/13 21:38, Rob Stewart wrote:
On Jan 10, 2013, at 6:29 PM, Joel de Guzman <djowel@gmail.com> wrote:
On 1/9/13 11:03 PM, Paul Smith wrote:
On Tue, Jan 8, 2013 at 8:48 PM, Peter Dimov <lists@pdimov.com> wrote:
Paul Smith wrote:
The only way to make everyone happy is to guarantee that a moved-from recursive variant is a valid variant.
Well, the performance hit from `new T` is making a lot of people unhappy, so technically, this is not true. :-)
Unfortunately, that's probably not the worst thing about it. The fact that it can also throw is the one that has the more subtle and far reaching consequences.
And that is utterly disappointing. This "conservative" move makes all proxy-like objects with pointer ownership very inefficient! While we are advocating pass by value!
So... can anyone finally give a good definition of what "valid" means?
Validity depends upon the class in question. A smart pointer can be null and still valid. In your case, the class has the semantics that it always references a value. Changing that, to permit a null state, may be acceptable or it may have unacceptable side effects. Perhaps a new type, with the null state, can be added and used in some of the current contexts.
The "truly tagged" variant here:
http://svn.boost.org/svn/boost/sandbox/variadic_templates/boost/composite_st...
has such a null state. When constructed with no arguments, it's in this null state. It's "truly tagged" because there can be duplicate bounded types distinguished only by the tag value. IOW, although:
Larry, sorry, but that's irrelevant. We are talking about recursive_wrapper here, not variant. Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com

On 01/11/13 00:17, Joel de Guzman wrote:
On 1/11/13 2:00 PM, Larry Evans wrote:
On 01/10/13 21:38, Rob Stewart wrote: [snip]
Validity depends upon the class in question. A smart pointer can be null and still valid. In your case, the class has the semantics that it always references a value. Changing that, to permit a null state, may be acceptable or it may have unacceptable side effects. Perhaps a new type, with the null state, can be added and used in some of the current contexts.
The "truly tagged" variant here:
http://svn.boost.org/svn/boost/sandbox/variadic_templates/boost/composite_st...
has such a null state. When constructed with no arguments, it's in this null state. It's "truly tagged" because there can be duplicate bounded types distinguished only by the tag value. IOW, although:
Larry, sorry, but that's irrelevant. We are talking about recursive_wrapper here, not variant.
Regards,
OK, I probably was jumping to conclusions, but from looking at: http://www.boost.org/doc/libs/1_52_0/doc/html/variant/tutorial.html#variant.... which contained: typedef boost::variant< int , boost::recursive_wrapper< binary_op<add> > , boost::recursive_wrapper< binary_op<sub> > > expression; I concluded that recursive_wrapper always occurred in a variant. I further jumped to the conclusion that *if* the variant had a null state, then it would not assume any of its bounded types were "in effect"; hence, would not call any destructors for any bounded type or perform any operations on any bounded type; hence, the pointer in recursive wrapper would not even need to be zeroed because the containing variant would not assume that it even had a recursive_wrapper as one of its bounded types. Sorry if I misunderstood, but it seemed reasonable suggestion at the time. -regards, Larry

Larry Evans wrote:
I further jumped to the conclusion that *if* the variant had a null state, then it would not assume any of its bounded types were "in effect"; hence, would not call any destructors for any bounded type or perform any operations on any bounded type; hence, the pointer in recursive wrapper would not even need to be zeroed because the containing variant would not assume that it even had a recursive_wrapper as one of its bounded types.
You're quite correct. The problem does not stem from inadequate tagging though. The problem stems from the never-empty guarantee: http://www.boost.org/doc/libs/1_52_0/doc/html/variant/design.html#variant.de... If the variant could be empty, or equivalently, if the variant always contained a type that can be reliably default-constructed (without an exception) such as int, the problems with assignment (both copy- and move-) and with move construction do not occur, because the target can be left holding the int. So, one option is to enable move only in this case.

On 01/11/13 08:18, Peter Dimov wrote:
Larry Evans wrote:
I further jumped to the conclusion that *if* the variant had a null state, then it would not assume any of its bounded types were "in effect"; hence, would not call any destructors for any bounded type or perform any operations on any bounded type; hence, the pointer in recursive wrapper would not even need to be zeroed because the containing variant would not assume that it even had a recursive_wrapper as one of its bounded types.
You're quite correct. The problem does not stem from inadequate tagging though. The problem stems from the never-empty guarantee:
http://www.boost.org/doc/libs/1_52_0/doc/html/variant/design.html#variant.de...
If the variant could be empty, or equivalently, if the variant always contained a type that can be reliably default-constructed (without an exception) such as int, the problems with assignment (both copy- and move-) and with move construction do not occur, because the target can be left holding the int.
So, one option is to enable move only in this case.
Are you suggesting that move is enabled only if the current tag (the variant<*>::which() result) indicates the current type is default constructable? I was thinking that after the move to the target variant, the source variant has its tag reset to something to indicate the current type is this default constructable type and that type would be empty. IOW, a default contructable something like: special_type<nothing_id> found here: http://svn.boost.org/svn/boost/sandbox/variadic_templates/boost/composite_st... IIUC, that would allow a move no matter what the current tag was, because afterwards, the source variant would have a null state, and in that null state, nothing could be done except during the ~variant() call when only the variant<...>::storage_ destructor would be called. Does that make sense? -regards, Larry

On Friday 11 January 2013 08:47:42 Larry Evans wrote:
On 01/11/13 08:18, Peter Dimov wrote:
I was thinking that after the move to the target variant, the source variant has its tag reset to something to indicate the current type is this default constructable type and that type would be empty. IOW, a default contructable something like:
special_type<nothing_id>
found here:
http://svn.boost.org/svn/boost/sandbox/variadic_templates/boost/composite_st orage/special_components.hpp
I vaugly remember that variant had a special support for type boost::blank that could be used as a fallback type for some operations. So if the variant has this type in the list of possible types the move operation could leave the source with boost::blank value. But that would only work if boost::blank was in the list.

Larry Evans wrote:
Are you suggesting that move is enabled only if the current tag (the variant<*>::which() result) indicates the current type is default constructable?
No, I'm suggesting that move (in the presence of recursive_wrappers) can be enabled only when there is at least one type that is nothrow-default-constructible, regardless of whether it's current. typedef variant<int, recursive_wrapper<foo>> V; V v1( std::move(v2) ); This move-constructs v1 from v2 and leaves int() into v2.

On 01/11/13 08:59, Peter Dimov wrote:
Larry Evans wrote:
Are you suggesting that move is enabled only if the current tag (the variant<*>::which() result) indicates the current type is default constructable?
No, I'm suggesting that move (in the presence of recursive_wrappers) can be enabled only when there is at least one type that is nothrow-default-constructible, regardless of whether it's current.
typedef variant<int, recursive_wrapper<foo>> V;
V v1( std::move(v2) );
This move-constructs v1 from v2 and leaves int() into v2.
Thanks for the clarification. However, that seems to involve a lot of checking because you'd have to check possibly all the bounded types to find one that is nothrow-default-constructible. Wouldn't it be simpler to define variant to have, implicitly, a nothrow-default-constructible type something like the special_type<nothing_id> I mentioned in my previous post or like the boost::blank which Andrey just mentioned in his post? -regards, Larry

Larry Evans wrote:
Thanks for the clarification.
However, that seems to involve a lot of checking because you'd have to check possibly all the bounded types to find one that is nothrow-default-constructible.
That's already what the copy constructor does. See the documentation on the never-emtpy guarantee.
Wouldn't it be simpler to define variant to have, implicitly, a nothrow-default-constructible type...
There's already such a type, boost::blank, but you need to include it explicitly in the list of possible types. If it's present, variant will use it to signify an empty variant. This is also explained on the aforementioned page.

On Fri, Jan 11, 2013 at 4:18 PM, Peter Dimov <lists@pdimov.com> wrote:
If the variant could be empty, or equivalently, if the variant always contained a type that can be reliably default-constructed (without an exception) such as int, the problems with assignment (both copy- and move-) and with move construction do not occur, because the target can be left holding the int.
So, one option is to enable move only in this case.
That's the first constructive solution so far. Something like: template <typename T> struct is_legitimate_move_target // specialize me : has_trivial_constructor<T> {}; -- Paul Smith

Paul Smith wrote:
That's the first constructive solution so far. Something like:
template <typename T> struct is_legitimate_move_target // specialize me : has_trivial_constructor<T> {};
has_nothrow_default_constructor is enough. There's no need to introduce a separate trait. Read http://www.boost.org/doc/libs/1_52_0/doc/html/variant/design.html#variant.de... people. It's all explained there.

On Fri, Jan 11, 2013 at 5:22 PM, Peter Dimov <lists@pdimov.com> wrote:
Paul Smith wrote:
That's the first constructive solution so far. Something like:
template <typename T> struct is_legitimate_move_target // specialize me : has_trivial_constructor<T> {};
has_nothrow_default_constructor is enough. There's no need to introduce a separate trait.
Read
http://www.boost.org/doc/libs/1_52_0/doc/html/variant/design.html#variant.de...
people. It's all explained there.
Even though it's called the "never-empty guarantee", I always thought that variant actually provides a strong exception guarantee (i.e. the variant is not just left unempty, but specifically in the previous state) so it's not directly relevant. Am I wrong? Anyway, there's no need to outright disallow move in any case. The goal is simply to find a good move target. Automatically assuming that a non-throwing default constructor is a good move target is a heuristic. A good one, but still a heuristic. A trivial default constructor looks like the most sensible default. If the user thinks otherwise, let him specialize the move-oriented trait. -- Paul Smith

Paul Smith wrote:
Even though it's called the "never-empty guarantee", I always thought that variant actually provides a strong exception guarantee (i.e. the variant is not just left unempty, but specifically in the previous state) so it's not directly relevant. Am I wrong?
Yes, I believe you are. The problem is that it's hard to provide the basic guarantee. Here's why. Imagine you are assigning one variant to another, but the contained types do not match. You need to destroy the target type first, then construct a source type into the target variant. This construction can throw. If it does, you have an invalid target, and no way to bring it back into a valid state. Any valid state, not just the original. If the variant did have an implicit empty state, you'd use it, but it doesn't, so you can't.
Anyway, there's no need to outright disallow move in any case. The goal is simply to find a good move target. Automatically assuming that a non-throwing default constructor is a good move target is a heuristic.
It's not a heuristic. It's the actual requirement that makes a type usable for the task. It makes no sense to use another requirement in its place, or let the user fiddle with it.

On Fri, Jan 11, 2013 at 5:56 PM, Peter Dimov <lists@pdimov.com> wrote:
Paul Smith wrote:
Even though it's called the "never-empty guarantee", I always thought that variant actually provides a strong exception guarantee (i.e. the variant is not just left unempty, but specifically in the previous state) so it's not directly relevant. Am I wrong?
Yes, I believe you are. The problem is that it's hard to provide the basic guarantee. Here's why. Imagine you are assigning one variant to another, but the contained types do not match. You need to destroy the target type first, then construct a source type into the target variant. This construction can throw. If it does, you have an invalid target, and no way to bring it back into a valid state. Any valid state, not just the original. If the variant did have an implicit empty state, you'd use it, but it doesn't, so you can't.
I'm aware of the difficulity, but I completely repressed the "Enabling Optimizations" section :) You're right.
Anyway, there's no need to outright disallow move in any case. The goal is simply to find a good move target. Automatically assuming that a non-throwing default constructor is a good move target is a heuristic.
It's not a heuristic. It's the actual requirement that makes a type usable for the task. It makes no sense to use another requirement in its place, or let the user fiddle with it.
It is a heuristic. Just because something is no-throw doesn't necessarily mean that it's cheap. But since it's no longer a precedence, it's fine. Still, there's no need to supress move construction in the absence of a no-throw default constructible type. -- Paul Smith

Paul Smith wrote:
It is a heuristic. Just because something is no-throw doesn't necessarily mean that it's cheap.
Well, if the variant picks the first such type, the user can place his preference first in the list.
Still, there's no need to supress move construction in the absence of a no-throw default constructible type.
No, you could still keep the current behavior. Incidentally, now that I think of it, the first implementation strategy in http://www.boost.org/doc/libs/1_52_0/doc/html/variant/design.html#variant.de... is possible when the target type has a nothrow move constructor. You could move-construct the temporary backup, destroy the target and if the copy fails, move-construct it back. So the assignment would go along the following lines: - if the types are the same, assign - if the source type has a nothrow copy* constructor, use it - if the target type has a nothrow move constructor, use it - if there is a nothrow-default-constructible type, use it - otherwise, do the heap backup thing * For move assignment, change copy* to move

On Fri, Jan 11, 2013 at 6:35 PM, Peter Dimov <lists@pdimov.com> wrote:
Paul Smith wrote:
Still, there's no need to supress move construction in the absence of a no-throw default constructible type.
No, you could still keep the current behavior.
I think this was affirmative, but I'm not sure.
Incidentally, now that I think of it, the first implementation strategy in
http://www.boost.org/doc/libs/1_52_0/doc/html/variant/design.html#variant.de...
is possible when the target type has a nothrow move constructor. You could move-construct the temporary backup, destroy the target and if the copy fails, move-construct it back. So the assignment would go along the following lines:
- if the types are the same, assign - if the source type has a nothrow copy* constructor, use it - if the target type has a nothrow move constructor, use it - if there is a nothrow-default-constructible type, use it - otherwise, do the heap backup thing
Sounds legit. On a similar note, we should probably use a nothrow default constructible type when move constructing a variant for any type with a throwing move-ctor, not just recursive_wrapper. -- Paul Smith

On Fri, Jan 11, 2013 at 7:15 PM, Paul Smith <pl.smith.mail@gmail.com> wrote:
On a similar note, we should probably use a nothrow default constructible type when move constructing a variant for any type with a throwing move-ctor, not just recursive_wrapper.
Ignore that, it's nonsense. -- Paul Smith

On Fri, Jan 11, 2013 at 7:19 PM, Paul Smith <pl.smith.mail@gmail.com> wrote:
On Fri, Jan 11, 2013 at 7:15 PM, Paul Smith <pl.smith.mail@gmail.com> wrote:
On a similar note, we should probably use a nothrow default constructible type when move constructing a variant for any type with a throwing move-ctor, not just recursive_wrapper.
Ignore that, it's nonsense.
FWIW, it becomes meaningful again if we provide an extension point for client types of variant to perform a destructive move. Now recursive_wrapper really does become just a special case. -- Paul Smith

Paul Smith wrote:
On Fri, Jan 11, 2013 at 6:35 PM, Peter Dimov <lists@pdimov.com> wrote:
Paul Smith wrote:
Still, there's no need to supress move construction in the absence of a no-throw default constructible type.
No, you could still keep the current behavior.
I think this was affirmative, but I'm not sure.
It was. :-) No, there's no need to suppress move construction. Not having a move constructor isn't really any better than having the throwing one. I'm not sure whether this will be satisfactory from Spirit point of view though. Do all Spirit uses contain an appropriate fallback type?

On 1/12/13 1:26 AM, Peter Dimov wrote:
Paul Smith wrote:
On Fri, Jan 11, 2013 at 6:35 PM, Peter Dimov <lists@pdimov.com> wrote:
Paul Smith wrote:
Still, there's no need to supress move construction in the absence of a no-throw default constructible type.
No, you could still keep the current behavior.
I think this was affirmative, but I'm not sure.
It was. :-) No, there's no need to suppress move construction. Not having a move constructor isn't really any better than having the throwing one.
I'm not sure whether this will be satisfactory from Spirit point of view though. Do all Spirit uses contain an appropriate fallback type?
At least all of the grammars I write do. That is also what I advocate. You'll almost always need a "blank" type in there to represent the "no-match" case and variant always default constructs with the first type in the type-list, and users are warned not to have heavy objects as the first type. Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com

On 1/11/13 11:02 PM, Paul Smith wrote:
On Fri, Jan 11, 2013 at 4:18 PM, Peter Dimov <lists@pdimov.com> wrote:
If the variant could be empty, or equivalently, if the variant always contained a type that can be reliably default-constructed (without an exception) such as int, the problems with assignment (both copy- and move-) and with move construction do not occur, because the target can be left holding the int.
So, one option is to enable move only in this case.
That's the first constructive solution so far. Something like:
template <typename T> struct is_legitimate_move_target // specialize me : has_trivial_constructor<T> {};
Paul, I agree! Peter, I like the direction this is heading to. I really appreciate your interest in this. Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com

2013/1/6 Joel de Guzman <djowel@gmail.com>:
Hi,
I just looked at the current state of variant and noticed that the implementation of recursive_variant's move ctor seems to be not as optimal as I hoped. The current implementation as written is: ...
I`ve finally got back from vacation, so let me add my 2 kopecks. We can make move constructor fast and noexcept (just like Joel proposed) and also guarantee non-empty behavior. The idea is to delay construction of object: T& get() { if (!get_pointer()) p_ = new T; return *get_pointer(); } const T& get() const { if (!get_pointer()) p_ = new T; // mark p_ as mutable return *get_pointer(); } So if move occurred object is empty, and if it is required again - it is default constructed. Any objections? -- Best regards, Antony Polukhin

Le 13/01/13 09:58, Antony Polukhin a écrit :
2013/1/6 Joel de Guzman <djowel@gmail.com>:
Hi,
I just looked at the current state of variant and noticed that the implementation of recursive_variant's move ctor seems to be not as optimal as I hoped. The current implementation as written is: ...
I`ve finally got back from vacation, so let me add my 2 kopecks.
We can make move constructor fast and noexcept (just like Joel proposed) and also guarantee non-empty behavior. The idea is to delay construction of object:
T& get() { if (!get_pointer()) p_ = new T; return *get_pointer(); }
const T& get() const { if (!get_pointer()) p_ = new T; // mark p_ as mutable return *get_pointer(); }
So if move occurred object is empty, and if it is required again - it is default constructed.
Any objections?
This would make less efficient the get operation. I would prefer to force the boost::blank default construction. Best, Vicente

2013/1/13 Vicente J. Botet Escriba <vicente.botet@wanadoo.fr>:
Le 13/01/13 09:58, Antony Polukhin a écrit :
2013/1/6 Joel de Guzman <djowel@gmail.com>:
Hi,
I just looked at the current state of variant and noticed that the implementation of recursive_variant's move ctor seems to be not as optimal as I hoped. The current implementation as written is:
...
I`ve finally got back from vacation, so let me add my 2 kopecks.
We can make move constructor fast and noexcept (just like Joel proposed) and also guarantee non-empty behavior. The idea is to delay construction of object:
T& get() { if (!get_pointer()) p_ = new T; return *get_pointer(); }
const T& get() const { if (!get_pointer()) p_ = new T; // mark p_ as mutable return *get_pointer(); }
So if move occurred object is empty, and if it is required again - it is default constructed.
Any objections?
This would make less efficient the get operation. I would prefer to force the boost::blank default construction.
I see no big difference between "BOOST_ASSERT(p_);" and "if (p_) p_ = new T;" except default construction requirement for T. Construction and "new" call will occur only if recursive_wrapper is being reused after move, which must be a really rare case. boost::blank is relevant to boost:variant, not to recursive_wrapper: boost::variant<boost::blank, boost::recursive_wrapper<foo> > var(std::move(variable)); // Will call recursive_wrapper(recursive_wrapper&&) if `variable` is a recursive_wrapper type or boost::blank(boost::blank&&) if `variable` is a boost::blank -- Best regards, Antony Polukhin

On Mon, Jan 14, 2013 at 11:09 AM, Antony Polukhin <antoshkka@gmail.com> wrote:
2013/1/13 Vicente J. Botet Escriba <vicente.botet@wanadoo.fr>:
This would make less efficient the get operation. I would prefer to force the boost::blank default construction.
I see no big difference between "BOOST_ASSERT(p_);" and "if (p_) p_ = new T;" except default construction requirement for T.
The assert will be removed in release builds, so the difference exists. Also, this makes get() a potential source of exceptions which is a breaking change and counter-intuitive. +1 for not making such a change.

On 01/13/13 13:33, Vicente J. Botet Escriba wrote:
Le 13/01/13 09:58, Antony Polukhin a écrit :
2013/1/6 Joel de Guzman <djowel@gmail.com>:
Hi,
I just looked at the current state of variant and noticed that the implementation of recursive_variant's move ctor seems to be not as optimal as I hoped. The current implementation as written is: ...
I`ve finally got back from vacation, so let me add my 2 kopecks.
We can make move constructor fast and noexcept (just like Joel proposed) and also guarantee non-empty behavior. The idea is to delay construction of object:
T& get() { if (!get_pointer()) p_ = new T; return *get_pointer(); }
const T& get() const { if (!get_pointer()) p_ = new T; // mark p_ as mutable return *get_pointer(); }
So if move occurred object is empty, and if it is required again - it is default constructed.
Any objections?
This would make less efficient the get operation. I would prefer to force the boost::blank default construction.
Best, Vicente
+2 for this. According to what Peter says here: http://lists.boost.org/Archives/boost/2013/01/200132.php the problems stem from the never-empty guarantee: http://www.boost.org/doc/libs/1_52_0/doc/html/variant/design.html#variant.de... That page indicates the problem occurs when some unexpected event, such as an exception during target copy construction, happens. For example, the above variant doc says: The crux of the problem, then, is this: in the event that copy-construction of the content of v2 fails, how can v1 maintain its "never-empty" guarantee? By the time copy-construction from v2 is attempted, v1 has already destroyed its content! The docs on the current solution for this problem: http://www.boost.org/doc/libs/1_52_0/doc/html/variant/design.html#variant.de... detail the solution steps as follows: 1. Copy-construct the content of the left-hand side to the heap; call the pointer to this data backup. 2. Destroy the content of the left-hand side. 3. Copy-construct the content of the right-hand side in the (now-empty) storage of the left-hand side. 4. In the event of failure, copy backup to the left-hand side storage. 5. In the event of success, deallocate the data pointed to by backup. Thus, in the event of failure(item 4 in the above), the current solution restores the left-hand side to it's previous state. Thus, there is no indication in the variant that anything has gone wrong other than the fact that the variant's value has not changed from it's state before the assignment. This test for the exact same value seems to be an "additional complexity-of-use" which, according to: http://www.boost.org/doc/libs/1_52_0/doc/html/variant/design.html#variant.de... the never-empty guarantee was supposed to insulate the user from. Wouldn't the "implicit" boost::blank default construction be a "less complexity-of-use" alternative? IOW, when an exception occurs during an assignment of two distinct types, the variant's type is changed to boost::blank even when that is not among the explicit bounded types. This would be a flag to the user that something unusual had happened and would easily be tested for with the which() function which would return some value (e.g. -2 or -999 or whatever) indicating that the value is "undefined". -regards, Larry

On Mon, Jan 14, 2013 at 4:37 PM, Larry Evans <cppljevans@suddenlink.net>wrote:
[...]
Thus, there is no indication in the variant that anything has gone
wrong other than the fact that the variant's value has not changed from it's state before the assignment. This test for the exact same value seems to be an "additional complexity-of-use" which, according to:
http://www.boost.org/doc/libs/1_52_0/doc/html/variant/design.html#variant.de...
the never-empty guarantee was supposed to insulate the user from.
Wouldn't the "implicit" boost::blank default construction be a "less complexity-of-use" alternative? IOW, when an exception occurs during an assignment of two distinct types, the variant's type is changed to boost::blank even when that is not among the explicit bounded types. This would be a flag to the user that something unusual had happened and would easily be tested for with the which() function which would return some value (e.g. -2 or -999 or whatever) indicating that the value is "undefined".
An exception being thrown (well, propagated) is a fairly strong indication that something unusual has happened. Variant does not swallow the exception that caused the rollback. -- gpd

On 01/14/13 11:11, Giovanni Piero Deretta wrote:
On Mon, Jan 14, 2013 at 4:37 PM, Larry Evans <cppljevans@suddenlink.net>wrote:
[...]
Thus, there is no indication in the variant that anything has gone
wrong other than the fact that the variant's value has not changed from it's state before the assignment. [snip]
An exception being thrown (well, propagated) is a fairly strong indication that something unusual has happened. Variant does not swallow the exception that caused the rollback.
-- gpd
Thanks, my bad; however, my excuse is there was no indication in the passages which I cited that exceptions are propagated. OTOH, prompted by your reply, I looked at: http://www.boost.org/doc/libs/1_52_0/doc/html/boost/variant.html#id1244408-b... which does indicate that operator= propagates exceptions: may fail with any exceptions arising from the assignment of the content of rhs into the content *this. Sorry for noise :( -regards, Larry

On 1/12/13 7:54 AM, Joel de Guzman wrote:
On 1/11/13 11:02 PM, Paul Smith wrote:
On Fri, Jan 11, 2013 at 4:18 PM, Peter Dimov <lists@pdimov.com> wrote:
If the variant could be empty, or equivalently, if the variant always contained a type that can be reliably default-constructed (without an exception) such as int, the problems with assignment (both copy- and move-) and with move construction do not occur, because the target can be left holding the int.
So, one option is to enable move only in this case.
That's the first constructive solution so far. Something like:
template <typename T> struct is_legitimate_move_target // specialize me : has_trivial_constructor<T> {};
Paul, I agree!
Peter, I like the direction this is heading to. I really appreciate your interest in this.
I am reopening this issue because I just got a notification that issue 7718 is being closed as "fixed" https://svn.boost.org/trac/boost/ticket/7718 It seems the author of the patch (mistakenly) concluded that the current "vote" is to keep the slow move implementation. IMO, that is wrong. The current consensus (there was no such a vote AFAIK) is Peter's suggestion (regarding the blank type) which makes a lot of sense. If you care about move performance for variant, then by all means holler! The current patch is not acceptable! Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com

On 1/11/13 11:38 AM, Rob Stewart wrote:
On Jan 10, 2013, at 6:29 PM, Joel de Guzman <djowel@gmail.com> wrote:
On 1/9/13 11:03 PM, Paul Smith wrote:
On Tue, Jan 8, 2013 at 8:48 PM, Peter Dimov <lists@pdimov.com> wrote:
Paul Smith wrote:
The only way to make everyone happy is to guarantee that a moved-from recursive variant is a valid variant.
Well, the performance hit from `new T` is making a lot of people unhappy, so technically, this is not true. :-)
Unfortunately, that's probably not the worst thing about it. The fact that it can also throw is the one that has the more subtle and far reaching consequences.
And that is utterly disappointing. This "conservative" move makes all proxy-like objects with pointer ownership very inefficient! While we are advocating pass by value!
So... can anyone finally give a good definition of what "valid" means?
Validity depends upon the class in question. A smart pointer can be null and still valid. In your case, the class has the semantics that it always references a value. Changing that, to permit a null state, may be acceptable or it may have unacceptable side effects. Perhaps a new type, with the null state, can be added and used in some of the current contexts.
That your tests passed after your changes suggests that Spirit might well use such a type without problem.
The problem Paul mentions is very unlikely to happen in real code. I agree with him that it is more of a paranoia. In 99.9999999% of the cases, the moved-from object will be destructed right away. Such a paranoid conservative move will penalize the 99.9999999% of cases, IMO. Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com

On 8 January 2013 12:09, Peter Dimov <lists@pdimov.com> wrote:
Paul Smith wrote:
Sure. My point was that "destructible and assignable" are not always enough as far as the library goes.
Yes, I agree. To expand on that, the requirements defined by the standard library such as CopyConstructible, LessThanComparable, and so on, do not have preconditions. So if a movable type claims to conform to these requirements, it can't impose a precondition that the operations only work on values that haven't been moved from.
I believe it can. Take n3485 25.4p3 (Sorted and related operations): "For algorithms other than those described in 25.4.3 to work correctly, comp has to induce a strict weak ordering on the values." This, for instance, allows sorting of doubles as long as none of them are NaN. -- Nevin ":-)" Liber <mailto:nevin@eviloverlord.com> (847) 691-1404

Nevin Liber wrote:
On 8 January 2013 12:09, Peter Dimov <lists@pdimov.com> wrote:
Yes, I agree. To expand on that, the requirements defined by the standard library such as CopyConstructible, LessThanComparable, and so on, do not have preconditions. So if a movable type claims to conform to these requirements, it can't impose a precondition that the operations only work on values that haven't been moved from.
I believe it can. Take n3485 25.4p3 (Sorted and related operations): "For algorithms other than those described in 25.4.3 to work correctly, comp has to induce a strict weak ordering on the values."
This, for instance, allows sorting of doubles as long as none of them are NaN.
If I have the values { 1, 7, 5, -3 } and a predicate that works correctly for these specific values, but returns garbage for others, does it satisfy the requirement? An interesting question... for another time, maybe.

Nevin Liber wrote:
Other than destroyable and (possibly) assignable,
Assignable and move-assignable are an absolute requirement, I'd say. I also consider copy and move construction from a moved-from object part of the absolute minimum.
it really is up to the author of the class to define what operations are allowed on moved-from objects.
That's trivially true, because "valid" is always defined by the author of the class. But some definitions are more useful than others.

on Tue Jan 08 2013, "Peter Dimov" <lists-AT-pdimov.com> wrote:
Nevin Liber wrote:
Other than destroyable and (possibly) assignable,
Assignable and move-assignable are an absolute requirement, I'd say. I also consider copy and move construction from a moved-from object part of the absolute minimum.
it really is up to the author of the class to define what operations are allowed on moved-from objects.
That's trivially true, because "valid" is always defined by the author of the class. But some definitions are more useful than others.
IMO the issue in this case isn't what definitions are useful, but that the definition of "valid" was already set before move semantics was added and it can't be changed without breaking code. -- Dave Abrahams BoostPro Computing Software Development Training http://www.boostpro.com Clang/LLVM/EDG Compilers C++ Boost

Hartmut Kaiser wrote
On 1/8/13 4:14 AM, Paul Smith wrote:
Joel de Guzman wrote:
Hi,
I just looked at the current state of variant and noticed that the implementation of recursive_variant's move ctor seems to be not as optimal as I hoped. The current implementation as written is:
recursive_wrapper <T> ::recursive_wrapper(recursive_wrapper&& operand) : p_(new T( detail::variant::move(operand.get()) )) { }
Unless I am mistaken, I think the heap allocation is not needed and the target should simply grab the pointer and mark the source pointer as 0 (with additional checks for 0 in the dtor):
template <typename T> recursive_wrapper <T> ::recursive_wrapper(recursive_wrapper&& operand) : p_(operand.release()) { }
template <typename T> T* recursive_wrapper <T> ::release() { T* ptr = p_; p_ = 0; return ptr; }
template <typename T> recursive_wrapper <T> ::~recursive_wrapper() { if (p_) boost::checked_delete(p_); }
The tests are running just fine (tested with gcc, msvc) with this (more optimal) implementation. I also run the Spirit test suite (Spirit makes heavy use of variant).
Thoughts? I can commit this patch if it is acceptable.
A recursive_wrapper is not a pointer. It's a value-like wrapper that is assumed to always contain a valid object. The move constructor should leave the moved-from recursive_wrapper in a valid state, which precludes nullifying it. That is, unless you suggest adding an "empty" state to recursive_wrapper, which doesn't sound like a very good idea.
I disagree. That state will happen only when copying rvalues which will immediately be destructed anyway. What danger do you see in that situation? Example:
recursive_wrapper <foo> bar() {...} // function returning recursive_wrapper
recursive_wrapper <foo> foo(bar()); // copy
Under no circumstances will anyone get to see that "empty" state. Do you see something that I don't?
Without this move optimization (as it currently is), it is very inefficient especially with big structures (e.g. tuples and fusion adapted structs). Without this optimization, such temporary copies will end up with two heap allocations and unnecessary copying of the structures, instead of one heap allocation and a simple pointer swap. That would mean the missed optimization in the order of magnitudes with applications that use variant heavily (e.g. Spirit).
I agree 100% with Joel. Move construction means move construction - i.e. the source object is by definition left in a zombie state. No harm done. What's the point in having a move constructor which essentially is equivalent to a copy constructor in the first place?
I have not see the code using recursive_wrapper. Do we really need that recursive_wrapper be movable? if yes, in which cases this optimization is a must have? Best, Vicente -- View this message in context: http://boost.2283326.n4.nabble.com/variant-Basic-rvalue-and-C-11-features-su... Sent from the Boost - Dev mailing list archive at Nabble.com.

Hartmut Kaiser wrote
On 1/8/13 4:14 AM, Paul Smith wrote:
Joel de Guzman wrote:
Hi,
I just looked at the current state of variant and noticed that the implementation of recursive_variant's move ctor seems to be not as optimal as I hoped. The current implementation as written is:
recursive_wrapper <T> ::recursive_wrapper(recursive_wrapper&& operand) : p_(new T( detail::variant::move(operand.get()) )) { }
Do you see something that I don't?
Without this move optimization (as it currently is), it is very inefficient especially with big structures (e.g. tuples and fusion adapted structs). Without this optimization, such temporary copies will end up with two heap allocations and unnecessary copying of the structures, instead of one heap allocation and a simple pointer swap. That would mean the missed optimization in the order of magnitudes with applications that use variant heavily (e.g. Spirit).
I agree 100% with Joel. Move construction means move construction - i.e. the source object is by definition left in a zombie state. No harm done. What's the point in having a move constructor which essentially is equivalent to a copy constructor in the first place?
I have not see the code using recursive_wrapper. Do we really need recursive_wrapper to be movable? if yes, in which cases this optimization is a must have? Best, Vicente -- View this message in context: http://boost.2283326.n4.nabble.com/variant-Basic-rvalue-and-C-11-features-su... Sent from the Boost - Dev mailing list archive at Nabble.com.

On Tue, Jan 8, 2013 at 2:04 AM, Joel de Guzman <djowel@gmail.com> wrote:
On 1/8/13 4:14 AM, Paul Smith wrote:
A recursive_wrapper is not a pointer. It's a value-like wrapper that is assumed to always contain a valid object. The move constructor should leave the moved-from recursive_wrapper in a valid state, which precludes nullifying it. That is, unless you suggest adding an "empty" state to recursive_wrapper, which doesn't sound like a very good idea.
I disagree. That state will happen only when copying rvalues which will immediately be destructed anyway. What danger do you see in that situation? Example:
recursive_wrapper<foo> bar() {...} // function returning recursive_wrapper
recursive_wrapper<foo> foo(bar()); // copy
Under no circumstances will anyone get to see that "empty" state. Do you see something that I don't?
Without this move optimization (as it currently is), it is very inefficient especially with big structures (e.g. tuples and fusion adapted structs). Without this optimization, such temporary copies will end up with two heap allocations and unnecessary copying of the structures, instead of one heap allocation and a simple pointer swap. That would mean the missed optimization in the order of magnitudes with applications that use variant heavily (e.g. Spirit).
I hear ya. However, the C++11 take on move semantics is, for better and for worse, a conservative one and not a destructive one. It's a common misconception that a moved-from object is "as good as dead" and that the move constructor can perform an autopsy. (It took me a while to realize that, too). A move constructor has merely a "license to mutate" the source object while keeping it's invariants, not a "license to kill". This is, I believe, the intended convention. This is more or less what the STL expect from client types. And, even though it's not enforced by the core language, the effective semantics of rvalues and rvalue-references are such that a destructive move is unsafe. In particular, subobjects of rvalues are rvalues of the same value category, and can equally well bind to rv-refs. Even if a top-level rvalue is immediately destroyed after one of its subojects has been moved-from, its destructor might still attempt to use that subobject. If a type gives me public access to one of it's subobjects, it means that I'm allowed to mutate it using it's public interface any way I want, but nothing more. If the subobject's move consutrctor (which I can rightfully invoke) does something that's impossible to acheive using the public interface, I'm breaking this contract, and even if I no longer use the containing object, I might get punished when it's destructor takes place: class base { void something_naughty(); // don't attempt to use the // object after calling me public: base(); base(base&& other) {other.something_naughty();} void something_ordinary(); }; struct derived : base { ~derived() {something_ordinary();} }; int main() { base b = derived(); // I'm only using the public interface // of derived. How did I manage to // screw something up? } Note that this also applies to data members. Also, note that rvalues can ofcourse be std::moved lvalues, in which case they can be accessed by the programmer after having moved them, and he might wish to reuse them (or, if the lvalue is an lv-ref parameter, he might not know if whoever passed him this lvalue does), even though, admittedly, I haven't seen many convincing examples of such cases. I agree that the cases for which a destructive move is appropriate seem by far the dominant ones (even if it's no different than a conservative move in many of them). If you're asking for a rationale as to why the semantics are the way they are, then you're talking to the wrong person. I am sure David Abrahams can give a much more credible answer than I can. Some of these issues are discussed in N1377, in particular, see the "Alternative Move Designs" -> "Destructive move semantics" section (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2002/n1377.htm#Alternativ... move designs) (Also note that the "Copy vs Move" section literally states: "The only requirement (from the move-ctor PS) is that the (source PS) object remain in a self consistent state (all internal invariants are still intact).") FWIW, I'm playing with the idea of writing a destructive-move proposal (although what I'm reffering to as destructive-move is a bit different than what the move proposal calls a destructive-move), and these kind of cases are exactly what I wish to address. If you have actual numbers that demonstrate how bad is the impact on Spirirt and co, I'd love to hear them.
Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Paul Smith

On 1/8/13 11:05 AM, Paul Smith wrote:
On Tue, Jan 8, 2013 at 2:04 AM, Joel de Guzman <djowel@gmail.com> wrote:
On 1/8/13 4:14 AM, Paul Smith wrote:
A recursive_wrapper is not a pointer. It's a value-like wrapper that is assumed to always contain a valid object. The move constructor should leave the moved-from recursive_wrapper in a valid state, which precludes nullifying it. That is, unless you suggest adding an "empty" state to recursive_wrapper, which doesn't sound like a very good idea.
I disagree. That state will happen only when copying rvalues which will immediately be destructed anyway. What danger do you see in that situation? Example:
recursive_wrapper<foo> bar() {...} // function returning recursive_wrapper
recursive_wrapper<foo> foo(bar()); // copy
Under no circumstances will anyone get to see that "empty" state. Do you see something that I don't?
Without this move optimization (as it currently is), it is very inefficient especially with big structures (e.g. tuples and fusion adapted structs). Without this optimization, such temporary copies will end up with two heap allocations and unnecessary copying of the structures, instead of one heap allocation and a simple pointer swap. That would mean the missed optimization in the order of magnitudes with applications that use variant heavily (e.g. Spirit).
I hear ya. However, the C++11 take on move semantics is, for better and for worse, a conservative one and not a destructive one. It's a common misconception that a moved-from object is "as good as dead" and that the move constructor can perform an autopsy. (It took me a while to realize that, too).
A move constructor has merely a "license to mutate" the source object while keeping it's invariants, not a "license to kill". This is, I believe, the intended convention. This is more or less what the STL expect from client types. And, even though it's not enforced by the core language, the effective semantics of rvalues and rvalue-references are such that a destructive move is unsafe. In particular, subobjects of rvalues are rvalues of the same value category, and can equally well bind to rv-refs. Even if a top-level rvalue is immediately destroyed after one of its subojects has been moved-from, its destructor might still attempt to use that subobject. If a type gives me public access to one of it's subobjects, it means that I'm allowed to mutate it using it's public interface any way I want, but nothing more. If the subobject's move consutrctor (which I can rightfully invoke) does something that's impossible to acheive using the public interface, I'm breaking this contract, and even if I no longer use the containing object, I might get punished when it's destructor takes place:
class base { void something_naughty(); // don't attempt to use the // object after calling me public: base(); base(base&& other) {other.something_naughty();}
void something_ordinary(); };
struct derived : base { ~derived() {something_ordinary();} };
int main() { base b = derived(); // I'm only using the public interface // of derived. How did I manage to // screw something up? }
Note that this also applies to data members. Also, note that rvalues can ofcourse be std::moved lvalues, in which case they can be accessed by the programmer after having moved them, and he might wish to reuse them (or, if the lvalue is an lv-ref parameter, he might not know if whoever passed him this lvalue does), even though, admittedly, I haven't seen many convincing examples of such cases.
I agree that the cases for which a destructive move is appropriate seem by far the dominant ones (even if it's no different than a conservative move in many of them). If you're asking for a rationale as to why the semantics are the way they are, then you're talking to the wrong person. I am sure David Abrahams can give a much more credible answer than I can. Some of these issues are discussed in N1377, in particular, see the "Alternative Move Designs" -> "Destructive move semantics" section (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2002/n1377.htm#Alternativ... move designs) (Also note that the "Copy vs Move" section literally states: "The only requirement (from the move-ctor PS) is that the (source PS) object remain in a self consistent state (all internal invariants are still intact).")
FWIW, I'm playing with the idea of writing a destructive-move proposal (although what I'm reffering to as destructive-move is a bit different than what the move proposal calls a destructive-move), and these kind of cases are exactly what I wish to address. If you have actual numbers that demonstrate how bad is the impact on Spirirt and co, I'd love to hear them.
Those are very good points, but I think you are being too pedantic. The example you posted concerns a class hierarchy (base / derived). The paper you cited mentions: "When dealing with class hierarchies, destructive move semantics becomes problematic." That is certainly not the case with recursive_wrapper. Do you see a reasonable cause for danger with (a non-hierarchical struct such as) recursive_wrapper? Finally, it would be best if you quote from the standard. That paper you posted is quite old (from 2002). Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com

On Tue, Jan 8, 2013 at 1:18 PM, Joel de Guzman <djowel@gmail.com> wrote:
On 1/8/13 11:05 AM, Paul Smith wrote:
On Tue, Jan 8, 2013 at 2:04 AM, Joel de Guzman <djowel@gmail.com> wrote:
On 1/8/13 4:14 AM, Paul Smith wrote:
A recursive_wrapper is not a pointer. It's a value-like wrapper that is assumed to always contain a valid object. The move constructor should leave the moved-from recursive_wrapper in a valid state, which precludes nullifying it. That is, unless you suggest adding an "empty" state to recursive_wrapper, which doesn't sound like a very good idea.
I disagree. That state will happen only when copying rvalues which will immediately be destructed anyway. What danger do you see in that situation? Example:
recursive_wrapper<foo> bar() {...} // function returning recursive_wrapper
recursive_wrapper<foo> foo(bar()); // copy
Under no circumstances will anyone get to see that "empty" state. Do you see something that I don't?
Without this move optimization (as it currently is), it is very inefficient especially with big structures (e.g. tuples and fusion adapted structs). Without this optimization, such temporary copies will end up with two heap allocations and unnecessary copying of the structures, instead of one heap allocation and a simple pointer swap. That would mean the missed optimization in the order of magnitudes with applications that use variant heavily (e.g. Spirit).
I hear ya. However, the C++11 take on move semantics is, for better and for worse, a conservative one and not a destructive one. It's a common misconception that a moved-from object is "as good as dead" and that the move constructor can perform an autopsy. (It took me a while to realize that, too).
A move constructor has merely a "license to mutate" the source object while keeping it's invariants, not a "license to kill". This is, I believe, the intended convention. This is more or less what the STL expect from client types. And, even though it's not enforced by the core language, the effective semantics of rvalues and rvalue-references are such that a destructive move is unsafe. In particular, subobjects of rvalues are rvalues of the same value category, and can equally well bind to rv-refs. Even if a top-level rvalue is immediately destroyed after one of its subojects has been moved-from, its destructor might still attempt to use that subobject. If a type gives me public access to one of it's subobjects, it means that I'm allowed to mutate it using it's public interface any way I want, but nothing more. If the subobject's move consutrctor (which I can rightfully invoke) does something that's impossible to acheive using the public interface, I'm breaking this contract, and even if I no longer use the containing object, I might get punished when it's destructor takes place:
class base { void something_naughty(); // don't attempt to use the // object after calling me public: base(); base(base&& other) {other.something_naughty();}
void something_ordinary(); };
struct derived : base { ~derived() {something_ordinary();} };
int main() { base b = derived(); // I'm only using the public interface // of derived. How did I manage to // screw something up? }
Note that this also applies to data members. Also, note that rvalues can ofcourse be std::moved lvalues, in which case they can be accessed by the programmer after having moved them, and he might wish to reuse them (or, if the lvalue is an lv-ref parameter, he might not know if whoever passed him this lvalue does), even though, admittedly, I haven't seen many convincing examples of such cases.
I agree that the cases for which a destructive move is appropriate seem by far the dominant ones (even if it's no different than a conservative move in many of them). If you're asking for a rationale as to why the semantics are the way they are, then you're talking to the wrong person. I am sure David Abrahams can give a much more credible answer than I can. Some of these issues are discussed in N1377, in particular, see the "Alternative Move Designs" -> "Destructive move semantics" section
(http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2002/n1377.htm#Alternativ... move designs) (Also note that the "Copy vs Move" section literally states: "The only requirement (from the move-ctor PS) is that the (source PS) object remain in a self consistent state (all internal invariants are still intact).")
FWIW, I'm playing with the idea of writing a destructive-move proposal (although what I'm reffering to as destructive-move is a bit different than what the move proposal calls a destructive-move), and these kind of cases are exactly what I wish to address. If you have actual numbers that demonstrate how bad is the impact on Spirirt and co, I'd love to hear them.
Those are very good points, but I think you are being too pedantic. The example you posted concerns a class hierarchy (base / derived). The paper you cited mentions:
"When dealing with class hierarchies, destructive move semantics becomes problematic."
That is certainly not the case with recursive_wrapper. Do you see a reasonable cause for danger with (a non-hierarchical struct such as) recursive_wrapper?
Finally, it would be best if you quote from the standard. That paper you posted is quite old (from 2002).
Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Not sure about being overly pedantic. I think I'm being appropriately paranoid :) This paper is indeed a bit old, but for the most part it's not outdated. AFAIK the general concept of move semantics hasn't been significantly changed since then, the only changes that underwent concerned the actual core language technicalities (i.e. what rv-refs can and can't bind to, implicitly declared move-ctors, etc'...) It's a little hard to quote the standard here, simply because it doesn't actually define the term "move semantics" - only the semantics of the related core language facilities. As I said, I believe these semantics imply a conservative move. As for the library, see 17.6.3.1 [utility.arg.requirements] tables 20 [moveconstructible] and 22 [moveassignable]. The semantics of rvalues have similar effects on any kind of suboject: struct X { // the user is allowed to modify this member recursive_wrapper<T> rw; // might as well be a // recursive variant, I'm // just being lazy. X(): rw(...) {} ~X() { if(predicate(rw.get())) do_something(); else do_something_else(); } }; int main() { recursive_wrapper<T> rw = X().rw; // oops } At the end of the day move semantics are mostly a convention. If we agree on a conservative move, then it's legitimate for the programmer to expect that moved-from objects remain valid. Period. And if we don't, well... then we have a problem. You don't have to convince me that in most cases a destructive move is fine, and you don't have to convince me that in cases like this one it's also desirable. I just think that the correct way to tackle this is by extending the lanuage to naturally allow it and not by abusing the current facilities. -- Paul Smith

On 1/7/2013 12:14 PM, Paul Smith wrote:
A recursive_wrapper is not a pointer. It's a value-like wrapper that is assumed to always contain a valid object. The move constructor should leave the moved-from recursive_wrapper in a valid state, which precludes nullifying it.
Paul, I think you are confused about move semantics. Joel and Hartmut are right. Nobody should ever be seeing a moved-from object. Move constructors and assignment operators exist precisely to enable this kind of optimization. -- Eric Niebler BoostPro Computing http://www.boostpro.com

on Mon Jan 07 2013, Eric Niebler <eric-AT-boostpro.com> wrote:
On 1/7/2013 12:14 PM, Paul Smith wrote:
A recursive_wrapper is not a pointer. It's a value-like wrapper that is assumed to always contain a valid object. The move constructor should leave the moved-from recursive_wrapper in a valid state, which precludes nullifying it.
Paul, I think you are confused about move semantics. Joel and Hartmut are right. Nobody should ever be seeing a moved-from object.
No, Paul is right on the money. The moved-from state is part of the object's invariant, and objects in that state may become exposed in several ways (e.g. use std::remove on a sequence of them). If you want a given class to have an empty moved-from state, that's fine... but you can't claim the object is invalid after a valid operation is used on it. You have to document the client-visible consequences of that state as part of the object's behavior, etc.
Move constructors and assignment operators exist precisely to enable this kind of optimization.
True, but irrelevant. :-) -- Dave Abrahams BoostPro Computing Software Development Training http://www.boostpro.com Clang/LLVM/EDG Compilers C++ Boost

On 1/8/2013 1:11 PM, Dave Abrahams wrote:
on Mon Jan 07 2013, Eric Niebler <eric-AT-boostpro.com> wrote:
On 1/7/2013 12:14 PM, Paul Smith wrote:
A recursive_wrapper is not a pointer. It's a value-like wrapper that is assumed to always contain a valid object. The move constructor should leave the moved-from recursive_wrapper in a valid state, which precludes nullifying it.
Paul, I think you are confused about move semantics. Joel and Hartmut are right. Nobody should ever be seeing a moved-from object.
No, Paul is right on the money. The moved-from state is part of the object's invariant, and objects in that state may become exposed in several ways (e.g. use std::remove on a sequence of them). If you want a given class to have an empty moved-from state, that's fine... but you can't claim the object is invalid after a valid operation is used on it. You have to document the client-visible consequences of that state as part of the object's behavior, etc.
Yes, I'd already figured out that Paul knows what he's talking about, and I apparently don't, so I've shut up. :-) This discussion has been fascinating, and I've learned a lot. Thanks.
Move constructors and assignment operators exist precisely to enable this kind of optimization.
True, but irrelevant. :-)
I admit that I find this somewhat disappointing. I'm now readjusting my expectations of move semantics. -- Eric Niebler BoostPro Computing http://www.boostpro.com

On 01/05/2013 05:40 PM, Joel de Guzman wrote:
Hi,
I just looked at the current state of variant and noticed that the implementation of recursive_variant's move ctor seems to be not as optimal as I hoped. The current implementation as written is:
recursive_wrapper<T>::recursive_wrapper(recursive_wrapper&& operand) : p_(new T( detail::variant::move(operand.get()) )) { }
Unless I am mistaken, I think the heap allocation is not needed and the target should simply grab the pointer and mark the source pointer as 0 (with additional checks for 0 in the dtor):
<snip>
Thoughts? I can commit this patch if it is acceptable.
Regards,
Hi Joel - I think the patch looks good and I would be extremely happy to see it adopted. I have several libraries that are heavy users of recursive_wrapper and would benefit from this change. Does variant have an active maintainer? michael -- Michael Caisse ciere consulting ciere.com

on Sat Jan 05 2013, Joel de Guzman <djowel-AT-gmail.com> wrote:
Hey Dave,
There's an interesting discussion about move semantics going on on the boost list. I thought I understood the implications of move, but it turns out I don't, and it's been a little disconcerting. I was wondering if you'd weigh in, if you can find the time.
Things get interesting here: http://thread.gmane.org/gmane.comp.lib.boost.devel/235534/focus=237666
I'm a little pressed ATM. If you could Skype or call me and give me the skinny that would make it easier. -- Dave Abrahams BoostPro Computing Software Development Training http://www.boostpro.com Clang/LLVM/EDG Compilers C++ Boost
participants (19)
-
Alec Ross
-
Andrey Semashev
-
Antony Polukhin
-
Dave Abrahams
-
Eric Niebler
-
Giovanni Piero Deretta
-
Hartmut Kaiser
-
Joel de Guzman
-
Larry Evans
-
Mathieu Champlon
-
Michael Caisse
-
Nevin Liber
-
paul Fultz
-
Paul Smith
-
Peter Dimov
-
Philipp Moeller
-
Rob Stewart
-
Vicente Botet
-
Vicente J. Botet Escriba