Re: [boost] [variant] Basic rvalue and C++11 features support

Subject: Re: [boost] [variant] Basic rvalue and C++11 features support
From: Philipp Moeller (philipp.moeller_at_[hidden])
Date: 2013-01-08 12:30:06

Paul Smith <pl.smith.mail_at_[hidden]> writes:

> On Tue, Jan 8, 2013 at 4:18 PM, Hartmut Kaiser <hartmut.kaiser_at_[hidden]> wrote:
>>> On Tue, Jan 8, 2013 at 2:49 AM, Hartmut Kaiser <hartmut.kaiser_at_[hidden]>
>>> 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.