C++ Runtime polymorphism calling unexpected override when multiple classes are inherited

Question

I am trying to create a GUI system with C++11 where, perhaps like Godot, every "node" has a very specific purpose such as grouping other nodes, drawing a rectangle, or detecting when a specific area of the screen is pressed down. Some parts of the code are getting a little repetitive as switch statements will determine a node's type and cast it appropriately, only to call the exact same function name in each case.

I thought I could clean this up by abstracting each node a bit more so the Rectangle class would inherit not just the BaseNode class, but also the Visibility class to specify that it has a Draw() function to call. By making Visibility::Draw() a virtual function and overriding it in each derived class, I should be able to replace the long switch statement with a single line that casts the BaseNode* to a Visibility* and call Draw() on that to display the rectangle, circle, image, polygon, or whatever specific functionality that individual node is programmed to have.

I wrote up this theoretically good solution yet found that it would call neither Rectangle::Draw() nor Visibility::Draw() but instead some random part of memory!

I eventually pinpointed the issue and wrote up this small demo to replicate it. Casting pRect to a Visibility* and calling the Draw() function will execute Rectangle::BasicFunc() inherited from BaseNode instead of Rectangle::Draw() inherited from Visibility.

#include <iostream>

class BaseNode {
    public:
        virtual void BasicFunc() {
            std::cout << "BaseNode BasicFunc()" << std::endl;
        }
};

class Visibility {
    public:
        virtual void Draw() {
            std::cout << "Visibility Draw()" << std::endl;
        }
};

class Rectangle : public BaseNode, public Visibility {
    public:
        void BasicFunc() {
            std::cout << "Rectangle BasicFunc()" << std::endl;
        }
        void Draw() {
            std::cout << "Rectangle Draw()" << std::endl;
        }
};

int main() {
    BaseNode* pRect = new Rectangle;
    
    ((Visibility*)pRect)->Draw(); // Calls Rectangle::BasicFunc() instead of Rectangle::Draw()

    return 0;
}

Some other testing I've done:

• Casting pRect to a Rectangle* instead will correctly call Rectangle::Draw(), but then it's back to the bulky switch statement.
• Removing the virtual keyword from Visibility::Draw will make the Draw() call execute Visibility::Draw(). Correct function name but not the correct class.
• Order of overriding functions in the Rectangle class does not matter.
• Initializing pRect as a BaseNode* will make BasicFunc() always work.
• Initializing pRect as a Visibility* will make Draw() always work.
• Casting pRect from a BaseNode* to a Visibility* will make Draw() always fail and call BasicFunc() instead. (The current issue)
• Casting pRect from a Visibility* to a BaseNode* will make BasicFunc() always fail and call Draw() instead.
• Initializing pRect as a void* will make both BasicFunc() and Draw() call the function associated with Rectangle's first inherited class.

I am completely unsure how to go about fixing this. Any advice is appreciated!

Answer 1

Running with address sanitizer indicates the problem immediately:

runtime error: member call on address 0x602000000010 which does not point to an object of type 'Visibility'

The issue here is that you cannot directly cast a BaseNode object to a Visibility object because there is no such relationship between these two classes. What you have here is undefined behavior.

Because a static_cast is impossible here, the compiler resorts to a reinterpret_cast, which in this case leads to you invoking the virtual table of a different class from the one the compiler thinks it's using.

One possible workaround is to use use dynamic_cast:

auto pVisibility = dynamic_cast<Visibility*>(pRect);
if (pVisibility) {
    pVisibility->Draw();
}

However, using this can often be seen as a "code smell". There are various other approaches you may use, but that's entirely up to your other design considerations.

As an aside, you should add a virtual destructor to BaseNode, because right now trying to delete pRect; has the potential to cause undefined behavior:

class BaseNode {
    public:
        virtual ~BaseNode() = default;
        virtual void BasicFunc() {
            std::cout << "BaseNode BasicFunc()" << std::endl;
        }
};