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