diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -279,9 +279,13 @@ def CXX11Narrowing : DiagGroup<"c++11-narrowing">; -def CXX11WarnOverrideDestructor : +def CXX11WarnInconsistentOverrideDestructor : DiagGroup<"inconsistent-missing-destructor-override">; -def CXX11WarnOverrideMethod : DiagGroup<"inconsistent-missing-override">; +def CXX11WarnInconsistentOverrideMethod : + DiagGroup<"inconsistent-missing-override">; +def CXX11WarnSuggestOverrideDestructor : DiagGroup<"suggest-destructor-override">; +def CXX11WarnSuggestOverride : DiagGroup<"suggest-override", + [CXX11WarnSuggestOverrideDestructor]>; // Original name of this warning in Clang def : DiagGroup<"c++0x-narrowing", [CXX11Narrowing]>; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2361,12 +2361,22 @@ "%select{function|functions}1">; def err_function_marked_override_not_overriding : Error< "%0 marked 'override' but does not override any member functions">; -def warn_destructor_marked_not_override_overriding : Warning < - "%0 overrides a destructor but is not marked 'override'">, - InGroup, DefaultIgnore; -def warn_function_marked_not_override_overriding : Warning < - "%0 overrides a member function but is not marked 'override'">, - InGroup; +def warn_destructor_marked_not_override_overriding : TextSubstitution < + "%0 overrides a destructor but is not marked 'override'">; +def warn_function_marked_not_override_overriding : TextSubstitution < + "%0 overrides a member function but is not marked 'override'">; +def warn_inconsistent_destructor_marked_not_override_overriding : Warning < + "%sub{warn_destructor_marked_not_override_overriding}0">, + InGroup, DefaultIgnore; +def warn_inconsistent_function_marked_not_override_overriding : Warning < + "%sub{warn_function_marked_not_override_overriding}0">, + InGroup; +def warn_suggest_destructor_marked_not_override_overriding : Warning < + "%sub{warn_destructor_marked_not_override_overriding}0">, + InGroup, DefaultIgnore; +def warn_suggest_function_marked_not_override_overriding : Warning < + "%sub{warn_function_marked_not_override_overriding}0">, + InGroup, DefaultIgnore; def err_class_marked_final_used_as_base : Error< "base %0 is marked '%select{final|sealed}1'">; def warn_abstract_final_class : Warning< diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -6930,7 +6930,7 @@ /// DiagnoseAbsenceOfOverrideControl - Diagnose if 'override' keyword was /// not used in the declaration of an overriding method. - void DiagnoseAbsenceOfOverrideControl(NamedDecl *D); + void DiagnoseAbsenceOfOverrideControl(NamedDecl *D, bool Inconsistent); /// CheckForFunctionMarkedFinal - Checks whether a virtual member function /// overrides a virtual member function marked 'final', according to diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -3045,7 +3045,7 @@ << MD->getDeclName(); } -void Sema::DiagnoseAbsenceOfOverrideControl(NamedDecl *D) { +void Sema::DiagnoseAbsenceOfOverrideControl(NamedDecl *D, bool Inconsistent) { if (D->isInvalidDecl() || D->hasAttr()) return; CXXMethodDecl *MD = dyn_cast(D); @@ -3061,12 +3061,23 @@ return; if (MD->size_overridden_methods() > 0) { - unsigned DiagID = isa(MD) - ? diag::warn_destructor_marked_not_override_overriding - : diag::warn_function_marked_not_override_overriding; - Diag(MD->getLocation(), DiagID) << MD->getDeclName(); - const CXXMethodDecl *OMD = *MD->begin_overridden_methods(); - Diag(OMD->getLocation(), diag::note_overridden_virtual_function); + auto EmitDiag = [this, MD](unsigned DiagID) { + Diag(MD->getLocation(), DiagID) << MD->getDeclName(); + const CXXMethodDecl *OMD = *MD->begin_overridden_methods(); + Diag(OMD->getLocation(), diag::note_overridden_virtual_function); + }; + if (Inconsistent) + EmitDiag( + isa(MD) + ? diag:: + warn_inconsistent_destructor_marked_not_override_overriding + : diag:: + warn_inconsistent_function_marked_not_override_overriding); + else + EmitDiag( + isa(MD) + ? diag::warn_suggest_destructor_marked_not_override_overriding + : diag::warn_suggest_function_marked_not_override_overriding); } } @@ -6749,13 +6760,10 @@ } } - 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. + if (HasOverridingMethodWithoutOverrideControl) { + bool InconsistentOverrideControl = HasMethodWithOverrideControl; for (auto *M : Record->methods()) - DiagnoseAbsenceOfOverrideControl(M); + DiagnoseAbsenceOfOverrideControl(M, InconsistentOverrideControl); } // Check the defaulted secondary comparisons after any other member functions. diff --git a/clang/test/SemaCXX/warn-suggest-destructor-override b/clang/test/SemaCXX/warn-suggest-destructor-override new file mode 100644 --- /dev/null +++ b/clang/test/SemaCXX/warn-suggest-destructor-override @@ -0,0 +1,32 @@ +// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -verify -Wsuggest-destructor-override + +class A { + public: + ~A() {} + void virtual run() {} +}; + +class B : public A { + public: + ~B() {} +}; + +class C { + public: + virtual void run() {} + virtual ~C() {} // expected-note 2{{overridden virtual function is here}} +}; + +class D : public C { + public: + void run() {} + ~D() {} + // expected-warning@-1 {{'~D' overrides a destructor but is not marked 'override'}} +}; + +class E : public C { + public: + void run() {} + virtual ~E() {} + // expected-warning@-1 {{'~E' overrides a destructor but is not marked 'override'}} +}; diff --git a/clang/test/SemaCXX/warn-suggest-override b/clang/test/SemaCXX/warn-suggest-override new file mode 100644 --- /dev/null +++ b/clang/test/SemaCXX/warn-suggest-override @@ -0,0 +1,47 @@ +// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -verify -Wsuggest-override + +class A { + public: + ~A() {} + void run() {} +}; + +class B : public A { + public: + ~B() {} + void run() {} +}; + +class C { + public: + virtual void run() {} // expected-note 2{{overridden virtual function is here}} + virtual ~C() {} // expected-note 2{{overridden virtual function is here}} +}; + +class D : public C { + public: + void run() {} + // expected-warning@-1 {{'run()' overrides a member function but is not marked 'override'}} + ~D() {} + // expected-warning@-1 {{'~D' overrides a destructor but is not marked 'override'}} +}; + +class E : public C { + public: + virtual void run() {} + // expected-warning@-1 {{'run()' overrides a member function but is not marked 'override'}} + virtual ~E() {} + // expected-warning@-1 {{'~E' overrides a destructor but is not marked 'override'}} +}; + +class F : public C { + public: + void run() override {} + ~F() override {} +}; + +class G : public C { + public: + void run() final {} + ~G() final {} +};