From: Daniel Jasper Date: Wed, 11 Jun 2014 12:18:24 +0000 (+0000) Subject: clang-tidy: [use-override] Remove 'override' if 'final' is also present. X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=05bdb09da1b8d37d75343d777d146e58347264d9;p=platform%2Fupstream%2Fllvm.git clang-tidy: [use-override] Remove 'override' if 'final' is also present. Also, make warning more precise by distinguishing different cases. llvm-svn: 210651 --- diff --git a/clang-tools-extra/clang-tidy/misc/UseOverride.cpp b/clang-tools-extra/clang-tidy/misc/UseOverride.cpp index 32f3d7c..7ad9ee6 100644 --- a/clang-tools-extra/clang-tidy/misc/UseOverride.cpp +++ b/clang-tools-extra/clang-tidy/misc/UseOverride.cpp @@ -61,14 +61,21 @@ void UseOverride::check(const MatchFinder::MatchResult &Result) { Method->isOutOfLine()) return; - if ((Method->getAttr() != nullptr || - Method->getAttr() != nullptr) && - !Method->isVirtualAsWritten()) + bool HasVirtual = Method->isVirtualAsWritten(); + bool HasOverride = Method->getAttr(); + bool HasFinal = Method->getAttr(); + + bool OnlyVirtualSpecified = HasVirtual && !HasOverride && !HasFinal; + unsigned KeywordCount = HasVirtual + HasOverride + HasFinal; + + if (!OnlyVirtualSpecified && KeywordCount == 1) return; // Nothing to do. DiagnosticBuilder Diag = diag(Method->getLocation(), - "Prefer using 'override' or 'final' instead of 'virtual'"); + OnlyVirtualSpecified + ? "Prefer using 'override' or 'final' instead of 'virtual'" + : "Use exactly one of 'virtual', 'override' and 'final'"); CharSourceRange FileRange = Lexer::makeFileCharRange(CharSourceRange::getTokenRange( @@ -85,8 +92,7 @@ void UseOverride::check(const MatchFinder::MatchResult &Result) { Result.Context->getLangOpts()); // Add 'override' on inline declarations that don't already have it. - if (Method->getAttr() == nullptr && - Method->getAttr() == nullptr) { + if (!HasFinal && !HasOverride) { SourceLocation InsertLoc; StringRef ReplacementText = "override "; @@ -120,6 +126,12 @@ void UseOverride::check(const MatchFinder::MatchResult &Result) { Diag << FixItHint::CreateInsertion(InsertLoc, ReplacementText); } + if (HasFinal && HasOverride) { + SourceLocation OverrideLoc = Method->getAttr()->getLocation(); + Diag << FixItHint::CreateRemoval( + CharSourceRange::getTokenRange(OverrideLoc, OverrideLoc)); + } + if (Method->isVirtualAsWritten()) { for (Token Tok : Tokens) { if (Tok.is(tok::raw_identifier) && GetText(Tok, Sources) == "virtual") { diff --git a/clang-tools-extra/test/clang-tidy/use-override-fix.cpp b/clang-tools-extra/test/clang-tidy/use-override-fix.cpp index e2e61b4..e389fc0 100644 --- a/clang-tools-extra/test/clang-tidy/use-override-fix.cpp +++ b/clang-tools-extra/test/clang-tidy/use-override-fix.cpp @@ -25,6 +25,8 @@ struct Base { virtual void j() const; virtual MustUseResultObject k(); virtual bool l() MUST_USE_RESULT; + + virtual void m(); }; struct SimpleCases : public Base { @@ -54,6 +56,9 @@ public: // CHECK: {{^ MustUseResultObject k\(\) override;}} virtual bool l() MUST_USE_RESULT; // Has an explicit attribute // CHECK: {{^ bool l\(\) override MUST_USE_RESULT;}} + + virtual void m() override final; + // CHECK: {{^ void m\(\) final;}} }; void SimpleCases::i() {} @@ -125,6 +130,9 @@ struct Macros : public Base { #define F virtual void f(); F // CHECK: {{^ F}} + + VIRTUAL void g() OVERRIDE final; + // CHECK: {{^ VIRTUAL void g\(\) final;}} }; // Tests for templates. diff --git a/clang-tools-extra/test/clang-tidy/use-override.cpp b/clang-tools-extra/test/clang-tidy/use-override.cpp index 5f1f430..bb30508 100644 --- a/clang-tools-extra/test/clang-tidy/use-override.cpp +++ b/clang-tools-extra/test/clang-tidy/use-override.cpp @@ -7,6 +7,8 @@ struct Base { virtual void b(); virtual void c(); virtual void d(); + virtual void e(); + virtual void f(); }; struct SimpleCases : public Base { @@ -15,11 +17,15 @@ public: // CHECK: :[[@LINE-1]]:11: warning: Prefer using 'override' or 'final' instead of 'virtual' void a(); - // CHECK: :[[@LINE-1]]:8: warning: Prefer using + // CHECK: :[[@LINE-1]]:8: warning: Use exactly virtual void b(); // CHECK: :[[@LINE-1]]:16: warning: Prefer using - void c() override; + virtual void c() override; + // CHECK: :[[@LINE-1]]:16: warning: Use exactly + void d() override final; + // CHECK: :[[@LINE-1]]:8: warning: Use exactly + void e() override; // CHECK-NOT: :[[@LINE-1]]:{{.*}} warning: - void d() final; + void f() final; // CHECK-NOT: :[[@LINE-1]]:{{.*}} warning: };