81

Everyone knows that the desructor of base class usually has to be virtual. But what is about the destructor of derived class? In C++11 we have keyword "override" and ability to use the default destructor explicitly.

struct Parent
{
  std::string a;
  virtual ~Parent()
  {
  }

};

struct Child: public Parent
{
  std::string b;
  ~Child() override = default;
};

Is it correct to use both keywords "override" and "=default" in the destructor of Child class? Will compiler generate correct virtual destructor in this case?

If yes, then can we think that it is good coding style, and we should always declare destructors of derived classes this way to ensure that base class destructors are virtual?

Sandro
  • 2,707
  • 2
  • 20
  • 30
  • 8
    Might as well do `static_assert(std::has_virtual_destructor::value, "contract violated");` – milleniumbug Dec 06 '16 at 16:10
  • 3
    Note that it isn't always a requirement that the base class destructor be virtual. So this is only (possibly) a good idea if that is a requirement. – juanchopanza Dec 06 '16 at 16:10
  • 3
    If it works, I like it, but milleniumbug's is better (much clearer intent). On the other hand, Stroustrup hates "coding standard" constructs that guard against common errors, and insists the compiler should generate suitable warnings, instead. – Kenny Ostrom Dec 06 '16 at 16:13
  • 3
    I think @milleniumbug's approach expresses the intent clearly. If I came across `~Child() override = default;` in a code base I might just remove the line. – juanchopanza Dec 06 '16 at 16:13
  • Well, using static_assert requires the same amount of programmers discipline as using virtual destructor, so I am not sure how one is better than another. – SergeyA Dec 06 '16 at 16:17
  • 3
    @milleniumbug I am confused. The *point* of `override`--the ***only*** point--is to force a compiler error if the parent's method isn't `virtual`. How is a `static_assert` an improvement? – Kyle Strand Dec 06 '16 at 16:22
  • @juanchopanza It depends how you are using it. It is a requirement in some cases (when you work with pointers to base class). – Sandro Dec 06 '16 at 16:24
  • 1
    @Sandro Only when you delete said pointers. Anyway, I think I misread the end of your question. – juanchopanza Dec 06 '16 at 16:26
  • 2
    The static_assert could be placed in a cpp-file, which makes it more convenient to add it without causing recompilation - and also hide it as an implementation detail. However, that would make it harder to find - and the downside with static_assert is that you have to repeat the name of the base-class; whereas override only has the minimal needed information. – Hans Olsson Dec 06 '16 at 16:27
  • 1
    @juanchopanza Preferring the `static_assert` seems like madness. If I came across the `static_assert` version, I'd replace it with `override`; that's the *point* of `override`, whereas `static_assert` is just a confusing and non-idiomatic way to try to write in verbose protections against shooting oneself in the foot. Every line of code is a liability, especially non-idiomatic and confusing ones. – Kyle Strand Dec 06 '16 at 16:27
  • @KyleStrand IYAM if you have to deal with someone randomly removing `virtual`ness of the destructor in your base class, you have bigger problems to deal with (see "contract violated" message). And if a third-party library is doing that, oh boy. – milleniumbug Dec 06 '16 at 16:27
  • 1
    @milleniumbug Right, both constructs are just ways to cause compile errors if something happens that *really really really* shouldn't happen. But that doesn't really explain why the less idiomatic one, which uses more complicated type-trait reflection, is preferable. – Kyle Strand Dec 06 '16 at 16:29
  • @KyleStrand the `static_assert` tells you exactly what is required and is completely idiomatic. If static_assert causes confusion, it may be worthwhile to take some time out to study some C++. The other option looks too much like redundant code. – juanchopanza Dec 06 '16 at 16:30
  • 6
    "it may be worthwhile to take some time out to study some C++" -- please see "blaming the programmer" at the end of [this post](https://eev.ee/blog/2016/12/01/lets-stop-copying-c/). Also, note that I didn't actually say that I don't understand the `static_assert`, just that it's *more* confusing than the `override` version. Which is true, because it's longer, more verbose, and uses a comparatively obscure feature of the standard library. – Kyle Strand Dec 06 '16 at 16:39
  • 1
    @KyleStrand The point is that checking for virtualness of the base destructor isn't the responsibility of the derived class. It's a responsibility of those who actually have base pointers to derived class objects and call `delete` on them. – milleniumbug Dec 06 '16 at 16:44
  • @KyleStrand To elaborate: if you have 30 derived classes, will you annotate all of them? No? Good. (side remark: `std::unique_ptr` is an excellent place to check for this, but I don't think such check is required by standard) – milleniumbug Dec 06 '16 at 16:53
  • 1
    @milleniumbug I disagree from a design perspective. Classes should be *designed* so that they can be `delete`d safely from a pointer anywhere in the class hierarchy; checking whether the class was correctly designed in this way should *not* be the concern of the programmer, because we should expect that classes are well designed. (This is the point of classes--to subdivide the problem space into *discrete* design problems.) – Kyle Strand Dec 06 '16 at 17:07
  • I do, however, think that the *compiler* should issue a warning when a base-class pointer is created from a derived-class but the base-class destructor is not `virtual`, because this is the first step toward the erroneous `delete` situation. Unfortunately, I know of no compiler that actually implements this warning option. – Kyle Strand Dec 06 '16 at 17:07
  • 2
    @milleniumbug And, again, your objection about annotating 30 derived classes applies to *both* constructs (`override` and `static_assert`) equally well, and doesn't at all indicate that one should be *preferred* over the other (though it's certainly an argument against doing such an annotation as a rule-of-thumb as the question asks). – Kyle Strand Dec 06 '16 at 17:15
  • 3
    _"Everyone knows that the desructor of base class usually has to be virtual."_ Ehm no not really – Lightness Races in Orbit Dec 06 '16 at 17:22
  • 2
    @LightnessRacesinOrbit Are you objecting to "everyone knows" or to "usually has to be virtual"? (Or both?) – Kyle Strand Dec 06 '16 at 17:59
  • 2
    @KyleStrand: The latter and, roughly as a consequence, the former. – Lightness Races in Orbit Dec 06 '16 at 18:07
  • @KyleStrand I am not sure how a static_assert that explicitly asks for the base class to have a virtual destructor is more confusing than declaring a destructor in your class when you *don't even need one*, just so you can check the same thing. And it will take about three seconds to explain the static_assert to any competent programmer that finds that festure obscure in any way. – juanchopanza Dec 06 '16 at 21:53

7 Answers7

33

Is it correct to use both keywords "override" and "=default" in the destructor of Child class? Will compiler generate correct virtual destructor in this case?

Yes, it is correct. On any sane compiler, if the code compiles without error, this destructor definition will be a no-op: its absence must not change the behavior of the code.

can we think that it is good coding style

It's a matter of preference. To me, it only makes sense if the base class type is templated: it will enforce a requirement on the base class to have a virtual destructor, then. Otherwise, when the base type is fixed, I'd consider such code to be noise. It's not as if the base class will magically change. But if you have deadheaded teammates that like to change things without checking the code that depends on what they may be possibly breaking, it's best to leave the destructor definition in - as an extra layer of protection.

Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313
25

According to the CppCoreGuidelines C.128 the destructor of the derived class should not be declared virtual or override.

If a base class destructor is declared virtual, one should avoid declaring derived class destructors virtual or override. Some code base and tools might insist on override for destructors, but that is not the recommendation of these guidelines.

UPDATE: To answer the question why we have a special case for destructors.

Method overriding is a language feature that allows a subclass or child class to provide a specific implementation of a method that is already provided by one of its superclasses or parent classes. The implementation in the subclass overrides (replaces) the implementation in the superclass by providing a method that has same name, same parameters or signature, and same return type as the method in the parent class.

In other words, when you call an overridden method only the last implementation of that method (in the class hierarchy) is actually executed while all the destructors (from the last child to the root parent) must be called to properly release all the resources owned by the object.

Thus we don't really replace (override) the destructor, we add additional one into the chain of object destructors.

UPDATE: This CppCoreGuidelines C.128 rule was changed (by 1448, 1446 issues) in an effort to simplify already exhaustive list of exceptions. So the general rule can be summarized as:

For class users, all virtual functions including destructors are equally polymorphic.

Marking destructors override on state-owning subclasses is textbook hygiene that you should all be doing by routine (ref.).

Alexandre A.
  • 1,619
  • 18
  • 29
  • 4
    Very interesting! But they don't explain why do we have a special case for destructors, all other functions should be override or final... – Sandro Feb 14 '19 at 22:39
  • 2
    C.128 did not say that, it didn't say anything about destructor at all – reddy Aug 11 '20 at 23:32
  • 4
    @reddy Ok, it looks like they changed their mind about it. You can find more info here: https://github.com/isocpp/CppCoreGuidelines/issues/1446. I'll probably update my answer later. – Alexandre A. Aug 16 '20 at 09:27
  • Most valuable answer so far. Only problem is that all the **UPDATE** sections have to be read to understand this. If you find the time, it would make me happy to see a rework in that concern. It may be enough to give a hint to the final version right at the beginning. – Wolf Sep 13 '22 at 12:45
24

override is nothing more than a safety net. Destructor of the child class will always be virtual if base class destructor is virtual, no matter how it is declared - or not declared at all (i.e. using implicitly declared one).

SergeyA
  • 61,605
  • 5
  • 78
  • 137
9

There is (at least) one reason for using override here -- you ensure that the base class's destructor is always virtual. It will be a compilation error if the derived class's destructor believes it is overriding something, but there is nothing to override. It also gives you a convenient place to leave generated documentation, if you're doing that.

On the other hand, I can think of two reasons not to do this:

  • It's a little weird and backwards for the derived class to enforce behavior from the base class.
  • If you define a destuctor in the header (or if you make it inline), you do introduce the possibility for odd compilation errors. Let's say your class looks like this:

    struct derived {
        struct impl;
        std::unique_ptr<derived::impl> m_impl;
        ~derived() override = default;
    };
    

    You will likely get a compiler error because the destructor (which is inline with the class here) will be looking for the destructor for the incomplete class, derived::impl.

    This is my round-about way of saying that every line of code can become a liability, and perhaps it's best to just skip something if it functionally does nothing. If you really really need to enforce a virtual destructor in the base class from the parent class, someone suggested using static_assert in concert with std::has_virtual_destructor, which will produce more consistent results, IMHO.

cyberbisson
  • 382
  • 2
  • 9
  • 2
    The solution to this problem (forward declared classes) is to implement the destructor in your compile unit. For ex, in your .cpp file, `derived::~derived() = default;` You can use `= default` in your compile unit. – scx Jan 28 '20 at 20:06
8

I think "override" is kind of misleading on destructor. When you override virtual function, you replace it. The destructors are chained, so you can't override destructor literally

hakun bahun
  • 79
  • 1
  • 1
  • 3
    I wouldn't say that. Overridden functions should perform the same semantic task (though derived versions should be more specialised). Also the replacement isn't total. It just means that the call resolves to the type of the object by default. You can still explicitly call base class functions using the scope operator. See here https://stackoverflow.com/questions/38010286/how-can-i-call-virtual-function-definition-of-base-class-that-have-definitions-i – Paul Floyd Sep 12 '17 at 20:18
  • 3
    Override specifies a required property of the base class implementation: that function must have been virtual. It's actually perfectly reasonable to say: `~Derived() override = default;` Using override is the only way to guarantee the base class was properly defined. This is particularly important when declaring templates which derive from a template parameter, and need to guarantee the base properly declares its destructor. – Speed8ump Oct 30 '18 at 00:36
  • @Speed8ump It is not about the functions. It doesn't make much sens to override a constructor. There is an interesting discussion about it here https://github.com/isocpp/CppCoreGuidelines/issues/721 – Alexandre A. Feb 14 '19 at 15:20
  • 3
    In the english semantic sense you might be right that it doesn't make sense to override ('interrupt the action of, in order to take manual control') a destructor. In the C++ semantic sense ('require base class method be virtual') it makes sense to mark a destructor as override and has specific utility, as I mentioned – Speed8ump Feb 14 '19 at 17:08
4

The CPP Reference says that override makes sure that the function is virtual and that it indeed overrides a virtual function. So the override keyword would make sure that the destructor is virtual.

If you specify override but not = default, then you will get a linker error.

You do not need to do anything. Leaving the Child dtor undefined works just fine:

#include <iostream>

struct Notify {
    ~Notify() { std::cout << "dtor" << std::endl; }
};

struct Parent {
    std::string a;
    virtual ~Parent() {}
};

struct Child : public Parent {
    std::string b;
    Notify n;
};

int main(int argc, char **argv) {
    Parent *p = new Child();
    delete p;
}

That will output dtor. If you remove the virtual at Parent::~Parent, though, it will not output anything because that is undefined behavior, as pointed out in the comments.

Good style would be to not mention Child::~Child at all. If you cannot trust that the base class declared it virtual, then your suggestion with override and = default will work; I would hope that there are better ways to ensure that instead of littering your code with those destructor declarations.

Martin Ueding
  • 8,245
  • 6
  • 46
  • 92
  • 5
    If you remove the `virtual` at `Parent::~Parent`, you will have *undefined behavior*. It might output nothing. It might display a fatal error dialog. Or overwrite the data file you're working with. – Ben Voigt Dec 06 '16 at 16:36
  • 1
    Interesting! I have assumed that it would just call the base dtor on the subclass which would result in a well defined but likely undesired cleanup of the base members but not of any derived members. I have updated my answer to incorporate your comment. – Martin Ueding Dec 06 '16 at 17:47
0

Though destructors are not inherited there is clear written in the Standard that virtual destructors of derived classes override destructors of base classes.

From the C++ Standard (10.3 Virtual functions)

6 Even though destructors are not inherited, a destructor in a derived class overrides a base class destructor declared virtual; see 12.4 and 12.5.

On the other hand there is also written (9.2 Class member)

8 A virt-specifier-seq shall contain at most one of each virt-specifier. A virt-specifier-seq shall appear only in the declaration of a virtual member function (10.3).

Though destructors are called like special member functions nevertheless they are also member functions.

I am sure the C++ Standard should be edited such a way that it was unambiguous whether a destructor may have virt-specifier override. At present it is not clear.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • "compilers I tested issue an error..." that is *exactly the point* of the `override`. You've written your base class with a non-`virtual` destructor; the compilers are behaving correctly. – Kyle Strand Dec 06 '16 at 17:10
  • @KyleStrand I removed the note about compilers because I could use an old compiler. – Vlad from Moscow Dec 06 '16 at 17:18
  • I suspect you are missing my point. Could you copy and paste your code into an online compiler (e.g. [Coliru](http://coliru.stacked-crooked.com/))? – Kyle Strand Dec 06 '16 at 17:58