Why move ctor not called for construction from temporary object (result of operator+)?

Question

Now that it's answered: Don't bother reading this question, it's a bit lengthy and probably not worth your time. There were bugs in my code, and that was the reason why the move constructor wasn't called. Check the answers for details. Keep in mind that RVO and NRVO (Named Return Value Optimization) might account for the calls not happening as you expect.

---

I expect the move ctor to be called for this line, but the copy ctor is called instead:

Ding d3 = d1 + d2;

The Ding class has a user-defined move ctor and on operator+ overload. The reason I expect the move ctor to be called is that the operator+ returns a temporary object, an rvalue reference, so the move optimization could happen.

Everything I'm writing here might be wrong as I'm a C++ beginner. Here's the code:

// Copied and modified code from here: https://stackoverflow.com/a/3109981
#include <iostream>
#include <cstring>

struct Ding {
    char* data;

    Ding(const char* p) {
        std::cout << " ctor for: " << p << "\n";
        size_t size = strlen(p) + 1;
        data = new char[size];
        memcpy(data, p, size);
    }

    ~Ding() {
        std::cout << " dtor for: " << data << "\n";
        delete[] data;
    }

    Ding(const Ding& that) {
        std::cout << " copy for: " << that.data << "\n";
        size_t size = strlen(that.data) + 1;
        data = new char[size];
        memcpy(data, that.data, size);
    }

    Ding(Ding&& that) {
        std::cout << " MOVE for: " << that.data << "\n";
        data = that.data;
        that.data = nullptr;
    }

    Ding& operator=(Ding that) {
        std::cout << " assignment: " << that.data << "\n";
        std::swap(data, that.data);
        return *this;
    }

    Ding& operator+(const Ding that) const {
        std::cout << " plus for: " << that.data << "\n";
        size_t len_this = strlen(this->data);
        size_t len_that = strlen(that.data);
        char * tmp = new char[len_this + len_that + 1];
        memcpy( tmp,          this->data, len_this);
        memcpy(&tmp[len_this], that.data, len_that + 1);
        Ding * neu = new Ding(tmp);
        return *neu;
    }
};

void print(Ding d) {
    std::cout << "  (print): " << d.data << std::endl;
}

int main(void) {
    std::cout << "putting a Ding on the stack\n";
    Ding d1("jajaja");
    std::cout << "calling print routine\n";
    print(d1);
    std::cout << "putting a second Ding on the stack\n";
    Ding d2("nein");
//  std::cout << "calling print routine\n";
//  print(d2);
    std::cout << "Putting a third Ding on the stack, init from + op ...\n";
    std::cout << "... so expecting so see MOVE ctor used ...\n";
    Ding d3 = d1 + d2;
//  std::cout << "calling print routine\n";
//  print(d3);
    std::cout << "End of main, dtors being called ...\n";
}

The compiler invocations (on Win7) for VC2010 Express and MinGW (GCC 4.6) are as follows:

cl /nologo /W4 /EHsc /MD move-sem.cpp
g++ -std=c++0x move-sem.cpp -o move-gcc.exe

Both binaries produce the same output (sans order of destruction at end of program):

putting a Ding on the stack
 ctor for: jajaja
calling print routine
 copy for: jajaja
  (print): jajaja
 dtor for: jajaja
putting a second Ding on the stack
 ctor for: nein
Putting a third Ding on the stack, init from + op ...
... so expecting so see MOVE ctor used ...
 copy for: nein
 plus for: nein
 ctor for: jajajanein
 dtor for: nein
 copy for: jajajanein
End of main, dtors being called ...
 dtor for: jajajanein
 dtor for: nein
 dtor for: jajaja

To recall what the question was after this long text: Why isn't the move constructor called for Ding d3 = d1 + d2;?

I'm aware that there are other questions as to why move ctors aren't getting called but I cannot map their answers to this case.

• Why doesn't this C++0x code call the move constructor?
• Why is this code trying to call the copy constructor?
• Move constructor is not getting called in C++0x

Update

I changed the program as follows as per David Rodriguez' comments:

--- move-sem.cpp.orig   2012-03-17 17:00:56.901570900 +0100
+++ move-sem.cpp        2012-03-17 17:01:14.016549800 +0100
@@ -36,15 +36,14 @@
                return *this;
        }

-       Ding& operator+(const Ding that) const {
+       Ding operator+(const Ding that) const {
                std::cout << " plus for: " << that.data << "\n";
                size_t len_this = strlen(this->data);
                size_t len_that = strlen(that.data);
                char * tmp = new char[len_this + len_that + 1];
                memcpy( tmp,          this->data, len_this);
                memcpy(&tmp[len_this], that.data, len_that + 1);
-               Ding * neu = new Ding(tmp);
-               return *neu;
+               return tmp;
        }
 };

I then recompiled the program using the compiler invocations mentioned above and got an output where one copy (copy for: jajajanein) was removed. I then tried the following line:

g++ -std=c++0x -fno-elide-constructors move-sem.cpp -o move-gcc.exe

And tata! Now I'm seeing the move ctor at work! ... But I think there's another error now, the output of that new move-gcc.exe doesn't list the dtor invocations any more:

putting a Ding on the stack
 ctor for: jajaja
calling print routine
 copy for: jajaja
  (print): jajaja
 dtor for: jajaja
putting a second Ding on the stack
 ctor for: nein
Putting a third Ding on the stack, init from + op ...
... so expecting so see MOVE ctor used ...
 copy for: nein
 plus for: nein
 ctor for: jajajanein
 MOVE for: jajajanein
 dtor for:

Second Update

I replaced the bad operator+ with the following (possibly equally bad) code:

Ding& operator+=(const Ding & rhs) {
    std::cout << " op+= for: " << data << " and " << rhs.data << "\n";
    size_t len_this = strlen(this->data);
    size_t len_that = strlen(rhs.data);
    char * buf = new char[len_this + len_that + 1];
    memcpy( buf,         this->data, len_this);
    memcpy(&buf[len_this], rhs.data, len_that + 1);
    delete[] data;
    data = buf;
    return *this;
}

Ding operator+(const Ding & rhs) const {
    Ding temp(*this);
    temp += rhs;
    return temp;
}

I also removed the following line from the destructor, and it stopped the program from terminating abnormally:

std::cout << " dtor for: " << data << "\n";

The move constructor is now being called when compiling with MSVC and with g++ -std=c++0x -fno-elide-constructors.

Answer 1

    Ding * neu = new Ding(tmp);
    return *neu;

This is wrong. You are dynamically allocating a Ding and then forcing a copy of it. Because the Ding is dynamically allocated, you are leaking it, it's lifetime extends beyond the return statement and the compiler cannot move from it. Note that you are not returning a temporary.

Change that to:

    return Ding(tmp);

Or even:

    return tmp;

As your constructor taking a const char* is not explicit, the compiler will use it to create a new Ding object. In both cases, the lifetime of the temporary does not extend beyond the return statement and the compiler will move.

(This answer assumes that you understand that the copy from the returned object to the d3 has been elided, if that is where you expected the move, then the compiler did something better: avoid the operation altogether).

EDIT As DeadMG has written a canonical form but it includes errors I will follow up on that:

There is much to say about operator overloading, but a common recommendation (the one I give and follow) is to implement operatorX= as a member function (it is an operation applied to the left hand side) and then implement operator+ as a free function in terms of the former. In C++11 that would be:

class X {
   X& operator+=( X const & ); // we do not modify the rhs internally
};
X operator+( X lhs, X const & rhs ) {
  lhs += rhs;                  // reuse implementation
  return lhs;
}

Some things to note: operator+ is symmetric with respect to types as it is a free function. All implicit conversions that can happen on the right hand side are also available in the lhs. In your particular case, Ding is implicitly convertible from const char*, which means that by having a free function operator+ you can write:

Ding d( "A" );
const char* str = "B";
d + d;     // no conversions
d + str;   // conversion on the right hand side
str + d;   // conversion on the left hand side

By defining operator+= as a public member function you need to write a single implementation and that implementation can be reused, so you get two operators for the cost of one (and three extra lines of code that are trivial).

Argument by value and return by value. This enables the compiler to elide the copy to the parameter if the argument is a temporary. As there is a move constructor, there will not be any internal copy either, the argument will be modified and moved to the return object. (This second copy cannot be elided).

I wrote a bit more than this on operator overloading here some time ago... It does not explicitly deal with optimizations (moving) but there are other posts there that deal with the C++03 version of it. From that to C++11 features you should be able to fill in the blanks.

Answer 2

There is nothing to be moved.

You cannot move from d1 or d2, because that would destroy them. And you cannot move the return value of the operator+, because that is a reference.

Answer 3

You have not written a correct addition operator. The canonical form is

Ding operator+(Ding other) const {
    other += this;
    return other; 
    // return std::move(other) if you're on MSVC 
    // which sometimes doesn't do this properly
}
Ding& operator+=(const Ding& other);

Your code exhibits many problems, like memory leaks, which also result from having bad operator overloads.

Also, do not forget the potential impacts of RVO and NRVO on the expected output.