const member and assignment operator. How to avoid the undefined behavior?

Question

I answered the question about std::vector of objects and const-correctness, and received a comment about undefined behavior. I do not agree and therefore I have a question.

Consider the class with const member:

class A { 
public: 
    const int c; // must not be modified! 
    A(int c) : c(c) {} 
    A(const A& copy) : c(copy.c) { }     
    // No assignment operator
}; 

I want to have an assignment operator but I do not want to use const_cast like in the following code from one of the answers:

A& operator=(const A& assign) 
{ 
    *const_cast<int*> (&c)= assign.c;  // very very bad, IMHO, it is undefined behavior
    return *this; 
} 

My solution is

A& operator=(const A& right)  
{  
    if (this == &right) return *this;  
    this->~A() 
    new (this) A(right); 
    return *this;  
}  

Do I have undefined behavior (UB)?

What would be a solution without UB?

Answer 1

Your code causes undefined behavior.

Not just "undefined if A is used as a base class and this, that or the other". Actually undefined, always. return *this is already UB, because this is not guaranteed to refer to the new object.

Specifically, consider 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 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,

Now, "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" is exactly what you are doing.

Your object is of class type, and it does contain a non-static data member whose type is const-qualified. Therefore, after your assignment operator has run, pointers, references and names referring to the old object are not guaranteed to refer to the new object and to be usable to manipulate it.

As a concrete example of what might go wrong, consider:

A x(1);
B y(2);
std::cout << x.c << "\n";
x = y;
std::cout << x.c << "\n";

Expect this output?

1
2

Wrong! It's plausible you might get that output, but the reason const members are an exception to the rule stated in 3.8/7, is so that the compiler can treat x.c as the const object that it claims to be. In other words, the compiler is allowed to treat this code as if it was:

A x(1);
B y(2);
int tmp = x.c
std::cout << tmp << "\n";
x = y;
std::cout << tmp << "\n";

Because (informally) const objects do not change their values. The potential value of this guarantee when optimizing code involving const objects should be obvious. For there to be any way to modify x.c without invoking UB, this guarantee would have to be removed. So, as long as the standard writers have done their job without errors, there is no way to do what you want.

[*] In fact I have my doubts about using this as the argument to placement new - possibly you should have copied it to a void* first, and used that. But I'm not bothered whether that specifically is UB, since it wouldn't save the function as a whole.

Answer 2

First: When you make a data member const, you're telling the compiler and all the world that this data member never changes. Of course then you cannot assign to it and you certainly must not trick the compiler into accepting code that does so, no matter how clever the trick.
You can either have a const data member or an assignment operator assigning to all data members. You can't have both.

As for your "solution" to the problem:
I suppose that calling the destructor on an object within a member function invoked for that objects would invoke UB right away. Invoking a constructor on uninitialized raw data to create an object from within a member function that's been invoked for an object that resided where now the constructor is invoked on raw data... also very much sounds like UB to me. (Hell, just spelling this out makes my toenails curl.) And, no, I don't have chapter and verse of the standard for that. I hate reading the standard. I think I can't stand its meter.

However, technicalities aside, I admit that you might get away with your "solution" on just about every platform as long as the code stays as simple as in your example. Still, this doesn't make it a good solution. In fact, I'd argue it's not even an acceptable solution, because IME code never stays as simple as that. Over the years it will get extended, changed, mutated, and twisted and then it will silently fail and require a mind-numbing 36hrs shift of debugging in order to find the problem. I don't know about you, but whenever I find a piece of code like this responsible for 36hrs of debugging fun I want to strangle the miserable dumb-wit who did this to me.

Herb Sutter, in his GotW #23, dissects this idea piece by piece and finally concludes that it "is full of pitfalls, it's often wrong, and it makes life a living hell for the authors of derived classes... never use the trick of implementing copy assignment in terms of copy construction by using an explicit destructor followed by placement new, even though this trick crops up every three months on the newsgroups" (emphasize mine).

Answer 3

How can you possibly assign to an A if it has a const member? You're trying to accomplish something that's fundamentally impossible. Your solution has no new behaviour over the original, which is not necessarily UB but yours most definitely is.

The simple fact is, you're changing a const member. You either need to un-const your member, or ditch the assignment operator. There is no solution to your problem- it's a total contradiction.

Edit for more clarity:

Const cast does not always introduce undefined behaviour. You, however, most certainly did. Apart from anything else, it is undefined not to call all destructors- and you didn't even call the right one- before you placed into it unless you knew for certain that T is a POD class. In addition, there's owch-time undefined behaviours involved with various forms of inheritance.

You do invoke undefined behaviour, and you can avoid this by not trying to assign to a const object.

Answer 4

If you definitely want to have an immutable (but assignable) member, then without UB you can lay things out like this:

#include <iostream>

class ConstC
{
    int c;
protected:
    ConstC(int n): c(n) {}
    int get() const { return c; }
};

class A: private ConstC
{
public:
    A(int n): ConstC(n) {}
    friend std::ostream& operator<< (std::ostream& os, const A& a)
    {
        return os << a.get();
    }
};

int main()
{
    A first(10);
    A second(20);
    std::cout << first << ' ' << second << '\n';
    first = second;
    std::cout << first << ' ' << second << '\n';
}

Answer 5

Have a read of this link:

http://www.informit.com/guides/content.aspx?g=cplusplus&seqNum=368

In particular...

This trick allegedly prevents code reduplication. However, it has some serious flaws. In order to work, C’s destructor must assign NULLify every pointer that it has deleted because the subsequent copy constructor call might delete the same pointers again when it reassigns a new value to char arrays.

Answer 6

In absence of other (non-const) members, this doesn't make any sense at all, regardless of undefined behavior or not.

A& operator=(const A& assign) 
{ 
    *const_cast<int*> (&c)= assign.c;  // very very bad, IMHO, it is UB
    return *this; 
}

AFAIK, this is no undefined behavior happening here because c is not a static const instance, or you couldn't invoke the copy-assignment operator. However, const_cast should ring a bell and tell you something is wrong. const_cast was primarily designed to work around non const-correct APIs, and it doesn't seem to be the case here.

Also, in the following snippet:

A& operator=(const A& right)  
{  
    if (this == &right) return *this;  
    this->~A() 
    new (this) A(right); 
    return *this;  
}

You have two major risks, the 1st of which has already been pointed out.

1. In presence of both an instance of derived class of A and a virtual destructor, this will lead to only partial reconstruction of the original instance.
2. If the constructor call in new(this) A(right); throws an exception, your object will be destroyed twice. In this particular case, it won't be a problem, but if you happen to have significant cleanup, you're going to regret it.

Edit: if your class has this const member that is not considered "state" in your object (i.e. it is some sort of ID used for tracking instances and is not part of comparisons in operator== and the like), then the following might make sense:

A& operator=(const A& assign) 
{ 
    // Copy all but `const` member `c`.
    // ...

    return *this;
}