Re: [boost] [move][container] Review Request (new versions of Boost.Move and Boost.Container in sandbox and vault)

Subject: Re: [boost] [move][container] Review Request (new versions of Boost.Move and Boost.Container in sandbox and vault)
From: Thomas Klimpel (Thomas.Klimpel_at_[hidden])
Date: 2009-10-15 21:10:35

I know that this discussion is old, and that a review manager for the review is still missing. However, since the article series at "http://cpp-next.com" has now covered quite some ground, I somehow guess that a review manager will be found sooner or later. (More precisely, I guess that the article series on value semantics, moving and all that will soon be finished, and that then a review manager will appear somehow...)

And because the article series has covered quite some ground now, it's also easier for me to keep up with the discussion.

David Abrahams wrote:
> Thomas Klimpel wrote:
> > If the user wants to ensure that the threads get canceled, he can call "v2.clear();"
> > himself.
> 
> I disagree.  It's OK if transforming "x" into "move(x)" changes
> semantics with respect to x.  It isn't OK if it changes semantics with
> respect to objects other than x.  In particular,
> 
>         y = x;  ===>   y = move(x);
> 
> should not change anything about what happens with y.  That's just too
> capricious.

I'm not sure about this. An alternative would be to transform "x" into "X(move(x))" (move constructor call). At least this solution would be more general than the suggestion to call "v2.clear();". Yes, I know that the move constructor "X(X&&x)" may not exist for a general "x". But the case "X(move(x))" is still quite common, especially most "return std::move(x);" statements will probably happen in functions that return a value, so the move constructor call will happen implicitly.

> > So the question for me would be whether there are (generic) algorithms
> > (because they can't call x.clear() themselves) that will use more
> > resources if the move-assignment doesn't call x.clear() for
> > them. After all, those (generic) algorithms are the real clients that
> > will call std::move (x) and will have to deal with non-temporary moved
> > from objects.
> 
> Here's another example:
> 
>        // C++03
>        template <class Pair, class T>
>        T& replace2nd(Pair& c, T const& x)
>        {
>            return c.second = x;
>        }
> 
>        pair<thread,thread> threads;
>        ...
> 
>        long_running_operation(replace2nd(threads, new_thread())); // (**)
> 
> 
> OK, let's move-enable replace2nd:
> 
>        // addtional overload
>        template <class Pair, class T>
>        T& replace2nd(Pair& c, T&& x)
>        {
>            return c.second = move(x);
>        }
> 
> can't clear there; we don't know that T has a clear().
> 
> Now we've just changed the semantics of the line marked (**), since the
> old thread won't be canceled until the long-running operation is
> complete.

Interesting. I have to admit that I assumed the temporary objects for the arguments to replace2nd would be destroyed before the return value of replace2nd gets used, but this assumption was probably wrong. I guess the C++ standard just says that they get destroyed after the entire expression is evaluated. Just a simple question for my education: Does the C++ standard explicitly guarantees that the temporary objects don't get destroyed before the entire expression is evaluated, or is the exact moment of the destructor calls simply unspecified?

The appearance of move in the template function doesn't invalidate this counter example, because the generic code can't know whether T has a move-constructor that could be called to ensure a destructor call.

> Replace "thread" with "lock" and now you can easily deadlock, because
> you've messed with tghe locking/unlocking order.  I'm pretty sure we can
> massage this example easily to eliminate even the appearance of move()
> in the code.  Oh, sure:
> 
>        // C++03
>        template <class Pair, class F>
>        T& replace2nd(Pair& c, F const& f)
>        {
>            return c.second = f(); // f might return an rvalue
>        }

If "f" just returns an rvalue (not an rvalue-reference), it will be destroyed before replace2nd returns, and nothing will go wrong. OK, so you suggest that "f" might return an rvalue-reference? This might actually be an extremely good point, even if the C++ language would be changed to invalidate the first counter example. But I wonder a bit where "f" wants to store the object corresponding to the rvalue-reference, and what "f" plans to do when it gets called the next time. I somehow get the impression that "f" won't be very thread safe.

> > The words "..." don't forbid to implement move-assignment as swap
> > so if the programmer expects something
> > incompatible with this implementation, he is relying on undefined
> > behavior.
>
> Huh? How do you reach that conclusion?

If it is not undefined behavior, it should be possible to point out where this behavior is defined. Thanks to your article series, I now know at least one place where it is defined:
http://cpp-next.com/archive/2009/09/your-next-assignment/

So what is the conclusion? Do I really still pretend to need a convincing counter example? I certainly had hoped for a different kind of counter example, more something on the algorithmic side (something like move_iterator used with some existing standard algorithm running into trouble when move-assignment is implemented as swap).

The first counter example is really disappointing from my point of view. It's certainly not good style to call a counter example a language defect, but if "replace2nd" had the signature
       template <class Pair, class T>
       T& replace2nd(Pair& c, T x)
and "copy elision" kicked in, the destructor call would happen before the return value of replace2nd gets used. So even changing the signature to
       template <class Pair, class T>
       T& replace2nd(Pair& c, const T& x)
could have surprising side effects by changing the exact moment when the destructor of the object returned by new_thread() gets called.

The second counter example was certainly more interesting, because it shows that I would have to strongly discourage functions from returning rvalue-references, if I want to implement move-assignment as swap. Now I'm not sure whether returning rvalue-references is a good idea, but I have to admit that generally discouraging it seems to be strange.

So I'm left with the impression that there might be quite some good reasons not to implement move-assignment as swap, but no really conclusive counter examples against it. On the other hand, as long as the absence of conclusive counter examples can't be "proved", ruling that move-assignment may generally be implemented as swap will be risky or worse.

Regards,
Thomas