29

In my quest to learn C++ I stumbled across the article Writing Copy Constructors and Assignment Operators which proposes a mechanism to avoid code duplication across copy constructors and assignment operators.

To summarise/duplicate the content of that link, the proposed mechanism is:

struct UtilityClass
{
  ...

  UtilityClass(UtilityClass const &rhs)
    : data_(new int(*rhs_.data_))
  {
    // nothing left to do here
  }

  UtilityClass &operator=(UtilityClass const &rhs)
  {
    //
    // Leaves all the work to the copy constructor.
    //

    if(this != &rhs)
    {
      // deconstruct myself    
      this->UtilityClass::~UtilityClass();

      // reconstruct myself by copying from the right hand side.
      new(this) UtilityClass(rhs);
    }

    return *this;
  }

  ...
};

This seems like a nice way to avoid code duplication whilst ensuring "programatic integrity" but needs weighed against the risk of wasting effort freeing-then-allocating nested memory that could, instead, be re-used (as its author points out).

But I'm not familiar with the syntax that lies at its core:

this->UtilityClass::~UtilityClass()

I assume that this is a way to call the object's destructor (to destroy the contents of the object structure) whilst keeping the structure itself. To a C++ newbie, the syntax looks like a strange mixture of an object method and a class method.

Could someone please explain this syntax to me, or point me to a resource which explains it?

How does that call differ from the following?

this->~UtilityClass()

Is this a legitimate call? Does this additionally destroy the object structure (free from heap; pop off the stack)?

Shafik Yaghmour
  • 154,301
  • 39
  • 440
  • 740
duncan
  • 2,323
  • 3
  • 17
  • 21
  • If the destructor deletes the `data` OK. If not it creates a memory leak. – 101010 Aug 18 '14 at 13:59
  • 13
    This is a minefield of exception-unsafety (which is why Sutter quite correctly disparaged it). You might consider the [copy-and-swap idiom](http://stackoverflow.com/questions/3279543) to both avoid duplication and give a strong exception guarantee. – Mike Seymour Aug 18 '14 at 13:59
  • 1
    While I strongly advice you to not use this pattern, there shouldnt be any difference between both calls, you can see the second version `this->~UitilityClass()` as a shorthand. – Sebastian Hoffmann Aug 18 '14 at 14:01
  • @Paranaix The point is that the author of the link argues that they're different. –  Aug 18 '14 at 14:04
  • i dont think u r supposed to call destructor explicitly anywhere in the code.. –  Aug 18 '14 at 14:04
  • related: http://stackoverflow.com/questions/7257563/what-are-qualified-id-name-and-unqualified-id-name – eerorika Aug 18 '14 at 14:04
  • 3
    @STNYU Not true, think of placement new – Sebastian Hoffmann Aug 18 '14 at 14:05
  • 1
    Closely related : http://stackoverflow.com/questions/24295458/resolving-explicitly-the-scope-of-a-class-member – 101010 Aug 18 '14 at 14:06
  • @Paranaix ok, found this link http://stackoverflow.com/questions/16720201/calling-destructor-explicitly –  Aug 18 '14 at 14:07
  • @MikeSeymour, On the other hand, Howard Hinnant [suggests](http://accu.org/content/conf2014/Howard_Hinnant_Accu_2014.pdf) a `strong_assign` function for a strong exception guarantee. I'm interested in hearing more discussion on that. – chris Aug 18 '14 at 14:12
  • When opening the question, I though about `delete this;`. – Vi. Aug 18 '14 at 20:50
  • This is related too: http://stackoverflow.com/a/9117429/103167 – Ben Voigt Sep 04 '14 at 22:03

3 Answers3

27

TL;DR version: DO NOT FOLLOW ANY ADVICE GIVEN BY THE AUTHOR OF THAT LINK


The link suggests that this technique can be used in a base class as long as a virtual destructor call is not used, because doing so would destruct parts of the derived class, which isn't the responsibility of the base class operator=.

This line of reasoning totally fails. The technique can never ever be used in a base class. The reason is that the C++ Standard only allows in-place replacement of an object with another object of the exact same type (see section 3.8 of the Standard):

If, after the lifetime of an object has ended and before the storage which the object occupied is reused or released, a new object is created at the storage location which the original object occupied, a pointer that pointed to the original object, a reference that referred to the original object, or the name of the original object will automatically refer to the new object and, once the lifetime of the new object has started, can be used to manipulate the new object, if:

  • the storage for the new object exactly overlays the storage location which the original object occupied, and
  • the new object is of the same type as the original object (ignoring the top-level cv-qualifiers), and
  • the type of the original object is not const-qualified, and, if a class type, does not contain any non-static data member whose type is const-qualified or a reference type, and
  • the original object was a most derived object (1.8) of type T and the new object is a most derived object of type T (that is, they are not base class subobjects).

In the original code, both return *this; and subsequent use of the object are undefined behavior; they access an object which has been destroyed, not the newly created object.

This is a problem in practice as well: the placement-new call will set up a v-table ptr corresponding to the base class, not the correct derived type of the object.

Even for leaf classes (non-base classes) the technique is highly questionable.

Community
  • 1
  • 1
Ben Voigt
  • 277,958
  • 43
  • 419
  • 720
  • I don't understand why this placement-new doesn't fit the requirements for `*this` to refer to the newly created object. When the standard says "most derived object" does it mean `UtilityClass` *has* to be the final inheriting class of some base class, or does it mean it has to be a class which is not derived from? If it is the latter it looks as if the placement-new fits the last bullet point: the original object is of type `UtilityClass` and it is being replaced with an object of type `UtilityClass`. – David G Aug 18 '14 at 15:11
  • @0x499602D2: If you read the link, it's clearly stated that the original object is derived from `UtilityClass`. – Ben Voigt Aug 18 '14 at 15:15
  • As an aside, I would say that your TL;DR version also extends to almost all other C++ advice from that page. Some of it is written badly, some of it just strange/uncommon, some of it plain wrong/dangerous... – anderas Aug 19 '14 at 08:01
  • 1
    @anderas: My TL;DR wording is intended to apply to all advice from that source. Here we have a case of someone who is recommending poor practices and cannot be excused by ignorance; the author is fully aware that what they recommend is specifically called out by world experts as a pattern to be avoided, but rather than seeking to understand the rationale, constructs a faulty argument in favor of their opinion and doesn't even test it. And that's why my recommendation to disregard that author's advice is so broad. – Ben Voigt Aug 19 '14 at 13:55
  • @BenVoigt 100% agree. Sorry for being not clear, I meant to write "... advice from that AUTHOR", since (to me) your text looked like it was only about the linked page, instead of the full site. – anderas Aug 19 '14 at 14:29
21

TL;DR DO NOT DO THIS.

To answer the specific question:

In this particular example, there's no difference. As explained in the article you link to, there would be a difference if this were a polymorphic base class, with a virtual destructor.

A qualified call:

this->UtilityClass::~UtilityClass()

would specifically call the destructor of this class, not that of the most derived class. So it only destroys the subobject being assigned to, not the entire object.

An unqualified call:

this->~UtilityClass()

would use virtual dispatch to call the most derived destructor, destroying the complete object.

The article writer claims that the first is what you want, so that you only assign to the base sub-object, leaving the derived parts intact. However, what you actually do overwrite part of the object with a new object of the base type; you've changed the dynamic type, and leaked whatever was in the derived parts of the old object. This is a bad thing to do in any circumstances. You've also introduced an exception issue: if the construction of the new object fails, then the old object is left in an invalid state, and can't even be safely destroyed.

UPDATE: you also have undefined behaviour since, as described in another answer, it's forbidden to use placement-new to create an object on top of (part of) a differently-typed object.

For non-polymorphic types, a good way to write a copy-assignment operator is with the copy-and-swap idiom. That both avoids duplication by reusing the copy-constructor, and provides a strong exception guarantee - if the assignment fails, then the original object is unmodified.

For polymorphic types, copying objects is more involved, and can't generally be done with a simple assignment operator. A common approach is a virtual clone function, which each type overrides to dynamically allocate a copy of itself with the correct type.

Community
  • 1
  • 1
Mike Seymour
  • 249,747
  • 28
  • 448
  • 644
  • It's actually strictly forbidden to use placement-new to replace a base subobject; see my answer. – Ben Voigt Aug 18 '14 at 14:24
  • @BenVoigt: Thanks, I didn't know whether it was forbidden or merely wrong. – Mike Seymour Aug 18 '14 at 14:25
  • What's the difference between using clone functions and using `shared_from_this` (if they are even related...)? – David G Aug 18 '14 at 14:31
  • @0x499602D2: Clone functions create a new object; shared pointers share ownership of the existing object. – Mike Seymour Aug 18 '14 at 14:31
  • @0x499602D2: Assuming `Animal -> Cat -> Tiger` -- _clone_ would return a new copy of the underlying animal even when you've `Animal*`; `shared_from_this` will return a `shared_ptr` to an already existing animal object irrespective of the pointer you've, be it `Animal*`, `Cat*` or `Tiger*`. – legends2k Aug 18 '14 at 16:32
  • This is the right answer because it recognises a degenerated copy&swap idiom and gives a hint at how to fix it. – Michaël Le Barbier Aug 19 '14 at 08:27
  • Although `this->~UtilityClass` to obtain a virtual dispatch seems superior, in fact it is ill-formed. http://llvm.org/bugs/show_bug.cgi?id=20729 (You can use a `static_cast` though.) – Potatoswatter Aug 22 '14 at 02:54
9

You can decide to how to call the destructor:

this->MyClass::~MyClass(); // Non-virtual call

this->~MyClass();          // Virtual call
masoud
  • 55,379
  • 16
  • 141
  • 208