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

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

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

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

Solution 4

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

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

View more solutions

Comments

[Cody Gray]

The whole purpose of const is to prevent you from modifying something, that's why your code generates an error. Adding in const_cast is basically telling the compiler to shut up, that you know what you're doing. That's why it's not a good idea. If you don't want it to be const, don't declare it const.

[Rafał Rawicki]

The only thing which can be evil is a programmer. The const cast is not evil, it just should not be used when it is not necessary.

[Seth Carnegie]

@user1240436 And after that level of professionalism, you reach another level of professionalism at which you realise you never needed const_cast anyway and there was a better way of doing it in the first place.

[Mike DeSimone]

Neither of these functions should take a const SysOscillatorBase<T> &. Instead, they should take a SysOscillatorBase<T> &. Since oscillatorSrc is not const when it could have been, the programmer is telling the compiler (and the world) that it reserves the right to meddle with the innards of oscillatorSrc. If you aren't changing oscillatorSrc, then it should be const as well and your problem goes away. Same with the vector.

[Seth Carnegie]

Using that function for an example is actually good in more ways than may be apparent at first sight, +1.

[David Rodríguez - dribeas]

@SethCarnegie: lol... I had missed the ++p, thanks for pointing it out.

[Mike DeSimone]

No... If you have no control over the container, then the methods should not take const arguments.

[bradgonesurfing]

If the library that you have no control over is badly written and it is obvious that the container should hold const references but the const decl was just left out then why not make sure that your code holds const references and then adapt using const_cast at the interface to the bad library. When the library is fixed then remove the const_cast adaptation. Obviously if you are wrong and pass in a physical const object to the library and it tries to mutate it then kaboom I guess.

[Steve Jessop]

"any reason other than adapting to (old) libraries" - I agree, but try telling that to the people who believed Meyers's chapter in "Effective C++" about const and non-const accessors :-(

[Mike DeSimone]

Since it was created? At least since they used operator << for text output. Proper use of const is a concept that needs to be taught, sure, but it's not that hard. It's less confusing than C's choice of int as a default function return type and its implementation of for.

[criddell]

I believe that by declaring something as const, the compiler may make certain assumptions w.r.t. multi-threaded code. If you use const_cast and break your const promise, you may end up with different values for the thing in different threads.

[Seth Carnegie]

@criddell yes, that is an important part too.

[Kaiserludi]

"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?" Your code may have to pass the received parameter to some 3rd party function that you can't modify, but that is not const correct and for which you know that it won't ever modify the passed instance. In that case you can only make your own code const correct and still be able to pass the instance to that function, if you use a const_cast.

[Andrew Truckle]

What if both apis are Microsoft and one returns a const and then the next requires a non const?