On Thu, Oct 2, 2014 at 4:13 PM, Fariborz Jahanian <[email protected]> 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@+2 {{overridden virtual function is here}} > // expected-warning@+1 {{'sealed' keyword is a Microsoft extension}} > - virtual void SealedFunction() sealed; > + virtual void SealedFunction() sealed; // expected-note {{overridden > virtual function is here}} > }; > > // expected-note@+2 {{'SealedType' declared here}} > // expected-warning@+1 {{'sealed' keyword is a Microsoft extension}} > struct SealedType sealed : SomeBase { > // expected-error@+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@+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@+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@+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@+1 {{interface type cannot inherit from non-public > 'interface I1'}} > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
