Must I either mark the destructor of the base class as virtual function or mark it as protected func-CodePudding

As per this article, which says that[emphasis mine]:

Making base class destructor virtual guarantees that the object of derived class is destructed properly, i.e., both base class and derived class destructors are called.

As a guideline, any time you have a virtual function in a class, you should immediately add a virtual destructor (even if it does nothing). This way, you ensure against any surprises later.

I think even if your base class has no virtual function,you should either add a virtual destructor or mark the destructor of the base class as protected.Otherwise, you may face memory leak when trying to delete a pointer pointed to a derived instance. Am I right?

For example,here is the demo code snippet:

#include<iostream>

class Base {
public:
 Base(){};
 ~Base(){std::cout << "~Base()" << std::endl;}; 
};

class Derived : public Base {
private:
  double val;
public:
 Derived(const double& _val){};
 ~Derived(){std::cout << "~Derived()" << std::endl;}; //It would not be called
};

void do_something() {
 Base* p = new Derived{1};
 delete p;  
}

int main()
{
    do_something();
}

Here is the output of the said code snippet:

~Base()

Could anybody shed some light on this matter?

CodePudding user response：

This question will lead to a bunch of other questions about whether a programmer should always be super safe by protecting its code even against currently non existent problems.

In your current code, the Derived class only adds a trivial double to its base class, and a rather useless destructor, that only contains a trace print. If you deleted an object through a pointer to the base class, the Derived destructor will not be called, but it will be harmless. Furthermore as you were told in comment, using polymorphism (casting a pointer to a base class one) with no virtual function does not really makes sense.

Long story made short, it you have a class hierarchy with no virtual function, are aware of it and never delete an object through a pointer to its base class you have no strong reason to make the destructor virtual nor protected. But IMHO, you should at least leave a comment on the base class to warn future maintainers about that possible problem.

CodePudding user response：

The behavior of the program in the question is undefined. It deletes an object of a derived type through a pointer to its base type and the base type does not have a virtual destructor. So don't do that.

Some people like to write code that has extra overhead in order to "ensure against any surprises later". I prefer to write code that does what I need and to document what it does. If I decide later that I need it to do more, I can change it.

CodePudding user response：

fwiw it's happening because you try to delete it through Base. if you somehow really want to have object reference/pointer of only the base type, there are some alternatives.

void do_something() {
   Base&& b = Derived{1};
}

void do_something() {
   Derived d = Derived{1};
   Base* p = &d;
   Base& b = d;
}

void do_something() {
   std::shared_ptr<Base> sb = std::make_shared<Derived>(1);
}

// NOTE: `std::unique_ptr` doesn't work
void do_something() {
   std::unique_ptr<Base> ub = std::make_unique<Derived>(1); // warning: this not work
}

CodePudding user response：

Without a virtual destructor your call to delete is arguably wrong. It should be:

void do_something() {
    Base* p = new Derived{1};

    Derived *t = dynamic_cast<Derived*>(p);
    if (t) {
        delete t;
    } else {
        delete p;
    }
}

You can use up/down casting of objects to store them in a common array or vector and still be able to call methods of the derived objects all without a virtual destructor. It's usually a sign of bad design but it is legal C code. The cost is that you have to do cast back to the original types before delete like above.

Note: The dynamic_cast can only be done when Base has at least one virtual function. And if you have a virtual functions you should just add the virtual destructor.

And if you don't need to dynmaic_cast anywhere then show me case where you can't use an array of Base instead of Base *.

The point of making the destructor of the base class protected I guess is to generate an error when someone deletes a Base* so you can then make the destructor virtual. Means you don't have the overhead of a virtual destructor until you actually need it.