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

Reply via email to