Pure virtual method called

Question

I understand why calling a virtual function from a constructor is bad, but I'm not sure why defining a destructor would result in a "pure virtual method called" exception. The code uses const values to reduce the use of dynamic allocation - possibly also the culprit.

#include <iostream>
using namespace std;

class ActionBase {
 public:
    ~ActionBase() { } // Comment out and works as expected

    virtual void invoke() const = 0;
};

template <class T>
class Action : public ActionBase {
 public:
    Action( T& target, void (T::*action)())
     : _target( target ), _action( action ) { }

    virtual void invoke() const {
        if (_action) (_target.*_action)();
    }

    T&   _target;
    void (T::*_action)();
};

class View {
 public:
    void foo() { cout << "here" << endl; }
};

class Button : public View {
 public:
    Button( const ActionBase& action )
     : _action( action ) { }

    virtual void mouseDown() {
        _action.invoke();
    }

 private:
    const ActionBase& _action;
};

int main( int argc, char* argv[] )
{
    View view;
    Button button = Button( Action<View>( view, &View::foo ) );
    button.mouseDown();

    return 0;
}

Answer 1

You have Undefined Behavior. As the parameter to Button's ctor is a const& from a temporary, it is destroyed at the end of that line, right after the ctor finishes. You later use _action, after Action's dtor has already run. Since this is UB, the implementation is allowed to let anything happen, and apparently your implementation happens to do something slightly different depending on whether you have a trivial dtor in ActionBase or not. You get the "pure virtual called" message because the implementation is providing behavior for calling ActionBase::invoke directly, which is what happens when the implementation changes the object's vtable pointer in Action's dtor.

I recommend using boost.function or a similar 'action callback' library (boost has signals and signals2, for example).

Answer 2

Set a breakpoint on the destructor and it will become clear what is happening. Yup, you are passing a temporary instance of Action<> to the Button constructor. It is destroyed after the button construct runs. Write it like this and the problem disappears:

View view;
Action<View> event(view, &View::foo);
Button button = Button( event ); 
button.mouseDown();

Well, that's not a practical solution, event is not going to be in scope for a real mouseDown invocation. The Button constructor is going to have to create a copy of the "event" argument or it is going to have to manage a pointer to the delegate.

Answer 3

A class with virtual functions should always have a virtual destructor, so ~ActionBase() should be virtual, (and so should ~Action()). If you turn on more compiler warning you will get a warning about this.

Essentially, because of the lookup rules, the destructor is called for a type that the compiler knows cannot be instantiated (pure virtual), so it knows something must have gone wrong.

I'm sure someone else can explain better than I can :)