CODEWITHSUNDEEP
c++
Max
Max

Reputation: 3914

C++ "Virtual functions but no virtual destructors"

I have a base class Media and several derived classes, namely DVD, Book, etc... The base class is written as:

class Media{
    private:
        int id;
        string title;
        int year;
    public:
        Media(){ id = year = 0; title = ""; }
        Media(int _id, string _title, int _year): id(_id), title(_title), year(_year) {}
//      virtual ~Media() = 0;
        void changeID(int newID){ id = newID; }
        virtual void print(ostream &out);
};

The thing is: without the destructor, GCC gives me a bunch of warnings class has virtual functions but non-virtual destructor, but still compiles and my program works fine. Now I want to get rid of those annoying warnings so I satisfy the compiler by adding a virtual destructor, the result is: it doesn't compile, with the error:

undefined reference to `Media::~Media()`

Making the destructor pure virtual doesn't solve the problem. So what has gone wrong?

Upvotes: 51

Views: 81895

Answers (5)

Jerry Coffin
Jerry Coffin

Reputation: 490148

What you have commented out is a pure-virtual declaration for a destructor. That means the function must be overridden in a derived class to be able to instantiate an object of that class.

What you want is just a definition of the destructor as a virtual function:

virtual ~Media() {}

In C++ 11 or newer, it's generally preferable to define this as defaulted instead of using an empty body:

virtual ~Media() = default;

Upvotes: 13

Matthieu M.
Matthieu M.

Reputation: 299890

The thing is: without the destructor, GCC gives me a bunch of warnings "class has virtual functions but non-virtual destructor", but still compiles and my program works fine

This is an annoying warning in Modern C++, but in old object-style C++ it is generally correct.

The problem is about the way your objects are destructed. A simple test:

#include <iostream>

class Base {};
class Derived: public Base { public: ~Derived() { std::cout << "Aargh\n"; } };

int main() {
  Base* b = new Derived();
  Derived* d = new Derived();

  delete d;
  delete b;
}

This prints:

Aargh

Yep, only once.

The problem is that when you call delete on a variable of type Base*, the Base::~Base() method is called. If it is virtual, then the call is dynamically dispatched to the final method (based on the dynamic type), in this case Derived::~Derived(), but if it is not, then Derived::~Derived() is never called, thus never executed.

Therefore, if you wish to call delete (or use smart pointers which do it for you) on base types, then you need to add virtual ~Base() {} in their class definitions. This is why gcc warns you when you create a polymorphic class without a virtual destructor.

Note: time changed, and since then I implemented -Wdelete-non-virtual-dtor in Clang and it was replicated in gcc as well.

-Wnon-virtual-dtor is useful for library writers (as it warns on the base class), but may have a higher false positive rate; on the other hand -Wdelete-non-virtual-dtor fires at the call site, and has a much lower false positive rates (which you can generally work around by peppering final to remove the "polymorphic" property of the class).

Upvotes: 33

lucian.pantelimon
lucian.pantelimon

Reputation: 3669

You should implement the virtual destructor, not make it pure virtual.

Look at this similar question (identical maybe from the point of view of the virtual destructor error, not the warning) for more info.

EDIT: more generic solution, in reply to LuchianGrigore's comment (thank you for pointing it out)

You can also make the destructor pure virtual and implement as it is pointed in the above mentioned question.

The use of virtual destructors in you classes should be to prevent instantiation of the base class (i.e. when you have no other pure virtual methods to make the class abstract).

Upvotes: 7

Hiren
Hiren

Reputation: 341

Uncomment declaration first and then Try adding following line after class declaration

Media::~Media(){}

Upvotes: 0

Luchian Grigore
Luchian Grigore

Reputation: 258618

You need to also define the virtual destructor, not only add it.

//Media.h
class Media{
    //....
    virtual ~Media() = 0;
};

//Media.cpp
#include "Media.h"
//....
Media::~Media() {};

The reason you get the warnings is that all classes that will be derived from should have a virtual or protected (credit @Steve) destructor, otherwise deleting an instance via a pointer to a base class results in undefined behavior.

Note you HAVE TO provide a definition for destructors, even if they are pure virtual.

Upvotes: 55

Related Questions