Hi 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.

http://reviews.llvm.org/D9285

Files:
  clang-tidy/misc/UseOverrideCheck.cpp
  clang-tidy/misc/UseOverrideCheck.h
  test/clang-tidy/misc-use-override-keep-virtual.cpp

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
Index: clang-tidy/misc/UseOverrideCheck.cpp
===================================================================
--- clang-tidy/misc/UseOverrideCheck.cpp
+++ clang-tidy/misc/UseOverrideCheck.cpp
@@ -18,6 +18,10 @@
 namespace tidy {
 namespace misc {
 
+void UseOverrideCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "KeepVirtual", KeepVirtual);
+}
+
 void UseOverrideCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(methodDecl(isOverride()).bind("method"), this);
 }
@@ -91,11 +95,15 @@
     Message = "annotate this function with 'override' or (rarely) 'final'";
   } else {
     StringRef Redundant =
-        HasVirtual ? (HasOverride && HasFinal ? "'virtual' and 'override' are"
-                                              : "'virtual' is")
-                   : "'override' is";
+        (!KeepVirtual && HasVirtual)
+            ? (HasOverride && HasFinal ? "'virtual' and 'override' are"
+                                       : "'virtual' is")
+            : (HasOverride && HasFinal ? "'override' is" : "");
     StringRef Correct = HasFinal ? "'final'" : "'override'";
 
+    if (!Redundant.size())
+      return;
+
     Message =
         (llvm::Twine(Redundant) +
          " redundant since the function is already declared " + Correct).str();
@@ -170,7 +178,7 @@
         CharSourceRange::getTokenRange(OverrideLoc, OverrideLoc));
   }
 
-  if (HasVirtual) {
+  if (!KeepVirtual && HasVirtual) {
     for (Token Tok : Tokens) {
       if (Tok.is(tok::kw_virtual)) {
         Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
Index: clang-tidy/misc/UseOverrideCheck.h
===================================================================
--- clang-tidy/misc/UseOverrideCheck.h
+++ clang-tidy/misc/UseOverrideCheck.h
@@ -20,9 +20,14 @@
 class UseOverrideCheck : public ClangTidyCheck {
 public:
   UseOverrideCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+      : ClangTidyCheck(Name, Context),
+        KeepVirtual(Options.get("KeepVirtual", false)) {}
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const bool KeepVirtual;
 };
 
 } // namespace misc
Index: test/clang-tidy/misc-use-override-keep-virtual.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-use-override-keep-virtual.cpp
@@ -0,0 +1,248 @@
+// 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 test is based on misc-use-override.cpp.
+#define ABSTRACT = 0
+
+#define OVERRIDE override
+#define VIRTUAL virtual
+#define NOT_VIRTUAL
+#define NOT_OVERRIDE
+
+#define MUST_USE_RESULT __attribute__((warn_unused_result))
+#define UNUSED __attribute__((unused))
+
+struct MUST_USE_RESULT MustUseResultObject {};
+
+struct Base {
+  virtual ~Base() {}
+  virtual void a();
+  virtual void b();
+  virtual void c();
+  virtual void d();
+  virtual void d2();
+  virtual void e() = 0;
+  virtual void f() = 0;
+  virtual void g() = 0;
+
+  virtual void j() const;
+  virtual MustUseResultObject k();
+  virtual bool l() MUST_USE_RESULT UNUSED;
+  virtual bool n() MUST_USE_RESULT UNUSED;
+
+  virtual void m();
+  virtual void m2();
+  virtual void o() __attribute__((unused));
+};
+
+struct SimpleCases : public Base {
+public:
+  virtual ~SimpleCases();
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual'
+  // CHECK-FIXES: {{^}}  virtual ~SimpleCases() override;
+
+  void a();
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: annotate this
+  // CHECK-FIXES: {{^}}  void a() override;
+
+  void b() override;
+  // CHECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  void b() override;
+
+  virtual void c();
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual void c() override;
+
+  virtual void d() override;
+  // ChECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  virtual void d() override;
+
+  virtual void d2() final;
+  // ChECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  virtual void d2() final;
+
+  virtual void e() = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual void e() override = 0;
+
+  virtual void f()=0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual void f()override =0;
+
+  virtual void g() ABSTRACT;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual void g() override ABSTRACT;
+
+  virtual void j() const;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual void j() const override;
+
+  virtual MustUseResultObject k();  // Has an implicit attribute.
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual MustUseResultObject k() override;
+
+  virtual bool l() MUST_USE_RESULT UNUSED;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual bool l() override MUST_USE_RESULT UNUSED;
+
+  virtual bool n() UNUSED MUST_USE_RESULT;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual bool n() override UNUSED MUST_USE_RESULT;
+
+  void m() override final;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'override' is redundant since the function is already declared 'final'
+  // CHECK-FIXES: {{^}}  void m() final;
+
+  virtual void m2() override final;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'override' is redundant since the function is already declared 'final'
+  // CHECK-FIXES: {{^}}  virtual void m2() final;
+
+  virtual void o() __attribute__((unused));
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual void o() override __attribute__((unused));
+};
+
+// CHECK-MESSAGES-NOT: warning:
+
+void SimpleCases::c() {}
+// CHECK-FIXES: {{^}}void SimpleCases::c() {}
+
+SimpleCases::~SimpleCases() {}
+// CHECK-FIXES: {{^}}SimpleCases::~SimpleCases() {}
+
+struct DefaultedDestructor : public Base {
+  DefaultedDestructor() {}
+  virtual ~DefaultedDestructor() = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual ~DefaultedDestructor() override = default;
+};
+
+struct FinalSpecified : public Base {
+public:
+  virtual ~FinalSpecified() final;
+  // ChECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  virtual ~FinalSpecified() final;
+
+  void b() final;
+  // CHECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  void b() final;
+
+  virtual void d() final;
+  // CHECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  virtual void d() final;
+
+  virtual void e() final = 0;
+  // CHECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  virtual void e() final = 0;
+
+  virtual void j() const final;
+  // CHECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  virtual void j() const final;
+
+  virtual bool l() final MUST_USE_RESULT UNUSED;
+  // CHECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  virtual bool l() final MUST_USE_RESULT UNUSED;
+};
+
+struct InlineDefinitions : public Base {
+public:
+  virtual ~InlineDefinitions() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual ~InlineDefinitions() override {}
+
+  void a() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: annotate this
+  // CHECK-FIXES: {{^}}  void a() override {}
+
+  void b() override {}
+  // CHECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  void b() override {}
+
+  virtual void c() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual void c() override {}
+
+  virtual void d() override {}
+  // CHECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  virtual void d() override {}
+
+  virtual void j() const {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual void j() const override {}
+
+  virtual MustUseResultObject k() {}  // Has an implicit attribute.
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual MustUseResultObject k() override {}
+
+  virtual bool l() MUST_USE_RESULT UNUSED {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual bool l() override MUST_USE_RESULT UNUSED {}
+};
+
+struct Macros : public Base {
+  // Tests for 'virtual' and 'override' being defined through macros. Basically
+  // give up for now.
+  NOT_VIRTUAL void a() NOT_OVERRIDE;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: annotate this
+  // CHECK-FIXES: {{^}}  NOT_VIRTUAL void a() override NOT_OVERRIDE;
+
+  VIRTUAL void b() NOT_OVERRIDE;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  VIRTUAL void b() override NOT_OVERRIDE;
+
+  NOT_VIRTUAL void c() OVERRIDE;
+  // CHECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  NOT_VIRTUAL void c() OVERRIDE;
+
+  VIRTUAL void d() OVERRIDE;
+  // CHECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  VIRTUAL void d() OVERRIDE;
+
+#define FUNC(return_type, name) return_type name()
+  FUNC(void, e);
+  // CHECK-FIXES: {{^}}  FUNC(void, e);
+
+#define F virtual void f();
+  F
+  // CHECK-FIXES: {{^}}  F
+
+  VIRTUAL void g() OVERRIDE final;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'override' is redundant since the function is already declared 'final'
+  // CHECK-FIXES: {{^}}  VIRTUAL void g() final;
+};
+
+// Tests for templates.
+template <typename T> struct TemplateBase {
+  virtual void f(T t);
+};
+
+template <typename T> struct DerivedFromTemplate : public TemplateBase<T> {
+  virtual void f(T t);
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual void f(T t) override;
+};
+void f() { DerivedFromTemplate<int>().f(2); }
+
+template <class C>
+struct UnusedMemberInstantiation : public C {
+  virtual ~UnusedMemberInstantiation() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using
+  // CHECK-FIXES: {{^}}  virtual ~UnusedMemberInstantiation() override {}
+};
+struct IntantiateWithoutUse : public UnusedMemberInstantiation<Base> {};
+
+struct Base2 {
+  virtual ~Base2() {}
+  virtual void a();
+};
+
+// The OverrideAttr isn't propagated to specializations in all cases. Make sure
+// we don't add "override" a second time.
+template <int I>
+struct MembersOfSpecializations : public Base2 {
+  void a() override;
+  // CHECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  void a() override;
+};
+template <> void MembersOfSpecializations<3>::a() {}
+void ff() { MembersOfSpecializations<3>().a(); };
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to