| alexfh |
Currently, clang-tidy will complain about the virtual keyword for
override and final methods. Some coding styles such as Mozilla doesn't
prohibit the usage of the virtual keyword.
This patch adds a KeepVirtual option for the misc-use-override check to
instruct it to ignore existing virtual keywords.
Some coding styles such as Mozilla doesn't prohibit the usage of the virtual keyword.
Apparently, Mozilla style guide even mandates the use of both virtual and override on overridden methods:
Method declarations that override virtual methods from a base class should be annotated with both "virtual" and "override".
I find this rule weird and if I worked with Mozilla code, I'd better spend time to fix the style guide. But I don't so we can add this flag. See a few comments inline.
| clang-tidy/misc/UseOverrideCheck.cpp | ||
|---|---|---|
| 92 | In case KeepVirtual is true, the message will be inconsistent with the suggested fix. Namely, the "instead of 'virtual'" part. | |
| 104 | nit: s/!Redundant.size()/Redundant.empty()/ | |
| test/clang-tidy/misc-use-override-keep-virtual.cpp | ||
| 1 | This file should be svn/git cped from the original test. Or is just Phabricator failing to recognize this and display the diff appropriately? | |
Sorry, I kind of dropped the ball here!
At Mozilla we ended up deciding on allowing only a maximum of one of virtual, final or override per function declaration, similar to the Google coding style, so we won't need the KeepVirtual option. Should I still add it nevertheless?
No, I don't think the option (and the rule it backs up) makes sense. I'm glad you got rid of the relevant style rule ;)