2

I haven't programmed much since before the C++11 standard, so I'm still learning some of the newer idioms and how to use them.

I've been thinking how to write efficient assignment operators, and I just found out how placement new works.

So now I'm considering writing assignment operators by (1) destroying the object, and (2) using placement new to call the copy constructor, like:

MyClass& MyClass::operator=(const MyClass& other)
{
    if (&other == this)
        return (*this);

    this -> ~MyClass();
    return *(new (this) MyClass(other));
}

Normally I would never use an object after destructing it. BUT, I am immediately reconstructing it. Is this idiom safe and elegant to use? Or is it a TERRIBLE idea and I should immediately

delete this;
CPEG
  • 31
  • 2
  • 6
    This is a bad idea. Not only does it not compile (you are `return`'ing a `MyClass*` where a `MyClass&` is expected), but it is not exception-safe, either. The typical way to implement a copy assignment operator is to utilize the copy constructor via the [copy-swap](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) idiom instead. – Remy Lebeau Jan 26 '21 at 19:57
  • What if the copy constructor (called by placement-new) throws? Besides, you may or may not get UB (pre-C++20 IIRC) if you have `const` and/or reference members (possibly indirect) in the class, if something accesses them without `std::launder` after reconstruction, which is almost guaranteed to happen. – HolyBlackCat Jan 26 '21 at 19:57
  • This isn't dissimilar from what `std::variant` will sometimes do, when assigning a different type.. – Drew Dormann Jan 26 '21 at 19:59
  • I fixed my example code (is now return *(new etc) instead of return new etc). So it's pretty much just a problem if the copy constructor excepts? Is that a thing that's likely to happen? Why would that happen? – CPEG Jan 26 '21 at 20:01
  • @DrewDormann I think what it does is less risky. `std::variant` would call a destructor on its member, not on itself, so it can guarantee that the member is always accessed through `laudner` and so on. – HolyBlackCat Jan 26 '21 at 20:01
  • 1
    General rule: Manually calling the destructor is a *super bad idea*. Likewise `delete this` is also an astoundingly dangerous thing to do. Let the destructor play out naturally when the object falls out of scope or the owner `delete`s it. Calling the destructor manually is the equivalent of so utterly trashing your hotel room to the point where if anyone enters it, they *immediately die*. – tadman Jan 26 '21 at 20:06
  • @RemyLebeau I have already been using copy-and-swap, but I was considering an alternative. I can see how copy-and-swap is exception-safe, since I hadn't considered that you might get an exception from the copy constructor. – CPEG Jan 26 '21 at 20:11
  • @tadman I had been thinking that, after utterly trashing my hotel room, I would bring in contractors to rebuild a new hotel room in the same place. But it's been pointed out that the contractors might not deliver. ... "delete this;" was a joke, and will forever and always be how I describe horrendously bad code. – CPEG Jan 26 '21 at 20:13
  • Well, imagine you trash the hotel room so badly it's now as radioactive as Chernobyl's exploded reactor so anyone who goes in there to clean up is not going to survive. That's what it's like when you introduce undefined behaviour into your program. You've made a mess that's so catastrophic there is no recovering from it. – tadman Jan 26 '21 at 20:15
  • @tadman "General rule: Manually calling the destructor is a super bad idea." In this particular case it's fine on itself. – SergeyA Jan 26 '21 at 20:17
  • @SergeyA What if this object falls out of scope and the destructor gets called again? – tadman Jan 26 '21 at 20:17
  • @tadman if you manage to solve the exception issue (see my answer) than this is not a problem. – SergeyA Jan 26 '21 at 20:22
  • @tadman If the copy constructor works as planned... this isn't a problem. When the destructor is called a second time, it will be called on a perfectly valid constructed object, just as planned. – CPEG Jan 26 '21 at 20:36
  • If inheritance is involved, could get exciting! – Eljay Jan 26 '21 at 23:59
  • @Eljay inheritance by itself would not be an issue. – SergeyA Jan 27 '21 at 01:01

1 Answers1

2

This approach is not new, and people have tried it before. The main problem, as stated in the comments (the rest is less serious) is a potential for exception thrown during the call to the copy constructor.

If that happens, you end up in a very bad situation - you did call destructor already, so that object is no longer valid, and there is no undoing that. Even if you catch the exception, there is still no good recourse, as there is nothing you can do to restore the object.

There are also classes which need a certain pre-condition before destructor can be called (std::thread is a prime example). While I personally do not like this, those classes exist.

If you are doing this for your own fully-controlled class, where you know for a fact copy-constructor doesn't throw (and ideally annotated as such), and random destructing of the class is acceptable, this approach is viable - although, usually not worth it.

SergeyA
  • 61,605
  • 5
  • 78
  • 137
  • Thank you. Are default copy constructors and assignment operators noexcept? (And, are they efficient enough?) So far, I'm not sure I even needed to write copy constructors and assignment, since the only dynamic memory I've used is vectors. – CPEG Jan 26 '21 at 20:39
  • @CPEG when in doubt, do not write special functions. See [C.20: If you can avoid defining default operations, do](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-zero) – SergeyA Jan 26 '21 at 20:46
  • @CPEG if you're not doing dynamic memory allocation yourself, follow [the rule of zero](https://cpppatterns.com/patterns/rule-of-zero.html). – JHBonarius Jan 26 '21 at 20:48
  • Let's ignore that I think that it is not valid. It would also be dangerous if someone inherits from `MyClass`. ([example on godbolt](https://godbolt.org/z/vbM3qT)) – t.niese Jan 26 '21 at 20:51
  • @t.niese that assignment is not actually an issue. It is the subobject of type `Base` which is being manipulated. You can write your own assignment logic in `Dervied` class as well if you want to handle derived object assignment. – SergeyA Jan 26 '21 at 21:00