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,12 @@ 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">; // 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 @@ -6961,7 +6961,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,22 @@ 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 = [&](unsigned DiagInconsistent, unsigned DiagSuggest) { + unsigned DiagID = + Inconsistent && !Diags.isIgnored(DiagInconsistent, MD->getLocation()) + ? DiagInconsistent + : DiagSuggest; + Diag(MD->getLocation(), DiagID) << MD->getDeclName(); + const CXXMethodDecl *OMD = *MD->begin_overridden_methods(); + Diag(OMD->getLocation(), diag::note_overridden_virtual_function); + }; + if (isa(MD)) + EmitDiag( + diag::warn_inconsistent_destructor_marked_not_override_overriding, + diag::warn_suggest_destructor_marked_not_override_overriding); + else + EmitDiag(diag::warn_inconsistent_function_marked_not_override_overriding, + diag::warn_suggest_function_marked_not_override_overriding); } } @@ -6749,13 +6759,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 HasInconsistentOverrideControl = HasMethodWithOverrideControl; for (auto *M : Record->methods()) - DiagnoseAbsenceOfOverrideControl(M); + DiagnoseAbsenceOfOverrideControl(M, HasInconsistentOverrideControl); } // 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,27 @@ +// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -verify -Wsuggest-destructor-override + +struct A { + ~A(); + virtual void run(); +}; + +struct B : public A { + ~B(); +}; + +struct C { + virtual void run(); + virtual ~C(); // expected-note 2{{overridden virtual function is here}} +}; + +struct D : public C { + void run(); + ~D(); + // expected-warning@-1 {{'~D' overrides a destructor but is not marked 'override'}} +}; + +struct E : public C { + 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,38 @@ +// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -verify -Wsuggest-override + +struct A { + ~A(); + void run(); +}; + +struct B : public A { + ~B(); + void run(); +}; + +struct C { + virtual void run(); // expected-note 2{{overridden virtual function is here}} + virtual ~C(); +}; + +struct D : public C { + void run(); + // expected-warning@-1 {{'run()' overrides a member function but is not marked 'override'}} + ~D(); +}; + +struct E : public C { + virtual void run(); + // expected-warning@-1 {{'run()' overrides a member function but is not marked 'override'}} + virtual ~E(); +}; + +struct F : public C { + void run() override; + ~F() override; +}; + +struct G : public C { + void run() final; + ~G() final; +};