Is it a good practice to call the destructor explicitly and use placement new(this) in assignment op

Discussion in 'C++' started by Michael Tsang, Dec 12, 2009.

  1. -----BEGIN PGP SIGNED MESSAGE-----
    Hash: SHA1

    Consider the following codes:

    class Foo {
    public:
    // default constructor
    Foo();
    // l-value copy constructor
    Foo(const Foo &);
    // destructor
    virtual ~Foo();
    // assignment operator
    Foo &operator=(const Foo &x) {
    if(this != &x) {
    this->~Foo();
    new(this) Foo(x);
    }
    return *this;
    }
    private:
    // private data omitted
    };

    Is the above Foo::eek:perator=(const Foo &) written in good practice?
    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.9 (GNU/Linux)

    iEYEARECAAYFAksjq5gACgkQG6NzcAXitM/sBwCeNar9lgasbrpSQ3U7Yln9Yu8U
    uNUAn07Q/zHscyETayBwXYSt275noT0q
    =zi3m
    -----END PGP SIGNATURE-----
    Michael Tsang, Dec 12, 2009
    #1
    1. Advertising

  2. Michael Tsang

    Jonathan Lee Guest

    Re: Is it a good practice to call the destructor explicitly and useplacement new(this) in assignment operators?

    On Dec 12, 9:41 am, Michael Tsang <> wrote:
    > Is the above Foo::eek:perator=(const Foo &) written in good practice?


    No, I really don't think so. Imagine if your copy constructor
    throws. If the user recovers from the throw, your instance of
    Foo will be invalid. I believe there was another recent thread
    that pretty much decided calling the destructor led to undefined
    behaviour anyway.

    I believe the most common practice is to create a temp using
    the copy constructor and then call a nothrow swap, i.e.,

    Foo& operator=(const Foo& x) {
    if (this != &x) Foo(x).swap(*this);
    return *this;
    }

    That way, if the temp construction throws you don't damage
    the value being assigned to, and the user has a hope of
    recovering.

    --Jonathan
    Jonathan Lee, Dec 12, 2009
    #2
    1. Advertising

  3. Pete Becker wrote:

    > No. Think about what happens when someone derives from this class and
    > writes a normal assignment operator.
    >

    Sorry I forgot to make my assignment operator virtual. Now consider the
    following code:

    class Foo {
    public:
    // default constructor
    Foo();
    // l-value copy constructor
    Foo(const Foo &);
    // destructor
    virtual ~Foo();
    // assignment operator
    virtual Foo &operator=(const Foo &x) {
    if(this != &x) {
    this->~Foo();
    new(this) Foo(x);
    }
    return *this;
    }
    private:
    // private data omitted
    };

    class Bar : public Foo {
    public:
    // default constructor
    Bar() : Foo(), a(), b() {
    }
    // l-value copy constructor
    Bar(const Bar &x) : Foo(x), a(x.a), b(x.b) {
    }
    // destructor
    ~Bar() {
    }
    // assignment operator
    Bar &operator=(const Bar &x) {
    // assign base class subobject
    Foo::eek:perator=(x);
    // assign member subobjects
    a = x.a;
    b = x.b;
    return *this;
    }
    // other methods omitted
    private:
    int a;
    int b;
    };

    I don't know what strange things will happen when Bar::eek:perator=(const Bar
    &) is called.
    Michael Tsang, Dec 13, 2009
    #3
  4. Michael Tsang

    James Kanze Guest

    Re: Is it a good practice to call the destructor explicitly and useplacement new(this) in assignment operators?

    On 14 Dec, 04:56, Michael Tsang <> wrote:

    > Update: Is this UB?


    > class Foo {
    > public:
    > // default constructor
    > Foo();
    > // l-value copy constructor
    > Foo(const Foo &);
    > // destructor
    > virtual ~Foo();
    > // assignment operator
    > virtual Foo &operator=(const Foo &x) {
    > if(this != &x) {
    > // disable virtual mechanism
    > this->Foo::~Foo();
    > new(this) Foo(x);
    > }
    > return *this;
    > }
    > private:
    > // private data omitted
    > };


    > class Bar : public Foo {
    > public:
    > // default constructor
    > Bar() : Foo(), a(), b() {
    > }
    > // l-value copy constructor
    > Bar(const Bar &x) : Foo(x), a(x.a), b(x.b) {
    > }
    > // destructor
    > ~Bar() {
    > }
    > // assignment operator
    > Bar &operator=(const Bar &x) {
    > // assign base class subobject
    > Foo::eek:perator=(x);
    > // assign member subobjects
    > a = x.a;
    > b = x.b;
    > return *this;
    > }
    > // other methods omitted
    > private:
    > int a;
    > int b;
    > };


    Yes. In Bar::eek:perator=, after calling Foo::eek:perator=, there is
    no more Bar object, and any use of the this pointer (including
    simply returning it, I think) is undefined behavior.

    Try it replacing the int's in Barr with tracer objects, objects
    of a class type which logs all construction, destruction and
    assignment, and you'll find yourself assigning to a destructed
    object.

    --
    James Kanze
    James Kanze, Dec 14, 2009
    #4
  5. -----BEGIN PGP SIGNED MESSAGE-----
    Hash: SHA1

    Pete Becker wrote:


    > Think in terms of implementation here: the constructor for a type sets
    > the vtable pointer to point to the vtable for that type. When you've
    > stomped on the vtable pointer with new(this)Foo(x) you do not have a Bar
    > object. You have a mongrel, with Bar's data and Foo's vtable pointer.
    >
    > But technically, no, there's no undefined behavior here. There's no
    > behavior at all, since there's no code being executed. You'd have to
    > create and object, call operator=, and then do something with that object.
    >


    class Foo {
    public:
    // default constructor
    Foo();
    // l-value copy constructor
    Foo(const Foo &);
    // destructor
    virtual ~Foo();
    // assignment operator
    virtual Foo &operator=(const Foo &x) {
    if(this != &x) {
    // disable virtual mechanism
    this->Foo::~Foo();
    new(this) Foo(x);
    }
    return *this;
    }
    private:
    // private data omitted
    };

    class Bar : public Foo {
    public:
    // default constructor
    Bar() : Foo(), a(), b() {
    }
    // l-value copy constructor
    Bar(const Bar &x) : Foo(x), a(x.a), b(x.b) {
    }
    // destructor
    ~Bar() {
    }
    // assignment operator
    Bar &operator=(const Bar &x) {
    // assign base class subobject
    Foo::eek:perator=(x);
    // assign member subobjects
    a = x.a;
    b = x.b;
    return *this;
    }
    // other methods omitted
    private:
    int a;
    int b;
    };

    int main() {
    Bar b, c;
    c = b; // UB?
    }

    So, is there any better method to write Foo::eek:perator=(const Foo &) without
    causing problem on Bar::eek:perator=(const Bar &) if it is not changed?
    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.9 (GNU/Linux)

    iEYEARECAAYFAksmR48ACgkQG6NzcAXitM+8iwCgh32S4LQu2Kz0TVp3BIDfhM99
    fG4An1AXOmMSpajSTT0/9s9EFlHEQFPp
    =8mDm
    -----END PGP SIGNATURE-----
    Michael Tsang, Dec 14, 2009
    #5
  6. Re: Is it a good practice to call the destructor explicitly and useplacement new(this) in assignment operators?

    Michael Tsang wrote:
    > -----BEGIN PGP SIGNED MESSAGE-----
    > Hash: SHA1
    >
    > Pete Becker wrote:
    >
    >
    >> Think in terms of implementation here: the constructor for a type sets
    >> the vtable pointer to point to the vtable for that type. When you've
    >> stomped on the vtable pointer with new(this)Foo(x) you do not have a Bar
    >> object. You have a mongrel, with Bar's data and Foo's vtable pointer.
    >>
    >> But technically, no, there's no undefined behavior here. There's no
    >> behavior at all, since there's no code being executed. You'd have to
    >> create and object, call operator=, and then do something with that object.
    >>

    >
    > class Foo {
    > public:
    > // default constructor
    > Foo();
    > // l-value copy constructor
    > Foo(const Foo &);
    > // destructor
    > virtual ~Foo();
    > // assignment operator
    > virtual Foo &operator=(const Foo &x) {
    > if(this != &x) {
    > // disable virtual mechanism
    > this->Foo::~Foo();
    > new(this) Foo(x);
    > }
    > return *this;
    > }
    > private:
    > // private data omitted
    > };
    >
    > class Bar : public Foo {
    > public:
    > // default constructor
    > Bar() : Foo(), a(), b() {
    > }
    > // l-value copy constructor
    > Bar(const Bar &x) : Foo(x), a(x.a), b(x.b) {
    > }
    > // destructor
    > ~Bar() {
    > }
    > // assignment operator
    > Bar &operator=(const Bar &x) {
    > // assign base class subobject
    > Foo::eek:perator=(x);
    > // assign member subobjects
    > a = x.a;
    > b = x.b;
    > return *this;
    > }
    > // other methods omitted
    > private:
    > int a;
    > int b;
    > };
    >
    > int main() {
    > Bar b, c;
    > c = b; // UB?
    > }
    >
    > So, is there any better method to write Foo::eek:perator=(const Foo &) without
    > causing problem on Bar::eek:perator=(const Bar &) if it is not changed?
    > -----BEGIN PGP SIGNATURE-----
    > Version: GnuPG v1.4.9 (GNU/Linux)
    >
    > iEYEARECAAYFAksmR48ACgkQG6NzcAXitM+8iwCgh32S4LQu2Kz0TVp3BIDfhM99
    > fG4An1AXOmMSpajSTT0/9s9EFlHEQFPp
    > =8mDm
    > -----END PGP SIGNATURE-----
    >


    1) Your PGP signature is very annoying

    2) What kind of compiler you used to compile your example? You didn't
    include <new>, missing definitions of Foo's constructor and destructor.

    3) As an exercise, add this pure virtual method to Foo:
    virtual void ops() = 0;
    and implement it in Bar, like this:
    virtual void ops(){}
    Then call this method as the last step in the main(). When you compile
    and run, you will get a nice surprise :)



    --
    ultrasound www.ezono.com
    Vladimir Jovic, Dec 14, 2009
    #6
  7. Michael Tsang

    James Kanze Guest

    Re: Is it a good practice to call the destructor explicitly and useplacement new(this) in assignment operators?

    On Dec 14, 6:17 pm, Paavo Helde <> wrote:
    > Pete Becker <> wrote
    > innews::
    > > Paavo Helde wrote:
    > >> Pete Becker <> wrote in news:-
    > >> :


    > >>> Paavo Helde wrote:
    > >>>> Pete Becker <> wrote in
    > >>>>news::


    > >>>>> Paavo Helde wrote:
    > >>>>>> Michael Tsang <> wrote in
    > >>>>>>news:hg2tuh$bmb$-september.org:


    > >>>>>>> Pete Becker wrote:


    > >>>>>>>> No. Think about what happens when someone derives
    > >>>>>>>> from this class and writes a normal assignment
    > >>>>>>>> operator.


    > >>>>>>> Sorry I forgot to make my assignment operator virtual.
    > >>>>>>> Now consider the following code:


    > >>>>>>> class Foo {
    > >>>>>>> public:
    > >>>>>>> // default constructor
    > >>>>>>> Foo();
    > >>>>>>> // l-value copy constructor
    > >>>>>>> Foo(const Foo &);
    > >>>>>>> // destructor
    > >>>>>>> virtual ~Foo();
    > >>>>>>> // assignment operator
    > >>>>>>> virtual Foo &operator=(const Foo &x) {
    > >>>>>>> if(this != &x) {
    > >>>>>>> this->~Foo();
    > >>>>>>> new(this) Foo(x);
    > >>>>>> This slices any derived class object to Foo.


    > >>>>> It doesn't even do that. Slicing is well defined:


    > >>>>> Foo f = Bar(); // slices the Bar object; f is a valid Foo
    > >>>>> object


    > >>>>> That placement new constructs a Foo object where a Bar
    > >>>>> object used to exist, resulting in undefined behavior.
    > >>>> I cannot quite see how this is UB by itself. Consider:


    > >>>> void* p = malloc(sizeof(Bar));
    > >>>> Bar* b = new (p) Bar();
    > >>>> b->~Bar();
    > >>>> Foo* f = new (p) Foo();


    > >>>> Here also a Foo object is constructed where Bar was. Is this UB
    > >>>> too?


    > >>> That's not inside operator=.


    It also doesn't make any further use of b.

    > >> I presume this means no UB in the last example.


    > >> Then, how the operator= is any different? Clearly, just
    > >> being a member function should not affect anything. For
    > >> example, several people (incl. James Kanze) advocate using
    > >> "delete this;" from inside a member function. This clearly
    > >> invalidates the object, but as long as it is not accessed
    > >> any more, there should be no UB.


    As long as there is no further use of the this pointer, *nor* of
    any other pointer to the object, there is no problem. (Note
    that "delete this" is only appropriate for certain types of
    objects, which explicitly manage their own lifetime, and will
    take the appropriate steps in their destructor to ensure that no
    other pointers to them remain. Of course, in a lot of
    applications, no other types should be dynamically allocated to
    begin with.)

    > > operator= isn't usually called on objects that are going to
    > > be thrown away immediately.


    The problem, of course, isn't that you're still going to use the
    object. The problem is that you've changed its type.

    > In no way do I advocate actually using this, but here is an
    > example of using the object after assignment. UB or not UB?


    > #include <iostream>


    > class Foo {
    > public:
    > Foo(int x): x_(x) {}
    > void operator=(const Foo& b) {
    > void* p = static_cast<void*>(this);
    > this->~Foo();
    > new (p) Foo(b.x_);
    > }
    > virtual ~Foo() {}
    > virtual void f() {
    > std::cout << "Foo(" << x_ << ")\n";
    > }
    > protected:
    > int x_;
    > };


    > class Bar: public Foo {
    > public:
    > Bar(int x, int y): Foo(x), y_(y) {}
    > virtual void f() {
    > std::cout << "Bar(" << x_ << ", " << y_ << ")\n";
    > }
    > private:
    > int y_;
    > };


    > int main() {
    >
    > Foo* p = new Bar(42,43);


    > std::cout << "Before assignment: ";
    > p->f();


    > *p = Foo(31);


    > std::cout << "After assignment: ";
    > p->f();


    > delete p;
    > }


    Undefined behavior. The standard does allow constructing a new
    object where an old one existed. But pointers to the old one
    are only valid if the storage exactly overlays the old one (the
    case here, maybe), the new object has the same type (modulo
    cv-qualifiers) as the old one, and neither are base class
    subobjects. If you want to see why, consider modifying Bar as
    follows:

    struct BarBase { int i; /* take some space */ };
    class Bar : private BarBase, public Foo
    {
    // as before...
    };

    Now what do you get?

    --
    James Kanze
    James Kanze, Dec 14, 2009
    #7
  8. Michael Tsang

    James Kanze Guest

    Re: Is it a good practice to call the destructor explicitly and useplacement new(this) in assignment operators?

    On Dec 14, 3:03 pm, "Alf P. Steinbach" <> wrote:
    > * Michael Tsang:
    > > Pete Becker wrote:


    > >> Think in terms of implementation here: the constructor for
    > >> a type sets the vtable pointer to point to the vtable for
    > >> that type. When you've stomped on the vtable pointer with
    > >> new(this)Foo(x) you do not have a Bar object. You have a
    > >> mongrel, with Bar's data and Foo's vtable pointer.


    > >> But technically, no, there's no undefined behavior here.
    > >> There's no behavior at all, since there's no code being
    > >> executed. You'd have to create and object, call operator=,
    > >> and then do something with that object.


    > > class Foo {
    > > public:
    > > // default constructor
    > > Foo();
    > > // l-value copy constructor
    > > Foo(const Foo &);
    > > // destructor
    > > virtual ~Foo();
    > > // assignment operator
    > > virtual Foo &operator=(const Foo &x) {
    > > if(this != &x) {
    > > // disable virtual mechanism
    > > this->Foo::~Foo();
    > > new(this) Foo(x);


    If the actual object was of a type derived from Foo, the above
    has invalidated all of the pointers to it. I'm also not too
    sure, but I think destructing the base class other than from the
    destructor of the derived class is undefined behavior---it
    should be, anyway.

    > > }
    > > return *this;
    > > }


    > Assignment operator should as a rule never be virtual, because
    > that moves error detection from compile time to run time, and
    > also, it's easy to forget to override it, as it seems you have
    > forgotten in the class below?


    > In other words, it creates a mess. :)


    Not to forget that there's no way to implement the actual
    semantics of assignment in a derived class. Consider:

    struct Base { virtual ~Base() {}; }
    class D1 : public Base { /*...*/ };
    class D2 : public Base { /*...*/ };

    Base p = new D1;
    *p = D2() ;

    What should the last line mean? How do you implement it.

    If you need assignment in a polymorphic class, you can fake it
    using something like the letter/envelop idiom, but the cost is
    usually fairly high, and it's rarely appropriate.

    > The other problem is, as Pete noted above, that you're
    > stomping all over the vtable. It's just completely
    > unnecessary. What is the purpose that you /think/ you're
    > achieving (please forgive me for not reading up the whole
    > thread)?


    > > private:
    > > // private data omitted
    > > };

    >
    > > class Bar : public Foo {
    > > public:
    > > // default constructor
    > > Bar() : Foo(), a(), b() {
    > > }
    > > // l-value copy constructor
    > > Bar(const Bar &x) : Foo(x), a(x.a), b(x.b) {
    > > }
    > > // destructor
    > > ~Bar() {
    > > }
    > > // assignment operator
    > > Bar &operator=(const Bar &x) {
    > > // assign base class subobject
    > > Foo::eek:perator=(x);
    > > // assign member subobjects


    Of course, given his implementation of Foo::eek:perator=, there is
    no more Bar object here, and any use of the this pointer after
    this line is undefined behavior.

    > > a = x.a;
    > > b = x.b;
    > > return *this;
    > > }
    > > // other methods omitted
    > > private:
    > > int a;
    > > int b;
    > > };


    > > int main() {
    > > Bar b, c;
    > > c = b; // UB?
    > > }


    > > So, is there any better method to write Foo::eek:perator=(const
    > > Foo &) without causing problem on Bar::eek:perator=(const Bar
    > > &) if it is not changed?


    > Huh?


    > But as a general rule, the compiler generated assignment
    > operator is generally sufficient, and where it isn't, the fix
    > is most probably to equip member variables with proper copying
    > semantics instead of specializing the assignment operator and
    > other things to deal with their lack of proper copyability.


    Well, you've got to start somewhere. But generally, in my
    experience, a lot of classes shouldn't support assignment to
    begin with. Which avoids a lot of the questioning as well.

    > For example, a raw pointer member variable might seem to
    > require some custom assignment operator in the containing
    > class. Solution: use a smart pointer. Not a solution:
    > implementing a custom assignment operator.


    If the requirement is deep copy of some more or less complex
    structure (e.g. like std::map), then you have to implement it.
    The compiler will only do a shallow copy, and smart pointers
    will only go so far.

    > For those few remaining cases where it's most practical to do
    > the custom assignment operator, you'll often find that that
    > the swap idiom for assignment serves your needs. This is a way
    > to express assignment in terms of copy construction, and it's
    > generally exception and self-copy safe. It goes like


    > T& operator=( T const& other )
    > {
    > T const copy( other ); // Using copy constructor to copy.
    > this->swap( copy ); // Can't throw, by design.
    > }


    > I wrote "this->" just to make it clear since it's a snippet
    > out of context; writing "this->" is not something that one
    > should do in real code...


    More generally, you fully construct all necessary data, and do
    anything else which might throw, before modifying the original
    object. (The swap idiom is a convenient way of accomplishing
    this in many cases, but for the very simplest cases, like a
    pimpl using deep copy, it may be overkill, and be just as simple
    to implement using the idiom which was standard before the swap
    idiom was invented.)

    --
    James Kanze
    James Kanze, Dec 14, 2009
    #8
  9. Re: Is it a good practice to call the destructor explicitly and useplacement new(this) in assignment operators?

    * James Kanze:
    > On Dec 14, 3:03 pm, "Alf P. Steinbach" <> wrote:
    >
    >> T& operator=( T const& other )
    >> {
    >> T const copy( other ); // Using copy constructor to copy.
    >> this->swap( copy ); // Can't throw, by design.
    >> }

    >
    >> I wrote "this->" just to make it clear since it's a snippet
    >> out of context; writing "this->" is not something that one
    >> should do in real code...

    >
    > More generally, you fully construct all necessary data, and do
    > anything else which might throw, before modifying the original
    > object. (The swap idiom is a convenient way of accomplishing
    > this in many cases, but for the very simplest cases, like a
    > pimpl using deep copy, it may be overkill, and be just as simple
    > to implement using the idiom which was standard before the swap
    > idiom was invented.)


    Hello ... Hello ... JAMES!

    You forgot, there's a 'const' in there that shouldn't be there!

    But OK, you probably thought it was just a typo and it was...


    Cheers,

    - Alf
    Alf P. Steinbach, Dec 14, 2009
    #9
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Timothy Madden
    Replies:
    3
    Views:
    3,857
    Timothy Madden
    Nov 19, 2004
  2. vlsidesign
    Replies:
    26
    Views:
    947
    Keith Thompson
    Jan 2, 2007
  3. dragoncoder

    Placement new and destructor

    dragoncoder, May 10, 2006, in forum: C++
    Replies:
    1
    Views:
    344
  4. Replies:
    7
    Views:
    525
    Frederick Gotham
    Sep 8, 2006
  5. SM
    Replies:
    9
    Views:
    487
Loading...

Share This Page