static_cast from base class pointer to derived class pointer is invalid

Question

I am creating a simple test entity-component system. I have a base Component class with several derived classes. I then have several systems that apply some logic to these components.

// Component.h
// ------------
class Component
{
public:
    Component();
    ~Component();
}


// ControlComponent.h
// -------------------
#include <string>
#include "Component.h"

class ControlComponent : public Component
{
public:
    std::string input = ""; // store simple input instruction
    ControlComponent();
    ~ControlComponent();
};


// ControlSystem.cpp
void ControlSystem::update(Entity* entity)
{
    vector<Component*>* components = entity->getComponents();

    for (Component* component : *components)
    {
        PositionComponent* pc = static_cast<PositionComponent*>(component);
        ControlComponent* cc = static_cast<ControlComponent*>(component);

        if (pc != nullptr && cc != nullptr)
        {
            std::cout << "Which direction would you like to go?" << std::endl;
            std::string input;
            std::cin >> input;
            cc->input = input; // application breaks here

            // Apply some logic...
        }
    }
}

When I static_cast from base Component* to either of the derived components (PositionComponent* or ControlComponent*) and when both results are not nullptr(i.e the cast was successful), I get invalid values, like cc->input not being able to read characters from string etc.

I wire up the components in my entity factory, like this:

void EntityFactory::wireUpPlayer(Entity* player)
{
    player->addComponent(new HealthComponent());
    player->addComponent(new ControlComponent());
    player->addComponent(new PositionComponent());
}

And the implementation for addComponent is as follows:

void Entity::addComponent(Component* component)
{
    m_components.push_back(component);
}

These components are shown to have valid memory addresses, so I'm not sure where the issue is coming from.

You are attributing the features of a `dynamic_cast` to a `static_cast`. This cannot end well. — IInspectable, Feb 02 '17 at 20:16

score 8 · Accepted Answer · answered Feb 02 '17 at 20:17

8

static_cast does not check validity at runtime; if the cast compiled, it assumes at runtime that the conversion is okay. If you aren't casting a null pointer, the result of a static_cast will not be a null pointer. To get a checked cast you need dynamic_cast and that, in turn, requires the pointer being converted to point to a polymorphic type, i.e., one that has at least one virtual function. That means changing Component to have at least one virtual function.

answered Feb 02 '17 at 20:17

Pete Becker

74,985
8
76
165

2

And since `Component` is being derived from, making the d'tor virtual is almost always the right thing to do. – IInspectable Feb 02 '17 at 20:18
@IInspectable - maybe, but inheritance alone doesn't tell you much; it depends on how object lifetimes are being managed. The rule is that if the design calls for deleting an object of a derived type through a pointer to the base type, the base type's destructor must be virtual. – Pete Becker Feb 02 '17 at 20:21
1

True. That appears to be what the OP is doing, though, by holding pointers to the base class in a collection. – IInspectable Feb 02 '17 at 20:24
@IInspectable - maybe. But I really don't want to pursue design speculations that aren't relevant to the question. – Pete Becker Feb 02 '17 at 20:26

score 6 · Answer 2 · answered Feb 02 '17 at 20:16

When I static_cast from base Component* to either of the derived components (PositionComponent* or ControlComponent*) and when both results are not nullptr(i.e the cast was successful)...

When casting from a base class to a derived class, static_cast is telling the compiler, "Trust me, I know what I'm doing." In other words, if it's even potentially legal, it will "succeed" and return non-nullptr. If, at runtime, it's not legal, you'll get undefined behavior, from trying to use an instance of one class as if it were of another class.

Use dynamic_cast instead.

It's undefined behaviour, so you might get a null pointer or anything else — M.M, Feb 02 '17 at 21:09

score 1 · Answer 3 · edited May 23 '17 at 12:08

As Pete Becker and Josh Kelley said, use dynamic_cast and I believe you also need to set at least one function as virtual. If you do not, the compiler will not record the inheritance and dynamic_cast will likely still return nullptr. When performing inheritance, I suggest making the class destructors virtual. This is also good practice when unmanaged resources need to be disposed of in the destructor of a derived class and you only have a pointer to the base class, the derived class destructor will only be called so long as the destructors are virtual. There was a post that explained it here: When to use virtual destructors?

static_cast from base class pointer to derived class pointer is invalid

3 Answers3

Linked