2

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?

John
  • 2,963
  • 11
  • 33
  • https://stackoverflow.com/questions/461203/when-to-use-virtual-destructors – Mat May 28 '22 at 11:21
  • 1
    If you don't have virtual functions, then usually casting to base class does not make sense. – Sandro May 28 '22 at 11:31
  • 1
    +1, like the question, but (unrelated) I can't help but notice that such short snippet collects a bunch of what I consider bad habits (naked `new`, const reference to `double`, `std::endl`, explicitly defined empty constructor) – MatG May 28 '22 at 11:42
  • 1
    Since you have **inheritance**, it's *almost* a given that you have **polymorphism**. If you have polymorphism, you may as well have a `virtual` destructor. If you don't want to delete the object through a pointer to the base class, make it `protected`. (Technically, it doesn't need to be `virtual`, but if you have polymorphism, it's a negligible cost.) If you **don't** have polymorphism, then make the base class destructor `protected` and non-virtual. If the base class is *instantiable*, probably contrived code. – Eljay May 28 '22 at 12:18
  • @Sandro Really?Could you please explain that in more detail for me? – John May 28 '22 at 12:49
  • @MatG ***naked new, const reference to double, std::endl, explicitly defined empty constructor***. What's that? Could you please explain more in detail? – John May 28 '22 at 12:52
  • 1
    @John smart pointer or container, simply pass a value, `'\n'`, `=default` (or simply don't type anything) – apple apple May 28 '22 at 13:06
  • @John or `Base&& b = Derived{1};` (continue my previous comment.) – apple apple May 28 '22 at 13:10
  • @John instead of naked new, you should use `make_shared` or alike (and only if you really need to do dynamic allocation) – apple apple May 28 '22 at 13:11
  • @appleapple Could you explain why prefer `'\n'` to `std::endl`? Are there any harmful effects of flushing the output stream in this case? – wtz May 28 '22 at 15:03
  • 1
    @wtz -- if you redirect the output to a file you'll get a big performance hit if you're constantly flushing the output stream. Usually, deciding when to flush is best left to the implementation. There's no good reason for constantly flushing the output stream in this program. – Pete Becker May 28 '22 at 15:44

4 Answers4

1

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 the comment, using polymorphism (casting a pointer to a base class one) with no virtual function does not really makes sense.

Long story made short, if you have a class hierarchy with no virtual function, users are aware of it and never delete an object through a pointer to its base class. So 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.

John
  • 2,963
  • 11
  • 33
Serge Ballesta
  • 143,923
  • 11
  • 122
  • 252
  • 2
    Re: "the `Derived` constructor will not be called but it will be harmless" -- this is, in fact, the typical behavior, but it is not required. The behavior of the program is undefined. – Pete Becker May 28 '22 at 12:15
  • +1 for @PeteBecker. You could see that [In delete b], if the static type of the object to be deleted is different from its dynamic type, the static type shall be a base class of the dynamic type of the object to be deleted and the static type shall have a virtual destructor or the ***behavior is undefined***. – John May 28 '22 at 13:31
1

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.

Pete Becker
  • 74,985
  • 8
  • 76
  • 165
0

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{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
}
apple apple
  • 10,292
  • 2
  • 16
  • 36
0

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.

Goswin von Brederlow
  • 11,875
  • 2
  • 24
  • 42
  • Thanks for the reply. But there is something wrong. [The code snippet in your answer does not compile](https://godbolt.org/z/d68b399K4) since no function of the base class is marked with virtual. – John May 29 '22 at 02:14
  • As mentioned in the answer. If your classes are aggregate types you can do other casts but the idea remains the same. You have to delete the object as their original type. – Goswin von Brederlow May 29 '22 at 02:28
  • Without a virtual destructor your call to delete is arguably wrong.But ` Derived *t = dynamic_cast (p);` is also illegal for this case. **you can do other casts**. What casts? – John May 29 '22 at 02:31
  • static_cast or C-Style cast. They only work when you don't have virtual functions and some other conditions and you have to ensure you are casting to the right type yourself. – Goswin von Brederlow May 29 '22 at 02:57
  • That's not a good idea, indeed. – John May 29 '22 at 03:06
  • There is a reason why all of this is considered bad style. – Goswin von Brederlow May 29 '22 at 04:09