Default to making classes either `final` or give them a virtual destructor?
Asked Answered
A

3

12

Classes with non-virtual destructors are a source for bugs if they are used as a base class (if a pointer or reference to the base class is used to refer to an instance of a child class).

With the C++11 addition of a final class, I am wondering if it makes sense to set down the following rule:

Every class must fulfil one of these two properties:

  1. be marked final (if it is not (yet) intended to be inherited from)
  2. have a virtual destructor (if it is (or is intended to) be inherited from)

Probably there are cases were neither of these two options makes sense, but I guess they could be treated as exceptions that should be carefully documented.

Artiodactyl answered 6/6, 2015 at 17:27 Comment(2)
Not every inheritance hierarchy needs virtualness.Tremolo
True. Type trait classes often aren't even instantiated, so there's no need to destruct them either. So a third allowable case would be "no constructors".Madaras
O
9

The probably most common actual issue attributed to the lack of a virtual destructor is deletion of an object through a pointer to a base class:

struct Base { ~Base(); };
struct Derived : Base { ~Derived(); };

Base* b = new Derived();
delete b; // Undefined Behaviour

A virtual destructor also affects the selection of a deallocation function. The existence of a vtable also influences type_id and dynamic_cast.

If your class isn't use in those ways, there's no need for a virtual destructor. Note that this usage is not a property of a type, neither of type Base nor of type Derived. Inheritance makes such an error possible, while only using an implicit conversion. (With explicit conversions such as reinterpret_cast, similar problems are possible without inheritance.)

By using smart pointers, you can prevent this particular problem in many cases: unique_ptr-like types can restrict conversions to a base class for base classes with a virtual destructor (*). shared_ptr-like types can store a deleter suitable for deleting a shared_ptr<A> that points to a B even without virtual destructors.

(*) Although the current specification of std::unique_ptr doesn't contain such a check for the converting constructor template, it was restrained in an earlier draft, see LWG 854. Proposal N3974 introduces the checked_delete deleter, which also requires a virtual dtor for derived-to-base conversions. Basically, the idea is that you prevent conversions such as:

unique_checked_ptr<Base> p(new Derived); // error

unique_checked_ptr<Derived> d(new Derived); // fine
unique_checked_ptr<Base> b( std::move(d) ); // error

As N3974 suggests, this is a simple library extension; you can write your own version of checked_delete and combine it with std::unique_ptr.


Both suggestions in the OP can have performance drawbacks:

  1. Mark a class as final

This prevents the Empty Base Optimization. If you have an empty class, its size must still be >= 1 byte. As a data member, it therefore occupies space. However, as a base class, it is allowed not to occupy a distinct region of memory of objects of the derived type. This is used e.g. to store allocators in StdLib containers. C++20 has mitigated this with the introduction of [[no_unique_address]].

  1. Have a virtual destructor

If the class doesn't already have a vtable, this introduces a vtable per class plus a vptr per object (if the compiler cannot eliminate it entirely). Destruction of objects can become more expensive, which can have an impact e.g. because it's no longer trivially destructible. Additionally, this prevents certain operations and restricts what can be done with that type: The lifetime of an object and its properties are linked to certain properties of the type such as trivially destructible.


final prevents extensions of a class via inheritance. While inheritance is typically one of the worst ways to extend an existing type (compared to free functions and aggregation), there are cases where inheritance is the most adequate solution. final restricts what can be done with the type; there should be a very compelling and fundamental reason why I should do that. One cannot typically imagine the ways others want to use your type.

T.C. points out an example from the StdLib: deriving from std::true_type and similarly, deriving from std::integral_constant (e.g. the placeholders). In metaprogramming, we're typically not concerned with polymorphism and dynamic storage duration. Public inheritance often just the simplest way to implement metafunctions. I do not know of any case where objects of metafunction type are allocated dynamically. If those objects are created at all, it's typically for tag dispatching, where you'd use temporaries.


As an alternative, I'd suggest using a static analyser tool. Whenever you derive publicly from a class without a virtual destructor, you could raise a warning of some sort. Note that there are various cases where you'd still want to derive publicly from some base class without a virtual destructor; e.g. DRY or simply separation of concerns. In those cases, the static analyser can typically be adjusted via comments or pragmas to ignore this occurrence of deriving from a class w/o virtual dtor. Of course, there need to be exceptions for external libraries such as the C++ Standard Library.

Even better, but more complicated is analysing when an object of class A w/o virtual dtor is deleted, where class B inherits from class A (the actual source of UB). This check is probably not reliable, though: The deletion can happen in a Translation Unit different to the TU where B is defined (to derive from A). They can even be in separate libraries.

Orland answered 6/6, 2015 at 17:37 Comment(9)
I would hope that such a static analyzer can be taught to ignore deriving from std::true_type and std::false_type, at the very least.Bracer
@Bracer For the most part, the problem of derivation is restricted to new/delete (even though .~T() can occur on non-free-store data, if you are using a destructor manually, presumably you know what you are doing). Such types could be marked as "unsafe for dynamic allocation", and a warning issued when you (non-placement) new X?Seventh
I pretty much downvoted you for talking about performance first like it's an important issue here.Falmouth
@Falmouth It's the one issue that can be measured. OP is talking about a general rule. Last time I looked, libstdc++ didn't even support final allocators for example, because they always try to use EBO. -- The other drawbacks are essentially downsides that can be overcome by more effort (e.g. private inheritance + using-declarations, aggregation, ...). Trading "more effort" vs "safer code" is something that's often a company/team/product decision. After all, both options in the OP rule out one kind of error.Orland
Being able to measure it is useless since it's a non-issue.Falmouth
@Falmouth In that case, I probably don't understand why you mean with "non-issue". Do you think it cannot have a significant impact on performance in a real program?Orland
Could you elaborate a bit more on your remark "By using smart pointers, you can prevent this particular problem in many cases: unique_ptr-like types can restrict conversions to a base class for base classes with a virtual destructor". What does this mean in practice? If I just drop a Derived pointer into a std::unique_ptr<Base>, it does not seem to change anything. Is this about proposed future changes?Artiodactyl
@Artiodactyl The current spec for std::unique_ptr does not require (nor allow) such a check. However, you can easily write your own checked_delete deleter and combine it with std::unique_ptr. Either follow N3974 or check for something like is_convertible<U*, T*>::value && (is_same<remove_cv_t<T>,remove_cv_t<U>>::value || not is_class<T>::value || has_virtual_destructor<T>::value)Orland
Actually, that should be is_convertible<U*, T*>::value && (not is_strict_base_of<T, U>::value || has_virtual_destructor<T>::value) -- that is, only in the case of derived->base we need to enforce a virtual dtor. Where is_same<remove_cv_t<T>, remove_cv_t<U>>::value implies false == is_strict_base_of<T,U>::value.Orland
M
5

The question that I usually ask myself, is whether an instance of the class may be deleted via its interface. If this is the case, I make it public and virtual. If this is not the case, I make it protected. A class only needs a virtual destructor if the destructor will be invoked through its interface polymorphically.

Maxima answered 6/6, 2015 at 17:41 Comment(4)
I now found this article by Herb Sutter, which goes in more detail. Your answer is basically a summary of it (or rather its second part): gotw.ca/publications/mill18.htm.Artiodactyl
Reading Herb Sutter (long ago) has undoubtedly influenced my thinking yes. Now that thinking is a part of me.Maxima
I now feel that this answer is the best advice, and indicates that my original question was not entirely sensible. I was considering making this the accepted answer, however @Orland answers the original question more directly, so I am not sure if that would be the correct thing to do.Artiodactyl
I find @Orland 's answer worthwhile (and informative enough) to leave as the accepted answer (no objection). You could edit your post to indicate that the question is not entirely sensible, with reference to my answer if you want to.Maxima
F
0

Well, to be strictly clear, it's only if the pointer is deleted or the object is destructed (through the base class pointer only) that the UB is invoked.

There could be some exceptions for cases where the API user cannot delete the object, but other than that, it's generally a wise rule to follow.

Falmouth answered 6/6, 2015 at 17:32 Comment(5)
Isn't every object deleted/destructed at some point, latest at program termination (unless you do no cleanup)? So I don't know what you mean by your first comment.Artiodactyl
@Simon: UB occurs if an object that isn't trivially destructible is deleted using a pointer to a base class, and the base type's destructor is not virtual. You say every objects is destroyed, which is true unless it's leaked, but that doesn't mean it's deleted using a pointer to the base class. As long as you destroy it the right way there's no problem. Note also that even if all objects are destroyed, they aren't necessarily destroyed using delete.Honky
@SteveJessop Trivial destructibility doesn't matter; it is UB if the static type differs from the dynamic type and doesn't have a virtual destructor, regardless of whether the destructor is trivial.Bracer
@T.C.: I don't think that's correct, I think it says that if the dynamic type's destructor is trivial then you're OK. But I can't check it right now so you may be right.Honky
@SteveJessop #29842345Bracer

© 2022 - 2024 — McMap. All rights reserved.