r218925 - Patch to warn if 'override' is missing
David Majnemer
david.majnemer at gmail.com
Fri Oct 3 03:38:01 PDT 2014
On Thu, Oct 2, 2014 at 4:13 PM, Fariborz Jahanian <fjahanian at apple.com>
wrote:
> 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;
> + }
>
It is rather late over here so I might be reading this bit wrong but it
looks like this loop will execute exactly once.
If that is the intent, why not have:
if (MD->size_overridden_methods() > 0) {
const CXXMethodDecl *OMD = *MD->begin_overridden_methods();
Diag(OMD->getLocation(), diag::note_overridden_virtual_function);
}
> + }
> +}
> +
> /// 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'}}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141003/332b5441/attachment.html>
More information about the cfe-commits
mailing list