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>`. -Yusuke > On Oct 8, 2019, at 19:15, Ryosuke Niwa <rn...@webkit.org> wrote: > > On Tue, Oct 8, 2019 at 4:24 PM Yury Semikhatsky <yu...@chromium.org > <mailto:yu...@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 > <https://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 > > _______________________________________________ > webkit-dev mailing list > webkit-dev@lists.webkit.org > https://lists.webkit.org/mailman/listinfo/webkit-dev
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
