std::move changes address of variables?

Question

I found unexpected (at least for me) behavior.

class A
{
    char  _text[100];
    char* _beg;
    char* _end;
public:
    explicit A(char* text, size_t tsize) : _beg(&text[0]), _end(&text[std::min(tsize, 99)]) 
    {
        memcpy(_text, text, std::min(tsize, 99));
        *_end = '\0';
    }
    inline std::string get_text()
    {
        return std::move(std::string(_beg, _end));  
    }
};

After that somewhere in code I do that:

  A* add_A(A&& a)
  {
     list_a.push_back(std::move(a));
     return &(list_a.back());
  }

  std::list<A> list_a;
  {
      add_A(A("my_text", 7));
      list_a.back().get_text(); //returns "my_text"
  }
  list_a.back().get_text(); //returns trash

As only I move this class (using std::move), and call get_text() of object that was moved, I get trash, as if after movement address of variable _text changed, and so _beg and _end points to nowhere.

Does address of variables really can be changes after std::move (I thought move don't really move object, it was invented for that)?

If it can be changed, what is usual pattern to handle it (to change pointers accordingly)?

If it can't be change, may that behavior happens because I try to move such object to std::list (and so there somehow happens copying, it changes address of variables and makes pointers point to wrong positions)?

Answer 1

Moving in C++ is just a specialized form of copy, where you modify the data in the object being moved from. That's how unique_ptr works; you copy the pointer from one unique_ptr object to the other, then set the original value to NULL.

When you move an object, you are creating a new object, one who gets its data from another object. The address of members don't "change"; it's simply not the same object.

Because you didn't write a copy/move constructor, that means the compiler will write one for you. And all they do is copy each element. So the newly moved-to object will have pointers that point to the old object.

An object that is about to be destroyed.

It's like moving into a house that happens to look identical to your old one. No matter how much it looks like your old house, it isn't. You still have to change your address, since it's a new house. So too must the addresses of _beg and _end be updated.

Now, you could create a move constructor/assignment operator (along with a copy constructor/assignment operator) to update your pointers. But quite frankly, that's just wallpapering over bad design. It's not a good idea to have pointers to subobjects within the same object if you can help it. Instead of begin/end pointers, just have an actual size:

class A
{
    char  _text[100];
    size_t _size;
public:
    explicit A(char* text, size_t tsize) : _size(tsize)
    {
        strncpy(_text, text, 100);
    }
    inline std::string get_text()
    {
        return std::string(_text, _size); //Explicit `move` call is unnecessary
    }
};

This way, there is no need to store begin/end pointers. Those can be synthesized as needed.

Answer 2

std::move has no moving parts, it simply promotes the input parameter to an rvalue reference -- remember that inside the body of foo(T&& t) { ... } the use of t by name evaluates as an lvalue (reference to rvalue).

inline std::string get_text()
{
    return std::move(std::string(_beg, _end));
}

Breaking this down:

std::string(_beg, _end);

creates an anonymous, temporary std::string object constructed from _beg to _end. This is an rvalue.

std::move(...);

forcibly promotes this to an rvalue reference and prevents the compiler from performing return-value optimization. What you want is

return std::string(_beg, _end);

See assembly code comparison

You probably also want to use

list_a.emplace_back(std::move(a));

Unfortunately, there are two flaws in this approach.

The simpler is that the term moving can be a bit misleading, it sounds very one way. But in practice it is often a two way swap: the two objects exchange properties so that when the temporary object goes out of scope it performs cleanup of whatever the other object previously owned:

struct S {
    char* s_;
    S(const char* s) : s_(strdup(s)) {}
    ~S() { release(); }
    void release() { if (s_) free(s_); }
    S(const S& s) : s_(strdup(s.s_)) {}
    S(S&& s) : s_(s.s_) { s.s_ = nullptr; }
    S& operator=(const S& s) { release(); s_ = strdup(s); return *this; }
    S& operator=(S&& s) { std::swap(s_, s.s_); return *this; }
};

Note this line:

    S& operator=(S&& s) { std::swap(s_, s.s_); return *this; }

When we write:

S s1("hello");
s1 = S("world");

the second line invokes the move-assignment operator. The pointer for the copy of hello is moved into the temporary, the temporary goes out of scope and is destroyed, the copy of "hello" is freed.

Doing this swap with your array of characters is significantly less efficient than a one-way copy would be:

struct S {
    char s_[100];
    S(const S& s) {
        std::copy(std::begin(s.s_), std::end(s.s_), std::begin(s_));
    }
    S(S&& s) {
        char t_[100];
        std::copy(std::begin(s.s_), std::end(s.s_), std::begin(t_));
        std::copy(std::begin(s_), std::end(s_), std::begin(s.s_));
        std::copy(std::begin(t_), std::end(t_), std::end(s_));
    }
};

You don't have to do this, the rvalue parameter only needs to be in a safe to destroy state, but the above is what the default move operators are going to do.

The disasterous part of your code is that the default move operator is naive.

struct S {
    char text_[100];
    char *beg_, *end_;
    S() : beg_(text_), end_(text_ + 100) {}
};

Consider the following copy-construction:

S s(S());

What does s.beg_ point to?

Answer: it points to S().text_, not s.text_. You would need to write a copy constructor that copied the contents of text_ and then pointed its own beg_ and end_ to its own text_ rather than copying the source values.

The same problem occurs with the move operator: it will move the contents of text_ but it will also move the pointers, and have no clue that they are relative.

You'll either need to write copy/move constructors and assignment operators, or you could consider replacing beg_ and end_ with a single size_t size value.

But in either case, move is not your friend here: you're not transferring ownership or performing a shallow copy, all of your data is inside your object.