[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