clang-tidy: [use-override] Remove 'override' if 'final' is also present.
Also, make warning more precise by distinguishing different cases. llvm-svn: 210651
This commit is contained in:
parent
9ae3d02193
commit
05bdb09da1
|
@ -61,14 +61,21 @@ void UseOverride::check(const MatchFinder::MatchResult &Result) {
|
||||||
Method->isOutOfLine())
|
Method->isOutOfLine())
|
||||||
return;
|
return;
|
||||||
|
|
||||||
if ((Method->getAttr<clang::OverrideAttr>() != nullptr ||
|
bool HasVirtual = Method->isVirtualAsWritten();
|
||||||
Method->getAttr<clang::FinalAttr>() != nullptr) &&
|
bool HasOverride = Method->getAttr<OverrideAttr>();
|
||||||
!Method->isVirtualAsWritten())
|
bool HasFinal = Method->getAttr<FinalAttr>();
|
||||||
|
|
||||||
|
bool OnlyVirtualSpecified = HasVirtual && !HasOverride && !HasFinal;
|
||||||
|
unsigned KeywordCount = HasVirtual + HasOverride + HasFinal;
|
||||||
|
|
||||||
|
if (!OnlyVirtualSpecified && KeywordCount == 1)
|
||||||
return; // Nothing to do.
|
return; // Nothing to do.
|
||||||
|
|
||||||
DiagnosticBuilder Diag =
|
DiagnosticBuilder Diag =
|
||||||
diag(Method->getLocation(),
|
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 =
|
CharSourceRange FileRange =
|
||||||
Lexer::makeFileCharRange(CharSourceRange::getTokenRange(
|
Lexer::makeFileCharRange(CharSourceRange::getTokenRange(
|
||||||
|
@ -85,8 +92,7 @@ void UseOverride::check(const MatchFinder::MatchResult &Result) {
|
||||||
Result.Context->getLangOpts());
|
Result.Context->getLangOpts());
|
||||||
|
|
||||||
// Add 'override' on inline declarations that don't already have it.
|
// Add 'override' on inline declarations that don't already have it.
|
||||||
if (Method->getAttr<clang::OverrideAttr>() == nullptr &&
|
if (!HasFinal && !HasOverride) {
|
||||||
Method->getAttr<clang::FinalAttr>() == nullptr) {
|
|
||||||
SourceLocation InsertLoc;
|
SourceLocation InsertLoc;
|
||||||
StringRef ReplacementText = "override ";
|
StringRef ReplacementText = "override ";
|
||||||
|
|
||||||
|
@ -120,6 +126,12 @@ void UseOverride::check(const MatchFinder::MatchResult &Result) {
|
||||||
Diag << FixItHint::CreateInsertion(InsertLoc, ReplacementText);
|
Diag << FixItHint::CreateInsertion(InsertLoc, ReplacementText);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (HasFinal && HasOverride) {
|
||||||
|
SourceLocation OverrideLoc = Method->getAttr<OverrideAttr>()->getLocation();
|
||||||
|
Diag << FixItHint::CreateRemoval(
|
||||||
|
CharSourceRange::getTokenRange(OverrideLoc, OverrideLoc));
|
||||||
|
}
|
||||||
|
|
||||||
if (Method->isVirtualAsWritten()) {
|
if (Method->isVirtualAsWritten()) {
|
||||||
for (Token Tok : Tokens) {
|
for (Token Tok : Tokens) {
|
||||||
if (Tok.is(tok::raw_identifier) && GetText(Tok, Sources) == "virtual") {
|
if (Tok.is(tok::raw_identifier) && GetText(Tok, Sources) == "virtual") {
|
||||||
|
|
|
@ -25,6 +25,8 @@ struct Base {
|
||||||
virtual void j() const;
|
virtual void j() const;
|
||||||
virtual MustUseResultObject k();
|
virtual MustUseResultObject k();
|
||||||
virtual bool l() MUST_USE_RESULT;
|
virtual bool l() MUST_USE_RESULT;
|
||||||
|
|
||||||
|
virtual void m();
|
||||||
};
|
};
|
||||||
|
|
||||||
struct SimpleCases : public Base {
|
struct SimpleCases : public Base {
|
||||||
|
@ -54,6 +56,9 @@ public:
|
||||||
// CHECK: {{^ MustUseResultObject k\(\) override;}}
|
// CHECK: {{^ MustUseResultObject k\(\) override;}}
|
||||||
virtual bool l() MUST_USE_RESULT; // Has an explicit attribute
|
virtual bool l() MUST_USE_RESULT; // Has an explicit attribute
|
||||||
// CHECK: {{^ bool l\(\) override MUST_USE_RESULT;}}
|
// CHECK: {{^ bool l\(\) override MUST_USE_RESULT;}}
|
||||||
|
|
||||||
|
virtual void m() override final;
|
||||||
|
// CHECK: {{^ void m\(\) final;}}
|
||||||
};
|
};
|
||||||
|
|
||||||
void SimpleCases::i() {}
|
void SimpleCases::i() {}
|
||||||
|
@ -125,6 +130,9 @@ struct Macros : public Base {
|
||||||
#define F virtual void f();
|
#define F virtual void f();
|
||||||
F
|
F
|
||||||
// CHECK: {{^ F}}
|
// CHECK: {{^ F}}
|
||||||
|
|
||||||
|
VIRTUAL void g() OVERRIDE final;
|
||||||
|
// CHECK: {{^ VIRTUAL void g\(\) final;}}
|
||||||
};
|
};
|
||||||
|
|
||||||
// Tests for templates.
|
// Tests for templates.
|
||||||
|
|
|
@ -7,6 +7,8 @@ struct Base {
|
||||||
virtual void b();
|
virtual void b();
|
||||||
virtual void c();
|
virtual void c();
|
||||||
virtual void d();
|
virtual void d();
|
||||||
|
virtual void e();
|
||||||
|
virtual void f();
|
||||||
};
|
};
|
||||||
|
|
||||||
struct SimpleCases : public Base {
|
struct SimpleCases : public Base {
|
||||||
|
@ -15,11 +17,15 @@ public:
|
||||||
// CHECK: :[[@LINE-1]]:11: warning: Prefer using 'override' or 'final' instead of 'virtual'
|
// CHECK: :[[@LINE-1]]:11: warning: Prefer using 'override' or 'final' instead of 'virtual'
|
||||||
|
|
||||||
void a();
|
void a();
|
||||||
// CHECK: :[[@LINE-1]]:8: warning: Prefer using
|
// CHECK: :[[@LINE-1]]:8: warning: Use exactly
|
||||||
virtual void b();
|
virtual void b();
|
||||||
// CHECK: :[[@LINE-1]]:16: warning: Prefer using
|
// 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:
|
// CHECK-NOT: :[[@LINE-1]]:{{.*}} warning:
|
||||||
void d() final;
|
void f() final;
|
||||||
// CHECK-NOT: :[[@LINE-1]]:{{.*}} warning:
|
// CHECK-NOT: :[[@LINE-1]]:{{.*}} warning:
|
||||||
};
|
};
|
||||||
|
|
Loading…
Reference in New Issue