16
//code from https://skillsmatter.com/skillscasts/2188-move-semanticsperfect-forwarding-and-rvalue-references
class Widget {
public:
    Widget(Widget&& rhs)
        : pds(rhs.pds) // take source’s value
    { 
        rhs.pds = nullptr;  // why??
    }

private:
    struct DataStructure;
    DataStructure *pds;
};

I can't understand the reason for setting rhd.pds to nullptr .

What will happen if we remove this line : rhs.pds = nullptr;

uchar
  • 2,552
  • 4
  • 29
  • 50
  • 3
    Is there a destructor you removed that would have tried to free the pointer if it wasn't cleared? – user2357112 Mar 01 '14 at 11:56
  • so that no two objects members are pointing to same memory locations... – HadeS Mar 01 '14 at 11:56
  • The link requires a paid login. :( (Also, it sounds like a "skill smatter" is process that would give you a "smattering of skills.") – Potatoswatter Mar 01 '14 at 11:59
  • 1
    If the ownership isn't supposed to be shared its your way of telling the source of the **move** operation it no longer has a pointer to manage; `this` object does. And if that is the case `std::unique_ptr pds;` for the member and `pds(std::move(rhs.pds))` would be preferred. – WhozCraig Mar 01 '14 at 12:14
  • @Potatoswatter No just sign up .It doesn't require paying ... – uchar Mar 01 '14 at 12:17

3 Answers3

18

Some details of the class have been removed. In particular, the constructor dynamically allocates the DataStructure object and the destructor deallocates it. If, during a move, you just copied the pointer from one Widget to another, both Widgets would have pointers to the same allocated DataStructure object. Then, when those objects are destroyed, they would both attempt to delete it. This would give undefined behaviour. To avoid this, the Widget that is being moved from has its internal pointer to set to nullptr.

This a standard pattern when implementing a move constructor. You want to move ownership of some dynamically allocated objects from one object to another, so you need to make sure the original object no longer owns those allocated objects.

Diagrammatically, you start off with this situation, wanting to move ownership of the DataStructure from one Widget to the other:

    ┌────────┐        ┌────────┐
    │ Widget │        │ Widget │
    └───╂────┘        └────────┘
        ┃
        ▼
 ┌───────────────┐
 │ DataStructure │
 └───────────────┘

If you just copied the pointer, you'd have:

    ┌────────┐        ┌────────┐
    │ Widget │        │ Widget │
    └───╂────┘        └───╂────┘
        ┗━━━━━━━━┳━━━━━━━┛
                  ▼
         ┌───────────────┐
         │ DataStructure │
         └───────────────┘

If you then set the original Widget pointer to nullptr, you have:

    ┌────────┐         ┌────────┐
    │ Widget │         │ Widget │
    └────────┘         └───╂────┘
                           ┃
                           ▼
                  ┌───────────────┐
                  │ DataStructure │
                  └───────────────┘

Ownership has successfully been transferred, and when both Widgets can be destroyed without causing undefined behaviour.

Martin York
  • 257,169
  • 86
  • 333
  • 562
Joseph Mansfield
  • 108,238
  • 20
  • 242
  • 324
  • so I wonder that for this situation, is there any difference between the expressions `rhs.pds = nullptr;` and `pds = nullptr` I think I can write `pds = nullptr` instead of `rhs.pds = nullptr;`. can I ? – askque Jan 18 '16 at 22:33
  • @askque No, rhs.pds will cease to exist so its pointer must be set to null so its destructor does not deallocate memory. pds will continue to exist so its pointer must have the correct value. – mcabreb Jan 16 '20 at 13:59
2

The DataStructure object is likely "owned" by the Widget, and resetting the pointer prevents it from being accidentally deleted when the Widget is destroyed.

Alternately, it's conventional to reset objects to an "empty" or "default" state when they are moved-from, and resetting the pointer is a harmless way to follow the convention.

Potatoswatter
  • 134,909
  • 25
  • 265
  • 421
1
class Widget {
  public:
    Widget(Widget&& rhs)
       : pds(rhs.pds) // take source’s value
    { 
        rhs.pds = nullptr;  // why??
    }
    ~Widget() {delete pds}; // <== added this line

private:
    struct DataStructure;
    DataStructure *pds;
};

I added a destructor in the above class.

Widget make_widget() {
    Widget a;
    // Do some stuff with it
    return std::move(a);
}

int main {
    Widget b = make_widget;
    return 0;
}

To illustrate what would happen if you remove the nullptr assignment, check the above methods. A widget a would be created in a helper function and assigned to widget b.

Since widget a goes out of scope its destructor its called, which deallocates memory, and you are left with widget b which is pointing to invalid memory address.

If you assign nullptr to rhs, a destructor is also called, but since delete nullptr does nothing all is good :)

Blaz Bratanic
  • 2,279
  • 12
  • 17
  • 2
    Why return std::move(a) ?! – uchar Mar 01 '14 at 12:13
  • 1
    @omid To inhibit return value optimization :-) – juanchopanza Mar 01 '14 at 13:07
  • 1
    @Blaz, as omid and juanchopanza are pointing out `return move(a);` is usually a bad thing. The compiler (usually) implicitly will treat the return value as an rvalue. So you don't gain anything from `move`. But, more importantly, there is a further optimization that is possible (called RVO, faster than moving), and this optimization is disabled if you do an explicit `move`. There are cases where it does make sense, but if you blindly do it, you will slow things down more often than you speed them up. – Aaron McDaid Mar 01 '14 at 13:12
  • 1
    There's something in C++11 (can't remember where) that says if a return value is eligible for RVO, which this one is, then the return value is required to be constructed by move from the return expression. That move is still eligible for elision though, which I believe `move(a)` isn't because the `return` expression is no longer the name of a variable. – Steve Jessop Mar 01 '14 at 13:17