6

This is a bad pattern. Copy-and-swap is better.

foo & operator = ( foo const & other ) {
    static_assert ( noexcept( new (this) foo() ), "Exception safety violation" );

    this-> ~ foo();
    try {
        new (this) foo( other );
    } catch (...) {
        new (this) foo(); // does not throw
        throw;
    }
    return * this;
}

As long as foo is not polymorphic, what could go wrong? (However, assume that it is a base class.)

Background: I'm dealing with local-storage type erasure, and the alternative is to implement swap as two hard-coded assignments through a local storage space. The objects in the memory blobs of the source and destination are of different types and simply can't swap with each other. Copy/move construction defined in terms of such a swap is twice as complicated for seemingly no gain.

Community
  • 1
  • 1
Potatoswatter
  • 134,909
  • 25
  • 265
  • 421
  • `new (this) foo(); // does not throw` - how do you make sure it doesn't throw? Is that another precondition you're requiring of the type, which copy-and-swap doesn't? – Mike Seymour May 26 '15 at 14:57
  • @MikeSeymour Default constructors often do not throw. As for this question, take it as a statement of fact. `static_assert ( noexcept( new (this) foo() ), "Exception safety violation" );` if you prefer. – Potatoswatter May 26 '15 at 15:00
  • Care Oriented Programming? – Seçkin Savaşçı May 26 '15 at 15:03
  • [What if you're not careful...](http://stackoverflow.com/q/8829548) – Kerrek SB May 26 '15 at 15:03
  • @KerrekSB That's outside the scope, and the answer there begs the question here. Consider this question a follow-up to yours. – Potatoswatter May 26 '15 at 15:04
  • 1
    In some sense, assignment is an optimization of destroy and reconstruct. Consider `std::vector`: assignment reuses storage. – Kerrek SB May 26 '15 at 15:04
  • 3
    [basic.life]p7 does not mention polymorphism but simply requires that we're dealing with most derived types. So you'll probably end up (formally) with UB. – dyp May 26 '15 at 15:11
  • @dyp So it does. Add an answer for madd pointz. In the meantime, I will simply move the relevant destructor and constructor code into member functions. Busy busy. – Potatoswatter May 26 '15 at 15:39

2 Answers2

2

The standard makes guarantees about such interrupted lifetimes in [basic.life] §3.8/7:

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).

The last point disqualifies my use-case. However, since this is only for one non-polymorphic class, it's just as well to turn the destructor and constructors into destroy and init private member functions, respectively.

In other words, when destroy-and-regenerate is legal, you might as well do the same thing using member functions and no new/delete.

An "advantage" to this is that it ceases to look clever, so no ignorant passer-by would want to copy the design.

Community
  • 1
  • 1
Potatoswatter
  • 134,909
  • 25
  • 265
  • 421
0

Destroy-and-regenerate has fundamentally different behavior from copy-and-swap. Compare:

  1. Destroy the old value. It's gone forever now.
  2. Try to construct a new value.
  3. Give up and construct a default value if necessary.

Copy-and-swap:

  1. Try to construct a new value.
  2. Give up and leave the old value if necessary.
  3. Apply the new value.

Both have their merits, but copy-and-swap is ubiquitous so its disadvantages are mooted by the principle of least surprise. So let's emulate its behavior:

foo & operator = ( foo const & other ) {
    static_assert ( std::is_nothrow_move_constructible< foo >::value
                 || std::is_nothrow_default_constructible< foo >::value
                 , "Exception safety violation" );

    foo next( other );
    try {
        this-> ~ foo();
        new (this) foo( std::move( next ) );
    } catch (...) {
        new (this) foo();
        throw;
    }
    return * this;
}

Although more complicated, this is better-behaved than a throwing swap, which could leave a hodgepodge of the old and new values after an exception.

In the common case where the move constructor doesn't throw (you remembered to declare it noexcept, right?), the algorithm reduces nicely:

foo & operator = ( foo const & other ) {
    foo next( other );
    // The dangerous part is over now.

    this-> ~ foo();
    new (this) foo( std::move( next ) );
    return * this;
}
Potatoswatter
  • 134,909
  • 25
  • 265
  • 421