9

In this question of mine, @DeadMG says that reinitializing a class through the this pointer is undefined behaviour. Is there any mentioning thereof in the standard somewhere?

Example:

#include <iostream>

class X{
  int _i;
public:  
  X() : _i(0) { std::cout << "X()\n"; }
  X(int i) : _i(i) { std::cout << "X(int)\n"; }

  ~X(){ std::cout << "~X()\n"; }

  void foo(){
    this->~X();
    new (this) X(5);
  }

  void print_i(){
    std::cout << _i << "\n";
  }
};

int main(){
  X x;
  x.foo();
  // mock random stack noise
  int noise[20];
  x.print_i();
}

Example output at Ideone (I know that UB can also be "seemingly correct behaviour").
Note that I did not call the destructor outside of the class, as to not access an object whose lifetime has ended. Also note, that @DeadMG says that directly calling the destructor is okay as-long-as it's called once for every constructor.

Community
  • 1
  • 1
Xeo
  • 129,499
  • 52
  • 291
  • 397
  • 6
    This structure (calling the destructor then a constructor with placement new) was a somewhat popular way to implement the assignment operator until it was found exception unsafe. I don't remember anybody said it was UB in absence of exception. There are probably cases with virtual functions and multiple inheritance that are UB. – AProgrammer Jun 03 '11 at 07:23
  • If it's UB it would only be because of the use of `this`. If this is the case you could still get around it by taking a copy of `this` before calling the destructor. – Luc Danton Jun 03 '11 at 07:31
  • +1 AProgrammer. As a matter of fact, the C++0x standard (FDIS) contains an example of manual destruction + placement construction, so it is probably not *that* bad. It is in §9.5/4 as the way to change the *active* member of an union when some of the members have non-trivial constructors/destructors: `u.m.~M(); new (&u.n) N;` now, the examples does not do it from *inside* a method, but I don't know whether this makes any difference. – David Rodríguez - dribeas Jun 03 '11 at 07:33
  • @curiousguy A pointer to the storage would remain valid. I was speculating as to whether there were rules on the use of `this` as a keyword, not on its particular value. That is to say, I was considering whether *before* I could access the value (which is obviously correct due to other requirements), would the use of `this` be allowed at all? – Luc Danton Sep 30 '11 at 01:17
  • @LucDanton `this` is just a keyword used to get the value of the implicit parameter of non-static member functions. Even in programs where `this->~T();`, `delete this;`, or `new (this) T;` are never used, `this` can refer to an object that is not yet fully constructed, in some case such as no sub-object construction has even began. – curiousguy Sep 30 '11 at 02:13
  • @curiousguy Hence why I was using the speculative. *If* there *were* a problem, it could only have come from a weird rule about `this` because *otherwise* the code is correct. I was not saying 'using `this` is incorrect'. – Luc Danton Sep 30 '11 at 02:44
  • @LucDanton I see. C++ is more regular and consistent than you imagined! ;) – curiousguy Sep 30 '11 at 02:52

2 Answers2

8

That would be okay if it didn't conflict with stack unwinding.

You destroy the object, then reconstruct it via the pointer. That's what you would do if you needed to construct and destroy an array of objects that don't have a default constructor.

The problem is this is exception unsafe. What if calling the constructor throws an exception and stack is unwound and the destructor is called for the second time?

{
   X x;
   x.foo(); // here ~X succeeds, then construction fails
} //then the destructor is invoked for the second time.

That aspect specifically would be undefined behavior.

sharptooth
  • 167,383
  • 100
  • 513
  • 979
  • Ahh, good point about the exception safety. Can you think of any way to make that exception safe? – Xeo Jun 03 '11 at 07:30
  • What would make the member function special for this? (Nobody pretend for instance that `delete this` is an UB.) – AProgrammer Jun 03 '11 at 07:32
  • 1
    @Xeo Wrap the constructor call in a `try ... catch` block and handle the exception in a way that doesn't invoke UB. `std::abort` comes to mind here. Or, since this is a member, invoke a private nothrow constructor that puts the object in an unusable but destroyable state and then (re)throw. The user won't be able to see the object in that state and stack unwinding will work. But please **don't** consider this last solution as a practical thing to do (most of the time). – Luc Danton Jun 03 '11 at 07:33
  • @Xeo: Well, you'd need to disable (or disable effect of) stack unwinding temporarily. I don't think it can be done in any convenient and not error-prone manner. – sharptooth Jun 03 '11 at 07:34
  • 3
    @Xeo: I don't think it can be made exception safe in the general case. The problem being that before you construct, you must have destructed, and if the constructor fails, the state is not the original state any more. In a class with a nothrow move constructor you can avoid the problem altogether: construct a local variable, once constructed move it to `*this`. In that case it will be exception safe (assuming that the destructor is also nothrow). – David Rodríguez - dribeas Jun 03 '11 at 07:37
  • @David: Nice thought on the moving, that would indeed make it exception safe (non-throwing move-ctor assumed). – Xeo Jun 03 '11 at 07:40
  • @Xeo: nothing really new... exception safety is easily obtained in C++03 by constructing aside and using a non-throwing `swap`... this is just another way of applying the same idiom with different syntax. In fact, I still believe that the `copy-and-swap` idiom is better, as you don't really care on whether the destructor throws or not, destruction is performed on the copy *after* the original has been *swapped*, so the operation has been fully applied at that point. I guess *move* semantics is our new [golden hammer](http://en.wikipedia.org/wiki/Law_of_the_instrument) – David Rodríguez - dribeas Jun 03 '11 at 08:12
  • 1
    Ah, and BTW, if you implement a *move constructor*, you better make it nothrow. Doing otherwise is a recipe for disaster. I.e. you would have problems with some STL containers: vector will *move* object from one location to another during the buffer growth, if one of the move operations throw, the vector will be left in an inconsistent state with some of the objects in the original vector invalidated and some correct... I have not looked at the details of `unordered_map`, but my guess is that a similar problem would arise. – David Rodríguez - dribeas Jun 03 '11 at 08:19
0

Apart from @sharptooth's answer. I am just wondering for 2 more cases. At least they are worth mentioning.

(1) If the foo() was invoked using a pointer on heap which was allocated using malloc(). The constructor is not called. Can it be safe ?

(2) What if the derived class is calling foo(). Will it be a good behavior ? e.g.

struct Y : X {};  // especially when there is virtual ~X()
int main ()
{
  Y y;
  y.foo();
}
iammilind
  • 68,093
  • 33
  • 169
  • 336
  • 2
    (1) assuming you mean a pointer to memory allocated using `malloc`, then it's UB to call a function member on it unless placement new was used to create an object. Then it's the responsability of the user to explicitly call the destructor before `free`ing the memory. – Luc Danton Jun 03 '11 at 08:10
  • 1
    (2) since the destructor of `X` is not virtual, `this->~X()` correctly destroys the `X` subobject of `Y`, which is then recreated. If the destructor were virtual, `foo` could use `this->X::X()` to do the same thing. – Luc Danton Jun 03 '11 at 08:12
  • 3
    (2) At the end of the function (`main`), `y.~Y()` will be called. After calling `y.foo()`, the memory at `y` contains an `X`; calling `y.~Y()` is undefined behavior. (@Luc Danton You cannot "reconstruct" the base class of a derived class without the derived class ceasing to exist as such.) – James Kanze Jun 03 '11 at 10:34
  • @JamesKanze Thanks for the clarification; AProgrammer showed how this could go wrong indeed. – Luc Danton Jun 03 '11 at 10:38
  • @James @Luc, so these 2 cases makes this scenario a UB right ? – iammilind Jun 03 '11 at 10:39
  • 2
    Both of the cases in the answer above result in UB. In general, the idiom is to be avoided, because there are so many different ways it can end up causing UB. – James Kanze Jun 03 '11 at 10:52