const_cast a const member in a class constructor

Question

I sometimes use const_cast when I want a member variable of a class to be constant during the life of the class, but it needs to be mutable during the constructor. Example:

struct qqq {
 const vector<foo> my_foo;

  qqq(vector<foo>* other) {
    vector<foo>& mutable_foo = const_cast<vector<foo>&>(my_foo)
    other->swap(mutable_foo);
  }
};

I had assumed that doing this in the constructor was basically OK because nobody else is relying on it at this point so it wouldn't interact badly with optimization, etc.

However recently someone told me this is "undefined behavior" and that it's basically illegal to mutate a const object after it's been constructed under any circumstance.

Can someone clarify? Is this a bad / undefined behavior / thing to do?

Answer 1

It is Undefined Behavior. Per Paragraph 7.1.6.1/4 of the C++11 Standard:

Except that any class member declared mutable (7.1.1) can be modified, any attempt to modify a const object during its lifetime (3.8) results in undefined behavior.

In this case, it seems like you want your object to "become" constant after construction. This is not possible.

If your vector is meant to be const, you shall initialize it in the constructor's initialization list:

qqq(vector<foo>& other) 
    : my_foo(std::move(other)) 
//  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
{
}

Notice, that unless you have a good reason for passing by pointer - in which case, you should also be checking whether the pointer is non-null - you should consider passing by reference (as shown above), which is the common practice.

UPDATE:

As Pete Becker correctly points out in the comments, proper design would suggest that the decision to move from the vector argument should belong to the caller of qqq's constructor, and not to the constructor itself.

If the constructor is always supposed to move from its argument, then you could let it accept an rvalue reference, making it clear what the constructor itself is expecting out of the caller:

qqq(vector<foo>&& other) 
//             ^^
    : my_foo(std::move(other)) 
//  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
{
}

This way, the caller would have to provide an rvalue in input to qqq's constructor:

std::vector<foo> v;
// ...
qqq q1(v); // ERROR!
qqq q2(std::move(v)); // OK! Now the client is aware that v must be moved from

Answer 2

Yes, it's indeed UB (Undefined Behaviour). You cannot modify a const object once it's initialised. What you should do is use the member initialiser list, perhaps together with a function:

struct qqq {
  const vector<foo> my_foo;

  qqq(vector<foo> *other) : my_foo(initialiseFoo(*other)) {}

  static vector<foo> initialiseFoo(vector<foo> &other) {
    vector<foo> tmp;
    other.swap(tmp);
    return tmp;
  }
};

A decent optimiser should be able to get rid of the temporary.

If you can use C++11, it's actually even simpler:

struct qqq {
  const vector<foo> my_foo;

  qqq(vector<foo> *other) : my_foo(std::move(*other))
  {
    other->clear();  //Just in case the implementation of moving vectors is really weird
  }
};