[webkit-dev] 'final' class specifier and 'final' method specifier

[Konstantin Tokarev]

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:
>>> https://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
>> https://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] https://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".
>> (http://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."
>> (http://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

> ,
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev


-- 
Regards,
Konstantin