C++: Why is const_cast evil?

Question

I keep hearing this statement, while I can't really find the reason why const_cast is evil.

In the following example:

template <typename T>
void OscillatorToFieldTransformer<T>::setOscillator(const SysOscillatorBase<T> &src)
{
    oscillatorSrc = const_cast<SysOscillatorBase<T>*>(&src);
}

I'm using a reference, and by using const, I'm protecting my reference from being changed. On the other hand, if I don't use const_cast, the code won't compile. Why would const_cast be bad here?

The same applies to the following example:

template <typename T>
void SysSystemBase<T>::addOscillator(const SysOscillatorBase<T> &src)
{
    bool alreadyThere = 0;
    for(unsigned long i = 0; i < oscillators.size(); i++)
    {
        if(&src == oscillators[i])
        {
            alreadyThere = 1;
            break;
        }
    }
    if(!alreadyThere)
    {
        oscillators.push_back(const_cast<SysOscillatorBase<T>*>(&src));
    }
}

Please provide me some examples, in which I can see how it's a bad idea/unprofessional to use a const_cast.

Thank you for any efforts :)

Answer 1

Because you're thwarting the purpose of const, which is to keep you from modifying the argument. So if you cast away the constness of something, it's pointless and bloating your code, and it lets you break promises that you made to the user of the function that you won't modify the argument.

In addition, using const_cast can cause undefined behaviour. Consider this code:

SysOscillatorBase<int> src;
const SysOscillatorBase<int> src2;

...

aFieldTransformer.setOscillator(src);
aFieldTransformer.setOscillator(src2);

In the first call, all is well. You can cast away the constness of an object that is not really const and modify it fine. However, in the second call, in setOscillator you are casting away the constness of a truly const object. If you ever happen to modify that object in there anywhere, you are causing undefined behaviour by modifying an object that really is const. Since you can't tell whether an object marked const is really const where it was declared, you should just never use const_cast unless you are sure you'll never ever mutate the object ever. And if you won't, what's the point?

In your example code, you're storing a non-const pointer to an object that might be const, which indicates you intend to mutate the object (else why not just store a pointer to const?). That might cause undefined behaviour.

Also, doing it that way lets people pass a temporary to your function:

blah.setOscillator(SysOscillatorBase<int>()); // compiles

And then you're storing a pointer to a temporary which will be invalid when the function returns¹. You don't have this problem if you take a non-const reference.

On the other hand, if I don't use const_cast, the code won't compile.

Then change your code, don't add a cast to make it work. The compiler is not compiling it for a reason. Now that you know the reasons, you can make your vector hold pointers to const instead of casting a square hole into a round one to fit your peg.

So, all around, it would be better to just have your method accept a non-const reference instead, and using const_cast is almost never a good idea.

---

¹ Actually when the expression in which the function was called ends.

Answer 2

by using const, I'm protecting my reference from being changed

References can't be changed, once initialized they always refer to the same object. A reference being const means the object it refers to cannot be changed. But const_cast undoes that assertion and allows the object to be changed after all.

On the other hand, if I don't use const_cast, the code won't compile.

This isn't a justification for anything. C++ refuses to compile code that may allow a const object to be changed because that is the meaning of const. Such a program would be incorrect. const_cast is a means of compiling incorrect programs — that is the problem.

For example, in your program, it looks like you have an object

std::vector< SysOscillatorBase<T> * > oscillators

Consider this:

Oscillator o; // Create this object and obtain ownership
addOscillator( o ); // cannot modify o because argument is const
// ... other code ...
oscillators.back()->setFrequency( 3000 ); // woops, changed someone else's osc.

Passing an object by const reference means not only that the called function can't change it, but that the function can't pass it to someone else who can change it. const_cast violates that.

The strength of C++ is that it provides tools to guarantee things about ownership and value semantics. When you disable those tools to make the program compile, it enables bugs. No good programmer finds that acceptable.

As a solution to this particular problem, it looks likely that the vector (or whatever container you're using) should store the objects by value, not pointer. Then addOscillator can accept a const reference and yet the stored objects are modifiable. Furthermore, the container then owns the objects and ensures they are safely deleted, with no work on your part.

Answer 3

The use of const_cast for any reason other than adapting to (old) libraries where the interfaces have non-const pointers/references but the implementations don't modify the arguments is wrong and dangerous.

The reason that it is wrong is because when your interface takes a reference or pointer to a constant object you are promising not to change the object. Other code might depend on you not modifying the object. Consider for example, a type that holds an expensive to copy member, and that together with that it holds some other invariants.

Consider a vector<double> and a precomputed average value, the *average is updated whenever a new element is added through the class interface as it is cheap to update then, and if it is requested often there is no need to recompute it from the data every time. Because the vector is expensive to copy, but read access might be needed the type could offer a cheap accessor that returns a std::vector<double> const & for user code to check values already in the container. Now, if user code casts away the const-ness of the reference and updates the vector, the invariant that the class holds the average is broken and the behavior of your program becomes incorrect.

It is also dangerous because you have no guarantee that the object that you are passed is actually modifiable or not. Consider a simple function that takes a C null terminated string and converts that to uppercase, simple enough:

void upper_case( char * p ) {
   while (*p) {
     *p = ::to_upper(*p);
      ++p;
   }
}

Now lets assume that you decide to change the interface to take a const char*, and the implementation to remove the const. User code that worked with the older version will also work with the new version, but some code that would be flagged as an error in the old version will not be detected at compile time now. Consider that someone decided to do something as stupid as upper_case( typeid(int).name() ). Now the problem is that the result of typeid is not just a constant reference to a modifiable object, but rather a reference to a constant object. The compiler is free to store the type_info object in a read-only segment and the loader to load it in a read-only page of memory. Attempting to change it will crash your program.

Note that in both cases, you cannot know from the context of the const_cast whether extra invariants are maintained (case 1) or even if the object is actually constant (case 2).

On the opposite end, the reason for const_cast to exist was adapting to old C code that did not support the const keyword. For some time functions like strlen would take a char*, even though it is known and documented that the function will not modify the object. In that case it is safe to use const_cast to adapt the type, not to change the const-ness. Note that C has support for const for a very long time already, and const_cast has lesser proper uses.

Answer 4

The const_cast would be bad because it allows you to break the contract specified by the method, i.e. "I shall not modify src". The caller expects the method to stick to that.

Answer 5

It's at least problematic. You have to distinguish two constnesses:

• constness of the instantiated variable
  This may result in physical constness, the data being placed in a read-only segment

• constness of the reference parameter / pointer
  This is a logical constness, only enforced by the compiler

You are allowed to cast away the const only if it's not physically const, and you can't determine that from the parameter.

In addition, it's a "smell" that some parts of your code are const-correct, and others aren't. This is sometimes unavoidable.

---

In your first example, you assign a const reference to what I assume is a non-const pointer. This would allow you to modify the original object, which requires at least a const cast. To illustrate:

SysOscillatorBase<int> a;
const SysOscillatorBase<int> b;

obj.setOscillator(a); // ok, a --> const a --> casting away the const
obj.setOscilaltor(b); // NOT OK: casting away the const-ness of a const instance

Same applies to your second example.

Answer 6

, while I can't really find the reason why const_cast is evil.

It is not, when used responsibily and when you know what you're doing. (Or do you seriously copy-paste code for all those methods that differ only by their const modifier?)

However, the problem with const_cast is that it can trigger undefined behavior if you use it on variable that originally was const. I.e. if you declare const variable, then const_cast it and attempt to modify it. And undefined behavior is not a good thing.

Your example contains precisely this situation: possibly const variable converted into non-const. To avoid the problem store either const SysOscillatorBase<T>*(const pointer) or SysOscillatorBase<T> (copy) in your object list, or pass reference instead of const reference.

Answer 7

You are violating a coding contract. Marking a value as const is saying you can use this value but never change it. const_cast breaks this promise and can create unexpected behaviour .

In the examples you give, it seems your code is not quite right. oscillatorSrc should probably be a const pointer, although if you really do need to change the value then you should not pass it in as a const at all.

Answer 8

Basicly const promises you and the compiler that you will not change the value. The only time you should use when you use a C library function (where const didn't exist), that is known not to change the value.

bool compareThatShouldntChangeValue(const int* a, const int* b){
    int * c = const_cast<int*>(a);
    *c = 7;
    return a == b;
}

int main(){
   if(compareThatShouldntChangeValue(1, 7)){
      doSomething();
   }
}

Answer 9

You probably need to define you container as containing const objects

template <typename T> struct Foo {
    typedef std::vector<SysOscillator<T> const *>  ossilator_vector;
}

Foo::ossilator_vector<int> oscillators;


// This will compile
SysOscillator<int> const * x = new SysOscillator<int>();
oscillators.push_back(x);

// This will not
SysOscillator<int> * x = new SysOscillator<int>();
oscillators.push_back(x);

That being said if you have no control over the typedef for the container maybe it is ok to const_cast at the interface between your code and the library.