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: 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: 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 struct TemplateBase { + virtual void f(T t); +}; + +template struct DerivedFromTemplate : public TemplateBase { + virtual void f(T t); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using + // CHECK-FIXES: {{^}} virtual void f(T t) override; +}; +void f() { DerivedFromTemplate().f(2); } + +template +struct UnusedMemberInstantiation : public C { + virtual ~UnusedMemberInstantiation() {} + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using + // CHECK-FIXES: {{^}} virtual ~UnusedMemberInstantiation() override {} +}; +struct IntantiateWithoutUse : public UnusedMemberInstantiation {}; + +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 +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(); };