[PATCH] Add a KeepVirtual option to misc-use-override
Alexander Kornienko
alexfh at google.com
Fri May 8 04:06:58 PDT 2015
> 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.
================
Comment at: clang-tidy/misc/UseOverrideCheck.cpp:92
@@ -87,3 +91,3 @@
if (OnlyVirtualSpecified) {
Message =
"prefer using 'override' or (rarely) 'final' instead of 'virtual'";
----------------
In case `KeepVirtual` is true, the message will be inconsistent with the suggested fix. Namely, the "instead of 'virtual'" part.
================
Comment at: clang-tidy/misc/UseOverrideCheck.cpp:104
@@ -98,1 +103,3 @@
+ if (!Redundant.size())
+ return;
----------------
nit: s/!Redundant.size()/Redundant.empty()/
================
Comment at: test/clang-tidy/misc-use-override-keep-virtual.cpp:1
@@ +1,2 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s misc-use-override %t -config="{CheckOptions: [{key: misc-use-override.KeepVirtual, value: 1}]}" -- -std=c++11
+// REQUIRES: shell
----------------
This file should be `svn/git cp`ed from the original test. Or is just Phabricator failing to recognize this and display the diff appropriately?
http://reviews.llvm.org/D9285
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the cfe-commits
mailing list