Is this correct usage of C++ 'move' semantics?

Question

Tonight I've been taking a look at some code I've been working on over the last few days, and began reading up on move semantics, specifically std::move. I have a few questions to ask you pros to ensure that I am going down the right path and not making any stupid assumptions!

Firstly:

1) Originally, my code had a function that returned a large vector:

template<class T> class MyObject
{
public:
    std::vector<T> doSomething() const;
    {
        std::vector<T> theVector;

        // produce/work with a vector right here

        return(theVector);
    }; // eo doSomething
};  // eo class MyObject

Given "theVector" is temporary in this and "throw-away", I modified the function to:

    std::vector<T>&& doSomething() const;
    {
        std::vector<T> theVector;

        // produce/work with a vector right here

        return(static_cast<std::vector<T>&&>(theVector));
    }; // eo doSomething

Is this correct? Any pitfalls in doing it this way?

2) I noticed in a function I have that returns std::string that it automatically called the move constructor. Debugging in to Return of the String (thankyou, Aragorn), I noticed it called an explicit move constructor. Why is there one for the string class and not vector?

I didn't have to make any modifications to this function to take advantage of move semantics:

// below, no need for std::string&& return value?
std::string AnyConverter::toString(const boost::any& _val) const
{
    string ret;
    // convert here
    return(ret); // No need for static_cast<std::string&&> ?
}; // eo toString

3) Finally, I wanted to do some performance tests, is the amazingly-fast results I got because of std::move semantics or did my compiler (VS2010) do some optimizing too?

(Implementation of _getMilliseconds() omitted for brevity)

std::vector<int> v;
for(int a(0); a < 1000000; ++a)
    v.push_back(a);

std::vector<int> x;
for(int a(0); a < 1000000; ++a)
    x.push_back(a);

    int s1 = _getMilliseconds();
std::vector<int> v2 = v;
    int s2 =  _getMilliseconds();
std::vector<int> v3 = std::move(x);
    int s3 =  _getMilliseconds();

    int result1 = s2 - s1;
    int result2 = s3 - s2;

The results were, obviously, awesome. result1, a standard assignment, took 630ms. The second result, was 0ms. Is this a good performance test of these things?

I know some of this is obvious to a lot of you, but I want to make sure I understand the semantics right before I go blazer on my code.

Thanks in advance!

Answer 1

A reference is still a reference. In the same way you cannot return a reference to a local in C++03 (or you get UB), you can't in C++0x. You'll end up with a reference to a dead object; it just happens to be an rvalue reference. So this is wrong:

std::vector<T>&& doSomething() const
{
    std::vector<T> local;

    return local; // oops
    return std::move(local); // also oops
}

You should just do what you saw in number two:

// okay, return by-value 
std::vector<T> doSomething() const
{
    std::vector<T> local;

    return local; // exactly the same as:
    return std::move(local); // move-construct value
}

Variables local to a function are temporary when you return, so there's no need to change any of your code. The return type is the thing responsible for implementing move semantics, not you.

You want to use std::move to explicitly move something, when it wouldn't be done normally, like in your test. (Which seems to be fine; was that in Release? You should output the contents of the vector, or the compiler will optimize it away.)

If you want to learn about rvalue references, read this.

Answer 2

return(theVector);

This already moves implicitly due to a special language rule, because theVector is a local object. See section 12.8 paragraphs 34 and 35:

When certain criteria are met, an implementation is allowed to omit the copy/move construction of a class object, even if the copy/move constructor and/or destructor for the object have side effects. In such cases, the implementation treats the source and target of the omitted copy/move operation as simply two different ways of referring to the same object, and the destruction of that object occurs at the later of the times when the two objects would have been destroyed without the optimization. This elision of copy/move operations, called copy elision, is permitted in the following circumstances (which may be combined to eliminate multiple copies):

— in a return statement in a function with a class return type, when the expression is the name of a non-volatile automatic object with the same cv-unqualified type as the function return type, the copy/move operation can be omitted by constructing the automatic object directly into the function's return value

[...]

When the criteria for elision of a copy operation are met and the object to be copied is designated by an lvalue, overload resolution to select the constructor for the copy is first performed as if the object were designated by an rvalue.

Note that you must return a std::vector<T> (by value), not a std::vector<T>&& (by reference).

But why the parenthesis? return is not a function:

return theVector;

Answer 3

To add to GMan's answer: even if you change the return type to be std::vector<T> (without any references, otherwise you'll get UB), your change of return expression in "1)" will never make the performance better, but might make it a bit worse. As std::vector has move constructor, and you return a local object, vector's copy constructor will not be called, no matter you wrote return theVector;, return static_cast<std::vector<T>&&>(theVector);, or return std::move(theVector). In the last two cases the compiler will be forced to call the move constructor. But in the first case it has the freedom to optimized out the move altogether, if it can do NRVO for that function. If NRVO isn't possible for some reason, only then the compiler will resort to calling the move constructor. So don't change return x; to return std::move(x); if x is a local non-static object in the function being returned from, otherwise you'll prevent the compiler from using another optimization opportunity.

Answer 4

The standard way to move something is with std::move(x), not a static_cast. AFAIK, the Named Return Value Optimization is likely to kick in with returning a vector by value, so it would have performed well before move semantics as well.

Your performance test is a good illustration of how move semantics are good for performance: the copy-assignment has to copy a million elements, and the move-assignment essentially just swaps the vector's internal pointers around, which is a trivial word assignment or two, just a few cycles.