can I call destructor in move assignment operator?

Question

Is calling a d-tor inside move assignment operator good practice?

here some example code:

VectorList &operator = (VectorList &&other){
    ~VectorList(); // if this is not a good practice,
                   // I will need to paste whole d-tor here.

    _buffer     = std::move(other._buffer       );
    _dataCount  = std::move(other._dataCount    );
    _dataSize   = std::move(other._dataSize     );

    other._clear();

    return *this;
}

Should I use this code, or should I use swap() with move constructed object?

Have you considered creating a function to 'clean' the object and call this function both from the destructor and the move-assignement operator? — Lorenzo Belli, Sep 26 '15 at 11:41
Very similar to [Is move assignment via destruct+move construct safe?](http://stackoverflow.com/questions/13092240/is-move-assignment-via-destructmove-construct-safe?rq=1) — Bo Persson, Sep 26 '15 at 11:41
It is not usually the job of the *destructor* to return an object to an initialized state, so I would be uncomfortable doing this. Why not call `this->clear()`? — Galik, Sep 26 '15 at 12:07
@Nick as a side note, I hope you are aware that `std::move` doesn't "move" anything, it is just a cast to a rvalue reference. If `_buffer` is a raw pointer, then `_buffer = other._buffer;` is enough. — vsoftco, Sep 27 '15 at 04:49
yes I am but Scott Meyers "told" me you should do it with move, because of better style and to show intention. std::move is a simple cast... — Nick, Sep 27 '15 at 09:26
@LorenzoBelli No, it will add additional overhead on function calls. — ScienceDiscoverer, Aug 09 '23 at 14:52
@Nick You should listen to yourself more. Try to consider other's advices but also think with your own head a lot and make your own programming style you are comfortable with. — ScienceDiscoverer, Aug 09 '23 at 14:54

Yakk - Adam Nevraumont · Accepted Answer · 2015-09-26T12:21:48.347

4

~VectorList does more than run the code in the dtor body: it actually destroys the object.

After that, the storage is unused. You can construct a new object there using a constructor, but simply accessing members is going to either be undefined behaviour, or require language-lawyers to find the loophole that lets it be defined.

Even if defined, it is dangerous, as an exception thrown while an automatic storage object is destroyed is bad news. Plus if the assigned-to class is actually of derived type, the dtor call itself is UB!

Neither is a worthwhile approach. The benefits are too small.

The better alternative is copy-swap (which is at least easy to get correct: it can prevent some optimizations), or refactor out the 'clear' code from both the dtor and assignment. Then call it at both spots.

edited Sep 26 '15 at 12:21

answered Sep 26 '15 at 12:16

Yakk - Adam Nevraumont

262,606
27
330
524

"as an exception thrown while an automatic storage object is destroyed is bad news" --Who is doing that? Throwing from a destructor is almost never a good idea, unless there's no recourse left but to crash anyway. If you have somebody doing this, you're likely to have issues with destruction too, so I don't think defense against that is much of a consideration. – VoidStar Sep 26 '15 at 12:33
@voidstar Not "during destruction" but "while desteoyed" -- any time before the (hopefully following) ctor is finished. During reconstruction of the object, if an exception is thrown, you end up with a non constructed object. If in automatic storage, it will be double destroyed. – Yakk - Adam Nevraumont Sep 26 '15 at 12:49
I see what you mean, but the grammar in the answer text is ambiguous, it sounds like you're afraid of throwing destructors. Whereas instead you mean the real risk is a throwing operation inside the assignment operator code. BTW not considering the effect of an exception during assignment operator code is not safe for other reasons... mismatched field state could be equally bad too, like a wrong buffer length. – VoidStar Sep 26 '15 at 12:53
Manually calling the destructor doesn't do anything but run that function's code. The delete operator does not somehow get used, nor does the object become deallocated by prematurely going out of scope. Explicit destructor calls are commonly seen when destroying objects constructed with placement new. – Christopher Oicles Sep 26 '15 at 14:17
@chris No, you are wrong. I said nothing about `delete` or deallocation, so that part is right, but is irrelevant, as it disagrees with things I did not say. Calling the destructor does more than run the body of the destructor: it destroys the object, ending its lifetime. The storage remains, but no longer contains an object of the destroyed type. Valid uses of pointers to that storage are now extremely restricted, until a new object's lifetime begins in that storage. A function with the same body as a dtor *does not do that*. – Yakk - Adam Nevraumont Sep 26 '15 at 15:40
@Yakk Ok, I assumed you meant deallocation because this is the only action implementations perform outside executing the destructor body. However, the spec itself likely includes your object lifetime assertion. – Christopher Oicles Sep 26 '15 at 16:14
@chris also incorrect. The dtors of the parent types are run, as are those of members. In addition, vtable pointers can be messed with (or not). Finally, compilers can optimize around the fact that nearly all access in the interval between explicit dtor and ctor is UB, which can mean ... anything at all. – Yakk - Adam Nevraumont Sep 26 '15 at 22:33

score 3 · Answer 2 · answered Sep 26 '15 at 11:42

3

Scott Meyers says don't use swap(): http://scottmeyers.blogspot.sg/2014/06/the-drawbacks-of-implementing-move.html

Regarding your current implementation, it seems you can do it more simply. I imagine that the destructor actually deletes _buffer and does little else. If that's true, you should just replace your harder-to-reason-about explicit destructor call with delete _buffer.

answered Sep 26 '15 at 11:42

John Zwinck

239,568
38
324
436

@nick swap implementaion is strictly better than the above, however. – Yakk - Adam Nevraumont Sep 26 '15 at 12:17
2

@Nick Scott Meyers' point is pretty mute: the time is not lost in the triple pointer assignment, it is lost in the `delete` call. An assignment takes something like five cycles if in cache, a `new`/`delete` pair is in the range of 250 cycles. Also, there is not even a performance difference between `temp = a; a = b; b = temp; delete b;` and `delete a; a = b; b = nullptr;`: both versions load both `a` and `b` into a register, and then store a value to both `a` and `b`. Scott Meyers was clearly blinded by the abstraction of `a = b` when he wrote this article. – cmaster - reinstate monica Sep 26 '15 at 12:42

can I call destructor in move assignment operator?

2 Answers2