Skip to content

Commit b91c5d6

Browse files
author
Fariborz Jahanian
committedOct 2, 2014
Patch to warn if 'override' is missing
for an overriding method if class has at least one 'override' specified on one of its methods. Reviewed by Doug Gregor. rdar://18295240 (I have already checked in all llvm files with missing 'override' methods and Bob Wilson has fixed a TableGen of FastISel so no warnings are expected from build of llvm after this patch. I have already verified this). llvm-svn: 218925
1 parent a3f5869 commit b91c5d6

15 files changed

+125
-21
lines changed
 

‎clang/include/clang/Basic/DiagnosticGroups.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,8 @@ def CXX98CompatPedantic : DiagGroup<"c++98-compat-pedantic",
146146

147147
def CXX11Narrowing : DiagGroup<"c++11-narrowing">;
148148

149+
def CXX11WarnOverrideMethod : DiagGroup<"inconsistent-missing-override">;
150+
149151
// Original name of this warning in Clang
150152
def : DiagGroup<"c++0x-narrowing", [CXX11Narrowing]>;
151153

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1689,6 +1689,9 @@ def override_keyword_hides_virtual_member_function : Error<
16891689
"%select{function|functions}1">;
16901690
def err_function_marked_override_not_overriding : Error<
16911691
"%0 marked 'override' but does not override any member functions">;
1692+
def warn_function_marked_not_override_overriding : Warning <
1693+
"%0 overrides a member function but is not marked 'override'">,
1694+
InGroup<CXX11WarnOverrideMethod>;
16921695
def err_class_marked_final_used_as_base : Error<
16931696
"base %0 is marked '%select{final|sealed}1'">;
16941697
def warn_abstract_final_class : Warning<

‎clang/include/clang/Sema/Sema.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5084,6 +5084,10 @@ class Sema {
50845084

50855085
/// CheckOverrideControl - Check C++11 override control semantics.
50865086
void CheckOverrideControl(NamedDecl *D);
5087+
5088+
/// DiagnoseAbsenseOfOverrideControl - Diagnose if override control was
5089+
/// not used in the; otherwise, overriding method.
5090+
void DiagnoseAbsenseOfOverrideControl(NamedDecl *D);
50875091

50885092
/// CheckForFunctionMarkedFinal - Checks whether a virtual member function
50895093
/// overrides a virtual member function marked 'final', according to

‎clang/lib/Sema/SemaDeclCXX.cpp

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1897,6 +1897,31 @@ void Sema::CheckOverrideControl(NamedDecl *D) {
18971897
<< MD->getDeclName();
18981898
}
18991899

1900+
void Sema::DiagnoseAbsenseOfOverrideControl(NamedDecl *D) {
1901+
if (D->isInvalidDecl())
1902+
return;
1903+
CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D);
1904+
if (!MD || MD->isImplicit() || isa<CXXDestructorDecl>(MD))
1905+
return;
1906+
1907+
bool HasOverriddenMethods =
1908+
MD->begin_overridden_methods() != MD->end_overridden_methods();
1909+
if (HasOverriddenMethods) {
1910+
SourceLocation EndLocation =
1911+
(MD->isPure() || MD->hasAttr<FinalAttr>())
1912+
? SourceLocation() : MD->getSourceRange().getEnd();
1913+
Diag(MD->getLocation(), diag::warn_function_marked_not_override_overriding)
1914+
<< MD->getDeclName()
1915+
<< FixItHint::CreateReplacement(EndLocation, ") override");
1916+
for (CXXMethodDecl::method_iterator I = MD->begin_overridden_methods(),
1917+
E = MD->end_overridden_methods(); I != E; ++I) {
1918+
const CXXMethodDecl *OMD = *I;
1919+
Diag(OMD->getLocation(), diag::note_overridden_virtual_function);
1920+
break;
1921+
}
1922+
}
1923+
}
1924+
19001925
/// CheckIfOverriddenFunctionIsMarkedFinal - Checks whether a virtual member
19011926
/// function overrides a virtual member function marked 'final', according to
19021927
/// C++11 [class.virtual]p4.
@@ -4680,13 +4705,18 @@ void Sema::CheckCompletedCXXClass(CXXRecordDecl *Record) {
46804705
}
46814706
}
46824707

4708+
bool HasMethodWithOverrideControl = false,
4709+
HasOverridingMethodWithoutOverrideControl = false;
46834710
if (!Record->isDependentType()) {
46844711
for (auto *M : Record->methods()) {
46854712
// See if a method overloads virtual methods in a base
46864713
// class without overriding any.
46874714
if (!M->isStatic())
46884715
DiagnoseHiddenVirtualMethods(M);
4689-
4716+
if (M->hasAttr<OverrideAttr>())
4717+
HasMethodWithOverrideControl = true;
4718+
else if (M->begin_overridden_methods() != M->end_overridden_methods())
4719+
HasOverridingMethodWithoutOverrideControl = true;
46904720
// Check whether the explicitly-defaulted special members are valid.
46914721
if (!M->isInvalidDecl() && M->isExplicitlyDefaulted())
46924722
CheckExplicitlyDefaultedSpecialMember(M);
@@ -4705,6 +4735,14 @@ void Sema::CheckCompletedCXXClass(CXXRecordDecl *Record) {
47054735
}
47064736
}
47074737

4738+
if (HasMethodWithOverrideControl
4739+
&& HasOverridingMethodWithoutOverrideControl) {
4740+
// At least one method has the 'override' control declared.
4741+
// Diagnose all other overridden methods which do not have 'override' specified on them.
4742+
for (auto *M : Record->methods())
4743+
if (!M->hasAttr<OverrideAttr>())
4744+
DiagnoseAbsenseOfOverrideControl(M);
4745+
}
47084746
// C++11 [dcl.constexpr]p8: A constexpr specifier for a non-static member
47094747
// function that is not a constructor declares that member function to be
47104748
// const. [...] The class of which that function is a member shall be

‎clang/test/CXX/class.derived/class.virtual/p3-0x.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ struct D : B {
6161
namespace PR13499 {
6262
struct X {
6363
virtual void f();
64-
virtual void h();
64+
virtual void h(); // expected-note 2 {{overridden virtual function is here}}
6565
};
6666
template<typename T> struct A : X {
6767
void f() override;
@@ -83,21 +83,21 @@ namespace PR13499 {
8383
template<typename...T> struct E : X {
8484
void f(T...) override;
8585
void g(T...) override; // expected-error {{only virtual member functions can be marked 'override'}}
86-
void h(T...) final;
86+
void h(T...) final; // expected-warning {{'h' overrides a member function but is not marked 'override'}}
8787
void i(T...) final; // expected-error {{only virtual member functions can be marked 'final'}}
8888
};
8989
// FIXME: Diagnose these in the template definition, not in the instantiation.
9090
E<> e; // expected-note {{in instantiation of}}
9191

9292
template<typename T> struct Y : T {
9393
void f() override;
94-
void h() final;
94+
void h() final; // expected-warning {{'h' overrides a member function but is not marked 'override'}}
9595
};
9696
template<typename T> struct Z : T {
9797
void g() override; // expected-error {{only virtual member functions can be marked 'override'}}
9898
void i() final; // expected-error {{only virtual member functions can be marked 'final'}}
9999
};
100-
Y<X> y;
100+
Y<X> y; // expected-note {{in instantiation of}}
101101
Z<X> z; // expected-note {{in instantiation of}}
102102
}
103103

‎clang/test/FixIt/cxx11-fixit-missing-override.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// RUN: cp %s %t
2+
// RUN: %clang_cc1 -x c++ -std=c++11 -Winconsistent-missing-override -fixit %t
3+
// RUN: %clang_cc1 -x c++ -std=c++11 -Winconsistent-missing-override -Werror %t
4+
5+
struct A
6+
{
7+
virtual void foo();
8+
virtual void bar(); // expected-note {{overridden virtual function is here}}
9+
virtual void gorf() {}
10+
virtual void g() = 0; // expected-note {{overridden virtual function is here}}
11+
};
12+
13+
struct B : A
14+
{
15+
void foo() override;
16+
void bar(); // expected-warning {{'bar' overrides a member function but is not marked 'override'}}
17+
};
18+
19+
struct C : B
20+
{
21+
virtual void g() override = 0; // expected-warning {{'g' overrides a member function but is not marked 'override'}}
22+
virtual void gorf() override {}
23+
void foo() {}
24+
};
25+
26+
struct D : C {
27+
virtual void g()override ;
28+
virtual void foo(){
29+
}
30+
void bar() override;
31+
};
32+
33+

‎clang/test/FixIt/fixit-cxx0x.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ namespace SemiCommaTypo {
2424
int o;
2525

2626
struct Base {
27-
virtual void f2(), f3();
27+
virtual void f2(), f3(); // expected-note {{overridden virtual function is here}}
2828
};
2929
struct MemberDeclarator : Base {
3030
int k : 4,
@@ -33,7 +33,7 @@ namespace SemiCommaTypo {
3333
char c, // expected-error {{expected ';' at end of declaration}}
3434
typedef void F(), // expected-error {{expected ';' at end of declaration}}
3535
F f1,
36-
f2 final,
36+
f2 final, // expected-warning {{'f2' overrides a member function but is not marked 'override'}}
3737
f3 override, // expected-error {{expected ';' at end of declaration}}
3838
};
3939
}

‎clang/test/Parser/MicrosoftExtensions.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,12 +208,12 @@ extern TypenameWrongPlace<AAAA> PR16925;
208208

209209
__interface MicrosoftInterface;
210210
__interface MicrosoftInterface {
211-
void foo1() = 0;
211+
void foo1() = 0; // expected-note {{overridden virtual function is here}}
212212
virtual void foo2() = 0;
213213
};
214214

215215
__interface MicrosoftDerivedInterface : public MicrosoftInterface {
216-
void foo1();
216+
void foo1(); // expected-warning {{'foo1' overrides a member function but is not marked 'override'}}
217217
void foo2() override;
218218
void foo3();
219219
};

‎clang/test/Parser/cxx0x-decl.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,13 +83,13 @@ namespace PR5066 {
8383

8484
namespace FinalOverride {
8585
struct Base {
86-
virtual void *f();
86+
virtual void *f(); // expected-note {{overridden virtual function is here}}
8787
virtual void *g();
8888
virtual void *h();
8989
virtual void *i();
9090
};
9191
struct Derived : Base {
92-
virtual auto f() -> void *final;
92+
virtual auto f() -> void *final; // expected-warning {{'f' overrides a member function but is not marked 'override'}}
9393
virtual auto g() -> void *override;
9494
virtual auto h() -> void *final override;
9595
virtual auto i() -> void *override final;

‎clang/test/Parser/cxx0x-in-cxx98.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@ struct X {
1010

1111
struct B {
1212
virtual void f();
13-
virtual void g();
13+
virtual void g(); // expected-note {{overridden virtual function is here}}
1414
};
1515
struct D final : B { // expected-warning {{'final' keyword is a C++11 extension}}
1616
virtual void f() override; // expected-warning {{'override' keyword is a C++11 extension}}
17-
virtual void g() final; // expected-warning {{'final' keyword is a C++11 extension}}
17+
virtual void g() final; // expected-warning {{'final' keyword is a C++11 extension}} \
18+
// expected-warning {{'g' overrides a member function but is not marked 'override'}}
1819
};
1920

2021
void NewBracedInitList() {

‎clang/test/SemaCXX/MicrosoftExtensions.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -372,14 +372,14 @@ struct SomeBase {
372372

373373
// expected-note@+2 {{overridden virtual function is here}}
374374
// expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}
375-
virtual void SealedFunction() sealed;
375+
virtual void SealedFunction() sealed; // expected-note {{overridden virtual function is here}}
376376
};
377377

378378
// expected-note@+2 {{'SealedType' declared here}}
379379
// expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}
380380
struct SealedType sealed : SomeBase {
381381
// expected-error@+1 {{declaration of 'SealedFunction' overrides a 'sealed' function}}
382-
virtual void SealedFunction();
382+
virtual void SealedFunction(); // expected-warning {{'SealedFunction' overrides a member function but is not marked 'override'}}
383383

384384
// expected-warning@+1 {{'override' keyword is a C++11 extension}}
385385
virtual void OverrideMe() override;

‎clang/test/SemaCXX/attr-gnu.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,15 @@ void g(int a[static [[]] 5]); // expected-error {{static array size is a C99 fea
1515
namespace {
1616
class B {
1717
public:
18-
virtual void test() {}
18+
virtual void test() {} // expected-note {{overridden virtual function is here}}
1919
virtual void test2() {}
2020
virtual void test3() {}
2121
};
2222

2323
class D : public B {
2424
public:
25-
void test() __attribute__((deprecated)) final {} // expected-warning {{GCC does not allow an attribute in this position on a function declaration}}
25+
void test() __attribute__((deprecated)) final {} // expected-warning {{GCC does not allow an attribute in this position on a function declaration}} \
26+
// expected-warning {{'test' overrides a member function but is not marked 'override'}}
2627
void test2() [[]] override {} // Ok
2728
void test3() __attribute__((cf_unknown_transfer)) override {} // Ok, not known to GCC.
2829
};

‎clang/test/SemaCXX/cxx11-warn-missing-override.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// RUN: %clang_cc1 -fsyntax-only -Winconsistent-missing-override -verify -std=c++11 %s
2+
struct A
3+
{
4+
virtual void foo();
5+
virtual void bar(); // expected-note {{overridden virtual function is here}}
6+
virtual void gorf() {}
7+
virtual void g() = 0; // expected-note {{overridden virtual function is here}}
8+
};
9+
10+
struct B : A
11+
{
12+
void foo() override;
13+
void bar(); // expected-warning {{'bar' overrides a member function but is not marked 'override'}}
14+
};
15+
16+
struct C : B
17+
{
18+
virtual void g() = 0; // expected-warning {{'g' overrides a member function but is not marked 'override'}}
19+
virtual void gorf() override {}
20+
};
21+

‎clang/test/SemaCXX/cxx98-compat.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,12 @@ struct InClassInit {
120120

121121
struct OverrideControlBase {
122122
virtual void f();
123-
virtual void g();
123+
virtual void g(); // expected-note {{overridden virtual function is here}}
124124
};
125125
struct OverrideControl final : OverrideControlBase { // expected-warning {{'final' keyword is incompatible with C++98}}
126126
virtual void f() override; // expected-warning {{'override' keyword is incompatible with C++98}}
127-
virtual void g() final; // expected-warning {{'final' keyword is incompatible with C++98}}
127+
virtual void g() final; // expected-warning {{'final' keyword is incompatible with C++98}} \
128+
// expected-warning {{'g' overrides a member function but is not marked 'override'}}
128129
};
129130

130131
using AliasDecl = int; // expected-warning {{alias declarations are incompatible with C++98}}

‎clang/test/SemaCXX/ms-interface.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ __interface I1 {
1212
operator int();
1313
// expected-error@+1 {{nested class I1::(anonymous) is not permitted within an interface type}}
1414
struct { int a; };
15-
void fn2() {
15+
void fn2() { // expected-note {{overridden virtual function is here}}
1616
struct A { }; // should be ignored: not a nested class
1717
}
1818
protected: // expected-error {{interface types cannot specify 'protected' access}}
@@ -44,7 +44,7 @@ __interface I3 final {
4444
__interface I4 : I1, I2 {
4545
void fn1() const override;
4646
// expected-error@+1 {{'final' keyword not permitted with interface types}}
47-
void fn2() final;
47+
void fn2() final; // expected-warning {{'fn2' overrides a member function but is not marked 'override'}}
4848
};
4949

5050
// expected-error@+1 {{interface type cannot inherit from non-public 'interface I1'}}

0 commit comments

Comments
 (0)
Please sign in to comment.