Class has virtual method but non virtual destructor C++

12,264

Solution 1

Classes for use as polymorphic bases should have either a virtual destructor or a protected destructor.

The reason is that if you have a public, non-virtual destructor, then pretty much any use of it by an external user of the class is unsafe. For example:

GenericSymbolGenerator *ptr = new PascalPredefinedSymbolGenerator();
delete ptr; // behavior is undefined, we tried to call the base class destructor

By marking the destructor protected, you prevent the user from deleting a PascalPredefinedSymbolGenerator object via the base class. By making the destructor public and virtual, you get defined behavior when the user deletes through the base class. So pick one of those options.

Solution 2

I would argue that the compiler is correct to warn you about the non-virtual destructor in the base class. You have created a class that is clearly intended as the root of an inheritance hierarchy, but by making the base class destructor non-virtual, you've broken your ability to delete an object by pointer to the base class (as all that will be executed is the destructor of the base class, so no derived class specific actions will be taken). This is considered a really bad idea in C++ as you essentially break the implementation of polymorphism when it comes to this object hierarchy.

As you mentioned in your comments, your intent is to use GenericSymbolGenerator as an interface and force the user to instantiate and use derived classes containing the actual implementation code. The canonical way to declare an interface in C++ is to declare at least one function on the interface as a pure virtual function. This prohibits you from instantiating the base class, but still create instantiable derived classes. You've already done this by declaring generateSymbolTableCollection() as a pure virtual function in your base class. So all you need to do is make the destructor virtual as it really has to be virtual in this particular scenario.

Also, as an aside, the canonical signatures for the default constructor and destructor are normally written without using "void" as a parameter, just use the empty parentheses instead.

Share:
12,264
Sebi
Author by

Sebi

Updated on June 20, 2022

Comments

  • Sebi
    Sebi almost 2 years

    Possible Duplicate:
    GNU compiler warning “class has virtual functions but non-virtual destructor”

    I am writing an interface for two classes and I get the warning in the title. Here's the code:

    class GenericSymbolGenerator {
       protected:                     // <== ok 
        ~GenericSymbolGenerator(void) {}
    
       public:
        virtual GenericSymbolTableCollection* generateSymbolTableCollection(GenericSymbolTableCollection *gst) = 0;
        GenericSymbolGenerator(void) {}
        // ~GenericSymbolGenerator(void) {} // <== warning if used
    };
    
    class PascalPredefinedSymbolGenerator : public GenericSymbolGenerator {
       protected:
        ~PascalPredefinedSymbolGenerator(void) {} // <== ok
    
       public:
        GenericSymbolTableCollection* generateSymbolTableCollection(GenericSymbolTableCollection *pst); // initializes *pst
        PascalPredefinedSymbolGenerator(void) {}
        // ~PascalPredefinedSymbolGenerator(void) {} <== warning if used
    };
    
    class PascalSymbolGenerator : public GenericSymbolGenerator {
        protected:
             ~PascalSymbolGenerator(void) {} // <== ok
    
        public:
         GenericSymbolTableCollection* generateSymbolTableCollection(GenericSymbolTableCollection *st); // initializes st
         PascalSymbolGenerator(void) {}
         // ~PascalSymbolGenerator(void) {} // <== warning if used
    };
    

    As long as the constructor/destructor is void there is no issue in declaring the destructor as protected. The problem arises when the class makes use of the heap(the destructor being declared as protected there is no way of freeing the class from the "outside" making the object "indestructible"). Is there a more convenient approach(aside from going public all the way)?