'final' class specifier and 'final' method specifier

# Fujii Hironori (3 days ago)

devs,

'final' keyword can be applied to both a class and a method (non-static member function). I don't know correct terminology for them. I call them 'final' class specifier and 'final' method specifier.

I'm now going to update WebKit Code Style Guidelines for using 'final' class specifier for all classes which has no derived classes in Bug 192844. bugs.webkit.org/show_bug.cgi?id=192844

I believe this is not controversial. This is not a topic I'd like to discuss today.

I'd like to discuss which 'override' or 'final' should be used for methods.

Currently, WebKit is using both inconsistently. For example, CSSFontFaceSet class is marked 'final' and its methods are marked 'final', while CSSFontFaceSource class is marked 'final' and its methods are marked 'override'. trac.webkit.org/browser/webkit/trunk/Source/WebCore/css/CSSFontFaceSet.h?rev=239365#L93, trac.webkit.org/browser/webkit/trunk/Source/WebCore/css/CSSFontFaceSource.h?rev=239365#L83

According to WebKit Code Style Guidelines, 'final' method specifier is preferable to 'override' if possible. There is a previous discussion thread. webkit.org/code-style-guidelines/#override-methods, lists.webkit.org/pipermail/webkit-dev/2016-March/028022.html

I'd like to change this because 'final' doesn't necessarily imply 'override'. See the following stackoverflow: stackoverflow.com/questions/29412412/does-final-imply-override

IIUC, 'final' method specifier can be useful for such like the following case:

class B { public: virtual foo(); virtual bar(); }; class B1 : public B { foo() final; }; class D final : B1 { bar() override; };

The 'final' method specifier indicates D can't override the method foo. Thus, using 'final' method specifier implies other virtual functions can be overridden. I'm not sure my understanding right. Please correct me.

Contact us to advertise here
# Darin Adler (3 days ago)

On Dec 19, 2018, at 1:52 AM, Fujii Hironori <fujii.hironori at gmail.com> wrote:

I'd like to change this because 'final' doesn't necessarily imply 'override'. See the following stackoverflow: stackoverflow.com/questions/29412412/does-final-imply-override

I’d be happy to require both final and override if there was any benefit to doing so.

I read it fairly carefully, and I don’t think it says anything meaningful for us. Maybe we can find a real example in WebKit code where there’s a “final” and if we added “override”, it would have caught a mistake that “final” won’t catch?

— Darin

# Konstantin Tokarev (3 days ago)

19.12.2018, 12:53, "Fujii Hironori" <fujii.hironori at gmail.com>:

I'd like to change this because 'final' doesn't necessarily imply 'override'. See the following stackoverflow: stackoverflow.com/questions/29412412/does-final-imply-override

It does imply override, unless it is used in a declaration of new virtual method (which has no practical meaning fwiw)

IIUC, 'final' method specifier can be useful for such like the following case:

class B { public: virtual foo(); virtual bar(); }; class B1 : public B { foo() final; }; class D final : B1 { bar() override; };

The 'final' method specifier indicates D can't override the method foo. Thus, using 'final' method specifier implies other virtual functions can be overridden. I'm not sure my understanding right. Please correct me.

final method specifier on foo doesn't affect other methods anyhow, it just forbids overriding of foo

# Konstantin Tokarev (3 days ago)

19.12.2018, 21:02, "Darin Adler" <darin at apple.com>:

On Dec 19, 2018, at 1:52 AM, Fujii Hironori <fujii.hironori at gmail.com> wrote:

I'd like to change this because 'final' doesn't necessarily imply 'override'. See the following stackoverflow: stackoverflow.com/questions/29412412/does-final-imply-override

I’d be happy to require both final and override if there was any benefit to doing so.

I read it fairly carefully, and I don’t think it says anything meaningful for us. Maybe we can find a real example in WebKit code where there’s a “final” and if we added “override”, it would have caught a mistake that “final” won’t catch?

Adding override to method which already has final specifier doesn't affect anything, because both final and override may ony be used on virtual methods

# Michael Catanzaro (3 days ago)

On Wed, Dec 19, 2018 at 1:58 PM, Konstantin Tokarev <annulen at yandex.ru>

wrote:

Adding override to method which already has final specifier doesn't affect anything, because both final and override may ony be used on virtual methods

FWIW I prefer override because it's much more clear what that keyword is used for.

Michael

# Konstantin Tokarev (2 days ago)

19.12.2018, 23:27, "Michael Catanzaro" <mcatanzaro at igalia.com>:

On Wed, Dec 19, 2018 at 1:58 PM, Konstantin Tokarev <annulen at yandex.ru> wrote:

Adding override to method which already has final specifier doesn't affect anything, because both final and override may ony be used on virtual methods

FWIW I prefer override because it's much more clear what that keyword is used for.

If class itself has "final" specifier, "override" on methods works in the same way as "final", and I agree that it conveys intention more clear. However, Darin in [1] suggested that we use "final" aggressively to avoid accidentally losing compiler optimization (i.e. devirtualization of call)

[1] lists.webkit.org/pipermail/webkit-dev/2016-March/028022.html

# Darin Adler (2 days ago)

Let’s be clear about what we are discussing.

The choice is not be between “final” and “override”.

The choice is between “final override”, “override final”, and “final” for functions which are both overrides and final.

— Darin

Sent from my iPhone

# Ross.Kirsling at sony.com (2 days ago)

In that case, I'll point out that C++ Core Guidelines has a rule "Virtual functions should specify exactly one of virtual, override, or final". (isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-override)

Their tl;dr: " • virtual means exactly and only “this is a new virtual function.” • override means exactly and only “this is a non-final overrider.” • final means exactly and only “this is a final overrider.” "

FWIW, they also have a rule "Use final sparingly" with the note that "Claims of performance improvements from final should be substantiated." (isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-final)

Ross

On 12/19/18, 1:54 PM, "webkit-dev on behalf of Darin Adler" <webkit-dev-bounces at lists.webkit.org on behalf of darin at apple.com> wrote:

Let’s be clear about what we are discussing.

The choice is not be between “final” and “override”.

The choice is between “final override”, “override final”, and “final” for functions which are both overrides and final.

— Darin

Sent from my iPhone

> On Dec 19, 2018, at 12:27 PM, Michael Catanzaro <mcatanzaro at igalia.com> wrote:
> 
>> On Wed, Dec 19, 2018 at 1:58 PM, Konstantin Tokarev <annulen at yandex.ru> wrote:
>> Adding override to method which already has final specifier doesn't affect anything,
>> because both final and override may ony be used on virtual methods
> 
> FWIW I prefer override because it's much more clear what that keyword is used for.
> 
> Michael
_______________________________________________
webkit-dev mailing list
webkit-dev at lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit
# Fujii Hironori (2 days ago)

Thank you very much for the feedbacks.

On Thu, Dec 20, 2018 at 4:52 AM Konstantin Tokarev <annulen at yandex.ru>

wrote:

19.12.2018, 12:53, "Fujii Hironori" <fujii.hironori at gmail.com>:

I'd like to change this because 'final' doesn't necessarily imply 'override'. See the following stackoverflow: stackoverflow.com/questions/29412412/does-final-imply-override

It does imply override, unless it is used in a declaration of new virtual method (which has no practical meaning fwiw)

You are right. C++ allows using 'final' with 'virtual' without overriding. Even though I don't know practical use-cases for it, C++ allows it because 'final' doesn't mean overriding. And, this is the only reason 'final' doesn't necessarily imply override. This is a kind of chicken egg problem. I don't know which is true:

  1. C++ allows it because 'final' doesn't mean overriding.
  2. 'final' doesn't necessarily imply override because C++ allows it

On Thu, Dec 20, 2018 at 6:28 AM Konstantin Tokarev <annulen at yandex.ru>

wrote:

19.12.2018, 23:27, "Michael Catanzaro" <mcatanzaro at igalia.com>:

On Wed, Dec 19, 2018 at 1:58 PM, Konstantin Tokarev <annulen at yandex.ru> wrote:

Adding override to method which already has final specifier doesn't affect anything, because both final and override may ony be used on virtual methods

FWIW I prefer override because it's much more clear what that keyword is used for.

If class itself has "final" specifier, "override" on methods works in the same way as "final", and I agree that it conveys intention more clear.

I think so, especially after I will update the code style guidelines in Bug 192844.

Bug 192844 – Update code style guidelines for using 'final' specifier for all classes which has no derived classes bugs.webkit.org/show_bug.cgi?id=192844

However, Darin in [1] suggested that we use "final" aggressively to avoid accidentally losing compiler optimization (i.e. devirtualization of call)

[1] lists.webkit.org/pipermail/webkit-dev/2016-March/028022.html

I think this won't be a problem after all classes which has no derived classes are capped with 'final'.

On Thu, Dec 20, 2018 at 7:42 AM <Ross.Kirsling at sony.com> wrote:

In that case, I'll point out that C++ Core Guidelines has a rule "Virtual functions should specify exactly one of virtual, override, or final". (isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-override)

Their tl;dr: " • virtual means exactly and only “this is a new virtual function.” • override means exactly and only “this is a non-final overrider.” • final means exactly and only “this is a final overrider.” "

FWIW, they also have a rule "Use final sparingly" with the note that "Claims of performance improvements from final should be substantiated." (isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-final)

C.128 is a same rule with the current WebKit coding style guidelines. But, I think C.128 makes sense with C.139. C.139 is against to Bug 192844. After Bug 192844 update, we will have a lot of 'final' classes, not sparignly.

# Konstantin Tokarev (2 days ago)

20.12.2018, 09:17, "Fujii Hironori" <fujii.hironori at gmail.com>:

Thank you very much for the feedbacks.

On Thu, Dec 20, 2018 at 4:52 AM Konstantin Tokarev <annulen at yandex.ru> wrote:

19.12.2018, 12:53, "Fujii Hironori" <fujii.hironori at gmail.com>:

I'd like to change this because 'final' doesn't necessarily imply 'override'. See the following stackoverflow: stackoverflow.com/questions/29412412/does-final-imply-override

It does imply override, unless it is used in a declaration of new virtual method (which has no practical meaning fwiw)

You are right. C++ allows using 'final' with 'virtual' without overriding. Even though I don't know practical use-cases for it, C++ allows it because 'final' doesn't mean overriding. And, this is the only reason 'final' doesn't necessarily imply override. This is a kind of chicken egg problem. I don't know which is true:

  1. C++ allows it because 'final' doesn't mean overriding.
  2. 'final' doesn't necessarily imply override because C++ allows it

I see no problem here, because our code style checker forbids using 'final' and 'virtual' in the same declaration, so it's only allowed in overriding context.

On Thu, Dec 20, 2018 at 6:28 AM Konstantin Tokarev <annulen at yandex.ru> wrote:

19.12.2018, 23:27, "Michael Catanzaro" <mcatanzaro at igalia.com>:

On Wed, Dec 19, 2018 at 1:58 PM, Konstantin Tokarev <annulen at yandex.ru> wrote:

Adding override to method which already has final specifier doesn't affect anything, because both final and override may ony be used on virtual methods

FWIW I prefer override because it's much more clear what that keyword is used for.

If class itself has "final" specifier, "override" on methods works in the same way as "final", and I agree that it conveys intention more clear.

I think so, especially after I will update the code style guidelines in Bug 192844.

Bug 192844 – Update code style guidelines for using 'final' specifier for all classes which has no derived classes bugs.webkit.org/show_bug.cgi?id=192844

However, Darin in [1] suggested that we use "final" aggressively to avoid accidentally losing compiler optimization (i.e. devirtualization of call)

[1] lists.webkit.org/pipermail/webkit-dev/2016-March/028022.html

I think this won't be a problem after all classes which has no derived classes are capped with 'final'.

However, it might be a problem when leaf class with 'final' becomes non-final.

On Thu, Dec 20, 2018 at 7:42 AM <Ross.Kirsling at sony.com> wrote:

In that case, I'll point out that C++ Core Guidelines has a rule "Virtual functions should specify exactly one of virtual, override, or final". (isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-override)

Their tl;dr: " • virtual means exactly and only “this is a new virtual function.” • override means exactly and only “this is a non-final overrider.” • final means exactly and only “this is a final overrider.” "

FWIW, they also have a rule "Use final sparingly" with the note that "Claims of performance improvements from final should be substantiated." (isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-final)

C.128 is a same rule with the current WebKit coding style guidelines. But, I think C.128 makes sense with C.139. C.139 is against to Bug 192844. After Bug 192844 update, we will have a lot of 'final' classes, not sparignly.

Do you have an idea how to automate this? Otherwise we'll never reach the state where all leaf classes are final, because doing it manually will take lots of time, and I see no way to enforce the rule in new code

# Fujii Hironori (a day ago)

On Fri, Dec 21, 2018 at 5:02 AM Konstantin Tokarev <annulen at yandex.ru>

wrote:

>

On Thu, Dec 20, 2018 at 7:42 AM <Ross.Kirsling at sony.com> wrote:

In that case, I'll point out that C++ Core Guidelines has a rule "Virtual functions should specify exactly one of virtual, override, or final". ( isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-override)

Their tl;dr: " • virtual means exactly and only “this is a new virtual function.” • override means exactly and only “this is a non-final overrider.” • final means exactly and only “this is a final overrider.” "

FWIW, they also have a rule "Use final sparingly" with the note that "Claims of performance improvements from final should be substantiated." (isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-final)

C.128 is a same rule with the current WebKit coding style guidelines. But, I think C.128 makes sense with C.139. C.139 is against to Bug 192844. After Bug 192844 update, we will have a lot of 'final' classes, not sparignly.

Do you have an idea how to automate this? Otherwise we'll never reach the state where all leaf classes are final, because doing it manually will take lots of time, and I see no way to enforce the rule in new code

I don't have such intelligent plan. I don't like the divergence between WebKit Code Style Guidelines and WebKit source code. I've often got review feedbacks that I should modernize the code around my change. I'm going to change them manually, and all patches will be checked by reviewer's eyes. There is a previous such effort.

Bug 159802 – Add final keyword to WebCore/svg classes bugs.webkit.org/show_bug.cgi?id=159802

It seems that I shouldn't change the current 'final' method specifier rule of WebKit. Then, I think the sentences should be revised such like C.128.

Want more features?

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