r218925 - Patch to warn if 'override' is missing
Fariborz Jahanian
fjahanian at apple.com
Thu Oct 2 16:13:52 PDT 2014
Author: fjahanian
Date: Thu Oct 2 18:13:51 2014
New Revision: 218925
URL: http://llvm.org/viewvc/llvm-project?rev=218925&view=rev
Log:
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).
Added:
cfe/trunk/test/FixIt/cxx11-fixit-missing-override.cpp
cfe/trunk/test/SemaCXX/cxx11-warn-missing-override.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CXX/class.derived/class.virtual/p3-0x.cpp
cfe/trunk/test/FixIt/fixit-cxx0x.cpp
cfe/trunk/test/Parser/MicrosoftExtensions.cpp
cfe/trunk/test/Parser/cxx0x-decl.cpp
cfe/trunk/test/Parser/cxx0x-in-cxx98.cpp
cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp
cfe/trunk/test/SemaCXX/attr-gnu.cpp
cfe/trunk/test/SemaCXX/cxx98-compat.cpp
cfe/trunk/test/SemaCXX/ms-interface.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=218925&r1=218924&r2=218925&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Thu Oct 2 18:13:51 2014
@@ -146,6 +146,8 @@ def CXX98CompatPedantic : DiagGroup<"c++
def CXX11Narrowing : DiagGroup<"c++11-narrowing">;
+def CXX11WarnOverrideMethod : DiagGroup<"inconsistent-missing-override">;
+
// Original name of this warning in Clang
def : DiagGroup<"c++0x-narrowing", [CXX11Narrowing]>;
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=218925&r1=218924&r2=218925&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Oct 2 18:13:51 2014
@@ -1689,6 +1689,9 @@ def override_keyword_hides_virtual_membe
"%select{function|functions}1">;
def err_function_marked_override_not_overriding : Error<
"%0 marked 'override' but does not override any member functions">;
+def warn_function_marked_not_override_overriding : Warning <
+ "%0 overrides a member function but is not marked 'override'">,
+ InGroup<CXX11WarnOverrideMethod>;
def err_class_marked_final_used_as_base : Error<
"base %0 is marked '%select{final|sealed}1'">;
def warn_abstract_final_class : Warning<
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=218925&r1=218924&r2=218925&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Oct 2 18:13:51 2014
@@ -5084,6 +5084,10 @@ public:
/// CheckOverrideControl - Check C++11 override control semantics.
void CheckOverrideControl(NamedDecl *D);
+
+ /// DiagnoseAbsenseOfOverrideControl - Diagnose if override control was
+ /// not used in the; otherwise, overriding method.
+ void DiagnoseAbsenseOfOverrideControl(NamedDecl *D);
/// CheckForFunctionMarkedFinal - Checks whether a virtual member function
/// overrides a virtual member function marked 'final', according to
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=218925&r1=218924&r2=218925&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Oct 2 18:13:51 2014
@@ -1897,6 +1897,31 @@ void Sema::CheckOverrideControl(NamedDec
<< MD->getDeclName();
}
+void Sema::DiagnoseAbsenseOfOverrideControl(NamedDecl *D) {
+ if (D->isInvalidDecl())
+ return;
+ CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D);
+ if (!MD || MD->isImplicit() || isa<CXXDestructorDecl>(MD))
+ return;
+
+ bool HasOverriddenMethods =
+ MD->begin_overridden_methods() != MD->end_overridden_methods();
+ if (HasOverriddenMethods) {
+ SourceLocation EndLocation =
+ (MD->isPure() || MD->hasAttr<FinalAttr>())
+ ? SourceLocation() : MD->getSourceRange().getEnd();
+ Diag(MD->getLocation(), diag::warn_function_marked_not_override_overriding)
+ << MD->getDeclName()
+ << FixItHint::CreateReplacement(EndLocation, ") override");
+ for (CXXMethodDecl::method_iterator I = MD->begin_overridden_methods(),
+ E = MD->end_overridden_methods(); I != E; ++I) {
+ const CXXMethodDecl *OMD = *I;
+ Diag(OMD->getLocation(), diag::note_overridden_virtual_function);
+ break;
+ }
+ }
+}
+
/// CheckIfOverriddenFunctionIsMarkedFinal - Checks whether a virtual member
/// function overrides a virtual member function marked 'final', according to
/// C++11 [class.virtual]p4.
@@ -4680,13 +4705,18 @@ void Sema::CheckCompletedCXXClass(CXXRec
}
}
+ bool HasMethodWithOverrideControl = false,
+ HasOverridingMethodWithoutOverrideControl = false;
if (!Record->isDependentType()) {
for (auto *M : Record->methods()) {
// See if a method overloads virtual methods in a base
// class without overriding any.
if (!M->isStatic())
DiagnoseHiddenVirtualMethods(M);
-
+ if (M->hasAttr<OverrideAttr>())
+ HasMethodWithOverrideControl = true;
+ else if (M->begin_overridden_methods() != M->end_overridden_methods())
+ HasOverridingMethodWithoutOverrideControl = true;
// Check whether the explicitly-defaulted special members are valid.
if (!M->isInvalidDecl() && M->isExplicitlyDefaulted())
CheckExplicitlyDefaultedSpecialMember(M);
@@ -4705,6 +4735,14 @@ void Sema::CheckCompletedCXXClass(CXXRec
}
}
+ if (HasMethodWithOverrideControl
+ && HasOverridingMethodWithoutOverrideControl) {
+ // At least one method has the 'override' control declared.
+ // Diagnose all other overridden methods which do not have 'override' specified on them.
+ for (auto *M : Record->methods())
+ if (!M->hasAttr<OverrideAttr>())
+ DiagnoseAbsenseOfOverrideControl(M);
+ }
// C++11 [dcl.constexpr]p8: A constexpr specifier for a non-static member
// function that is not a constructor declares that member function to be
// const. [...] The class of which that function is a member shall be
Modified: cfe/trunk/test/CXX/class.derived/class.virtual/p3-0x.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.derived/class.virtual/p3-0x.cpp?rev=218925&r1=218924&r2=218925&view=diff
==============================================================================
--- cfe/trunk/test/CXX/class.derived/class.virtual/p3-0x.cpp (original)
+++ cfe/trunk/test/CXX/class.derived/class.virtual/p3-0x.cpp Thu Oct 2 18:13:51 2014
@@ -61,7 +61,7 @@ struct D : B {
namespace PR13499 {
struct X {
virtual void f();
- virtual void h();
+ virtual void h(); // expected-note 2 {{overridden virtual function is here}}
};
template<typename T> struct A : X {
void f() override;
@@ -83,7 +83,7 @@ namespace PR13499 {
template<typename...T> struct E : X {
void f(T...) override;
void g(T...) override; // expected-error {{only virtual member functions can be marked 'override'}}
- void h(T...) final;
+ void h(T...) final; // expected-warning {{'h' overrides a member function but is not marked 'override'}}
void i(T...) final; // expected-error {{only virtual member functions can be marked 'final'}}
};
// FIXME: Diagnose these in the template definition, not in the instantiation.
@@ -91,13 +91,13 @@ namespace PR13499 {
template<typename T> struct Y : T {
void f() override;
- void h() final;
+ void h() final; // expected-warning {{'h' overrides a member function but is not marked 'override'}}
};
template<typename T> struct Z : T {
void g() override; // expected-error {{only virtual member functions can be marked 'override'}}
void i() final; // expected-error {{only virtual member functions can be marked 'final'}}
};
- Y<X> y;
+ Y<X> y; // expected-note {{in instantiation of}}
Z<X> z; // expected-note {{in instantiation of}}
}
Added: cfe/trunk/test/FixIt/cxx11-fixit-missing-override.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/cxx11-fixit-missing-override.cpp?rev=218925&view=auto
==============================================================================
--- cfe/trunk/test/FixIt/cxx11-fixit-missing-override.cpp (added)
+++ cfe/trunk/test/FixIt/cxx11-fixit-missing-override.cpp Thu Oct 2 18:13:51 2014
@@ -0,0 +1,33 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -x c++ -std=c++11 -Winconsistent-missing-override -fixit %t
+// RUN: %clang_cc1 -x c++ -std=c++11 -Winconsistent-missing-override -Werror %t
+
+struct A
+{
+ virtual void foo();
+ virtual void bar(); // expected-note {{overridden virtual function is here}}
+ virtual void gorf() {}
+ virtual void g() = 0; // expected-note {{overridden virtual function is here}}
+};
+
+struct B : A
+{
+ void foo() override;
+ void bar(); // expected-warning {{'bar' overrides a member function but is not marked 'override'}}
+};
+
+struct C : B
+{
+ virtual void g() override = 0; // expected-warning {{'g' overrides a member function but is not marked 'override'}}
+ virtual void gorf() override {}
+ void foo() {}
+};
+
+struct D : C {
+ virtual void g()override ;
+ virtual void foo(){
+ }
+ void bar() override;
+};
+
+
Modified: cfe/trunk/test/FixIt/fixit-cxx0x.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/fixit-cxx0x.cpp?rev=218925&r1=218924&r2=218925&view=diff
==============================================================================
--- cfe/trunk/test/FixIt/fixit-cxx0x.cpp (original)
+++ cfe/trunk/test/FixIt/fixit-cxx0x.cpp Thu Oct 2 18:13:51 2014
@@ -24,7 +24,7 @@ namespace SemiCommaTypo {
int o;
struct Base {
- virtual void f2(), f3();
+ virtual void f2(), f3(); // expected-note {{overridden virtual function is here}}
};
struct MemberDeclarator : Base {
int k : 4,
@@ -33,7 +33,7 @@ namespace SemiCommaTypo {
char c, // expected-error {{expected ';' at end of declaration}}
typedef void F(), // expected-error {{expected ';' at end of declaration}}
F f1,
- f2 final,
+ f2 final, // expected-warning {{'f2' overrides a member function but is not marked 'override'}}
f3 override, // expected-error {{expected ';' at end of declaration}}
};
}
Modified: cfe/trunk/test/Parser/MicrosoftExtensions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/MicrosoftExtensions.cpp?rev=218925&r1=218924&r2=218925&view=diff
==============================================================================
--- cfe/trunk/test/Parser/MicrosoftExtensions.cpp (original)
+++ cfe/trunk/test/Parser/MicrosoftExtensions.cpp Thu Oct 2 18:13:51 2014
@@ -208,12 +208,12 @@ extern TypenameWrongPlace<AAAA> PR16925;
__interface MicrosoftInterface;
__interface MicrosoftInterface {
- void foo1() = 0;
+ void foo1() = 0; // expected-note {{overridden virtual function is here}}
virtual void foo2() = 0;
};
__interface MicrosoftDerivedInterface : public MicrosoftInterface {
- void foo1();
+ void foo1(); // expected-warning {{'foo1' overrides a member function but is not marked 'override'}}
void foo2() override;
void foo3();
};
Modified: cfe/trunk/test/Parser/cxx0x-decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx0x-decl.cpp?rev=218925&r1=218924&r2=218925&view=diff
==============================================================================
--- cfe/trunk/test/Parser/cxx0x-decl.cpp (original)
+++ cfe/trunk/test/Parser/cxx0x-decl.cpp Thu Oct 2 18:13:51 2014
@@ -83,13 +83,13 @@ namespace PR5066 {
namespace FinalOverride {
struct Base {
- virtual void *f();
+ virtual void *f(); // expected-note {{overridden virtual function is here}}
virtual void *g();
virtual void *h();
virtual void *i();
};
struct Derived : Base {
- virtual auto f() -> void *final;
+ virtual auto f() -> void *final; // expected-warning {{'f' overrides a member function but is not marked 'override'}}
virtual auto g() -> void *override;
virtual auto h() -> void *final override;
virtual auto i() -> void *override final;
Modified: cfe/trunk/test/Parser/cxx0x-in-cxx98.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx0x-in-cxx98.cpp?rev=218925&r1=218924&r2=218925&view=diff
==============================================================================
--- cfe/trunk/test/Parser/cxx0x-in-cxx98.cpp (original)
+++ cfe/trunk/test/Parser/cxx0x-in-cxx98.cpp Thu Oct 2 18:13:51 2014
@@ -10,11 +10,12 @@ struct X {
struct B {
virtual void f();
- virtual void g();
+ virtual void g(); // expected-note {{overridden virtual function is here}}
};
struct D final : B { // expected-warning {{'final' keyword is a C++11 extension}}
virtual void f() override; // expected-warning {{'override' keyword is a C++11 extension}}
- virtual void g() final; // expected-warning {{'final' keyword is a C++11 extension}}
+ virtual void g() final; // expected-warning {{'final' keyword is a C++11 extension}} \
+ // expected-warning {{'g' overrides a member function but is not marked 'override'}}
};
void NewBracedInitList() {
Modified: cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp?rev=218925&r1=218924&r2=218925&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp (original)
+++ cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp Thu Oct 2 18:13:51 2014
@@ -372,14 +372,14 @@ struct SomeBase {
// expected-note at +2 {{overridden virtual function is here}}
// expected-warning at +1 {{'sealed' keyword is a Microsoft extension}}
- virtual void SealedFunction() sealed;
+ virtual void SealedFunction() sealed; // expected-note {{overridden virtual function is here}}
};
// expected-note at +2 {{'SealedType' declared here}}
// expected-warning at +1 {{'sealed' keyword is a Microsoft extension}}
struct SealedType sealed : SomeBase {
// expected-error at +1 {{declaration of 'SealedFunction' overrides a 'sealed' function}}
- virtual void SealedFunction();
+ virtual void SealedFunction(); // expected-warning {{'SealedFunction' overrides a member function but is not marked 'override'}}
// expected-warning at +1 {{'override' keyword is a C++11 extension}}
virtual void OverrideMe() override;
Modified: cfe/trunk/test/SemaCXX/attr-gnu.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-gnu.cpp?rev=218925&r1=218924&r2=218925&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/attr-gnu.cpp (original)
+++ cfe/trunk/test/SemaCXX/attr-gnu.cpp Thu Oct 2 18:13:51 2014
@@ -15,14 +15,15 @@ void g(int a[static [[]] 5]); // expecte
namespace {
class B {
public:
- virtual void test() {}
+ virtual void test() {} // expected-note {{overridden virtual function is here}}
virtual void test2() {}
virtual void test3() {}
};
class D : public B {
public:
- void test() __attribute__((deprecated)) final {} // expected-warning {{GCC does not allow an attribute in this position on a function declaration}}
+ void test() __attribute__((deprecated)) final {} // expected-warning {{GCC does not allow an attribute in this position on a function declaration}} \
+ // expected-warning {{'test' overrides a member function but is not marked 'override'}}
void test2() [[]] override {} // Ok
void test3() __attribute__((cf_unknown_transfer)) override {} // Ok, not known to GCC.
};
Added: cfe/trunk/test/SemaCXX/cxx11-warn-missing-override.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx11-warn-missing-override.cpp?rev=218925&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/cxx11-warn-missing-override.cpp (added)
+++ cfe/trunk/test/SemaCXX/cxx11-warn-missing-override.cpp Thu Oct 2 18:13:51 2014
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fsyntax-only -Winconsistent-missing-override -verify -std=c++11 %s
+struct A
+{
+ virtual void foo();
+ virtual void bar(); // expected-note {{overridden virtual function is here}}
+ virtual void gorf() {}
+ virtual void g() = 0; // expected-note {{overridden virtual function is here}}
+};
+
+struct B : A
+{
+ void foo() override;
+ void bar(); // expected-warning {{'bar' overrides a member function but is not marked 'override'}}
+};
+
+struct C : B
+{
+ virtual void g() = 0; // expected-warning {{'g' overrides a member function but is not marked 'override'}}
+ virtual void gorf() override {}
+};
+
Modified: cfe/trunk/test/SemaCXX/cxx98-compat.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx98-compat.cpp?rev=218925&r1=218924&r2=218925&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/cxx98-compat.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx98-compat.cpp Thu Oct 2 18:13:51 2014
@@ -120,11 +120,12 @@ struct InClassInit {
struct OverrideControlBase {
virtual void f();
- virtual void g();
+ virtual void g(); // expected-note {{overridden virtual function is here}}
};
struct OverrideControl final : OverrideControlBase { // expected-warning {{'final' keyword is incompatible with C++98}}
virtual void f() override; // expected-warning {{'override' keyword is incompatible with C++98}}
- virtual void g() final; // expected-warning {{'final' keyword is incompatible with C++98}}
+ virtual void g() final; // expected-warning {{'final' keyword is incompatible with C++98}} \
+ // expected-warning {{'g' overrides a member function but is not marked 'override'}}
};
using AliasDecl = int; // expected-warning {{alias declarations are incompatible with C++98}}
Modified: cfe/trunk/test/SemaCXX/ms-interface.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ms-interface.cpp?rev=218925&r1=218924&r2=218925&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/ms-interface.cpp (original)
+++ cfe/trunk/test/SemaCXX/ms-interface.cpp Thu Oct 2 18:13:51 2014
@@ -12,7 +12,7 @@ __interface I1 {
operator int();
// expected-error at +1 {{nested class I1::(anonymous) is not permitted within an interface type}}
struct { int a; };
- void fn2() {
+ void fn2() { // expected-note {{overridden virtual function is here}}
struct A { }; // should be ignored: not a nested class
}
protected: // expected-error {{interface types cannot specify 'protected' access}}
@@ -44,7 +44,7 @@ __interface I3 final {
__interface I4 : I1, I2 {
void fn1() const override;
// expected-error at +1 {{'final' keyword not permitted with interface types}}
- void fn2() final;
+ void fn2() final; // expected-warning {{'fn2' overrides a member function but is not marked 'override'}}
};
// expected-error at +1 {{interface type cannot inherit from non-public 'interface I1'}}
More information about the cfe-commits
mailing list