virtual destructor annotations in subclasses

# Yury Semikhatsky (2 days ago)

This question came up in a code review where I annotated subclass's destructor with 'override':

class SuperClass { public: virtual ~SuperClass() {} };

class Subclass : public SuperClass { public: ~Subclass() override; };

The style guide recommends webkit.org/code-style-guidelines/#overriding-virtual-methods

annotating overriden methods with either 'override' or 'final'. At the same time with a very few exceptions, the vast majority of the subclasses in WebKit annotate their destructors with 'virtual' keyword. It might be because some of the code predates c++11 and virtual was the default choice back then. In any case it prevents the compiler from generating errors when super destructor is accidentally not made virtual.

So my question is if there is a reason to exempt destructors from the above rule and mark them virtual in subclasses or they should be annotated with override/final similar to other virtual methods?

In the latter case we could update existing code once by automatically generating fix with clang-tidy.

Contact us to advertise here
# Ryosuke Niwa (a day ago)

On Tue, Oct 8, 2019 at 4:24 PM Yury Semikhatsky <yurys at chromium.org> wrote:

This question came up in a code review where I annotated subclass's destructor with 'override':

class SuperClass { public: virtual ~SuperClass() {} };

class Subclass : public SuperClass { public: ~Subclass() override; };

The style guide recommends webkit.org/code-style-guidelines/#overriding-virtual-methods annotating overriden methods with either 'override' or 'final'. At the same time with a very few exceptions, the vast majority of the subclasses in WebKit annotate their destructors with 'virtual' keyword. It might be because some of the code predates c++11 and virtual was the default choice back then. In any case it prevents the compiler from generating errors when super destructor is accidentally not made virtual.

So my question is if there is a reason to exempt destructors from the above rule and mark them virtual in subclasses or they should be annotated with override/final similar to other virtual methods?

In the latter case we could update existing code once by automatically generating fix with clang-tidy.

I don't have an opinion either way about the actual style but I'm against adding override everywhere en masse the same way we don't land a bunch of whitespace or other style tidy ups. If we were to decide that we want to add override, that should happen over time as people touch different classes and files.

  • R. Niwa
# Yusuke Suzuki (a day ago)

While I don’t have any opinion about adding override on methods, I would like to see more final annotation in derived classes’ header when the classes are final and inherits something. In JSC, we leverage this final information in jsCast<> and jsDynamicCast to optimize the generated code[1]. So, annotating final on JS classes actually changes the generated code and makes it optimized.

[1]: If the given JS class is annotated as final, we can skip traversing subclasses when doing jsDynamicCast<FinalAnnotatedClass>. We can use this information by std::is_final<T>.

Want more features?

Request early access to our private beta of readable email premium.