46

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?

roschach
  • 8,390
  • 14
  • 74
  • 124
Max
  • 3,824
  • 8
  • 41
  • 62
  • 1
    Why do you want a pure virtual constructor in the first place? A empty virtual constructor works just well (and it amounts to the same number of non whitespace chars) – Gunther Piez Apr 05 '12 at 08:20
  • 1
    @LuchianGrigore That wasn't what I meant. I rephrase it for you: Why do you want the class Media to be abstract? – Gunther Piez Apr 05 '12 at 08:23
  • 1
    @drhirsch Usually because you want to disallow its creation. Perhaps an object of type Media doesn't make sense. – Luchian Grigore Apr 05 '12 at 08:31
  • 2
    If someone just needs to disable the "class has virtual functions but non-virtual destructor" warnings, it's `-Wno-non-virtual-dtor`. I'm not suggesting it as an answer to this question though. – Raptor007 Apr 21 '17 at 22:02

5 Answers5

52

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.

Luchian Grigore
  • 253,575
  • 64
  • 457
  • 625
  • 4
    "all classes that will be derived from should have a virtual destructor" - or a protected destructor, which will prevent outsiders from deleting via the wrong pointer. This is for cases where the outsider isn't supposed to directly delete anyway (for example because the objects are created via some factory that returns a smart pointer). – Steve Jessop Apr 05 '12 at 08:09
  • Can't believe I have to implement an empty destructor. Problem solved, thanks guys! – Max Apr 05 '12 at 08:15
  • 6
    @crazyfffan: If your compiler is new enough, you can also say `= default` (C++11 feature) – MSalters Apr 05 '12 at 08:43
  • 1
    Isn't it enough just to define empty constructor: `virtual ~Media() {}` (Jerry Coffin's answer)? – pevik Apr 20 '16 at 09:45
  • 1
    @pevik If one can use `= default`, one should; user-providing a special member function, even if in practice the implementation is exactly what the default would do, can block or hinder other features. – underscore_d Apr 17 '21 at 10:04
31

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

Matthieu M.
  • 287,565
  • 48
  • 449
  • 722
  • But note that least officially, most of this is speculation. Officially, destroying a derived object via a pointer/reference to base requires a virtual dtor--otherwise, the behavior is simply undefined. – Jerry Coffin Apr 30 '21 at 16:23
  • There are other reasons you should give every class with at least one virtual function a virtual destructor. – Spencer Dec 06 '22 at 19:08
  • @Spencer: can you name them? I can't think of anything, other than actually destructing the object correctly. – Violet Giraffe Mar 09 '23 at 18:16
11

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;
Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111
7

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

Community
  • 1
  • 1
lucian.pantelimon
  • 3,673
  • 4
  • 29
  • 46
0

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

Media::~Media(){}
Hiren
  • 341
  • 1
  • 4
  • 17
  • 1
    No. Adding this AFTER the class declaration will yield multiple definition during linkage. The definition must be in only one translation unit. – Luchian Grigore Apr 05 '12 at 08:09
  • 1
    That's still not correct. Adding it after the class declaration, inside a header file, will result in error. – Luchian Grigore Apr 05 '12 at 08:15
  • @LuchianGrigore can you tell me what kind of error will the compiler give if I add definition in .h file? It works just fine on Visual Studio – Hiren Apr 05 '12 at 08:49
  • @Hiren If you put it in a header file included in multiple c++ sources, you'll get a linker error because the definition would be replicated once per .cpp file. – Al.G. May 23 '19 at 10:37