r220703 - c++11 patch to issue warning on missing 'override' on
Nico Weber
thakis at chromium.org
Tue Oct 28 14:56:20 PDT 2014
Cool!
Would it be possible to emit a fixit for inserting override for the cases
where the method isn't from a macro expansion? That would make it easy to
automatically insert override everywhere it's missing. (I think there's
some function somewhere already to find of "override" is usually spelled
"OVERRIDE" in the codebase, in case the code's using override through a
macro.)
On Mon, Oct 27, 2014 at 12:11 PM, Fariborz Jahanian <fjahanian at apple.com>
wrote:
> Author: fjahanian
> Date: Mon Oct 27 14:11:51 2014
> New Revision: 220703
>
> URL: http://llvm.org/viewvc/llvm-project?rev=220703&view=rev
> Log:
> c++11 patch to issue warning on missing 'override' on
> overriding methods. Patch review by Richard Smith.
> rdar://18295240
>
> 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/Parser/MicrosoftExtensions.cpp
> cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=220703&r1=220702&r2=220703&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Oct 27 14:11:51
> 2014
> @@ -147,6 +147,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=220703&r1=220702&r2=220703&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Oct 27
> 14:11:51 2014
> @@ -1691,6 +1691,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=220703&r1=220702&r2=220703&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Mon Oct 27 14:11:51 2014
> @@ -5138,6 +5138,10 @@ public:
>
> /// CheckOverrideControl - Check C++11 override control semantics.
> void CheckOverrideControl(NamedDecl *D);
> +
> + /// DiagnoseAbsenceOfOverrideControl - Diagnose if 'override' keyword
> was
> + /// not used in the declaration of an overriding method.
> + void DiagnoseAbsenceOfOverrideControl(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=220703&r1=220702&r2=220703&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Oct 27 14:11:51 2014
> @@ -1897,6 +1897,22 @@ void Sema::CheckOverrideControl(NamedDec
> << MD->getDeclName();
> }
>
> +void Sema::DiagnoseAbsenceOfOverrideControl(NamedDecl *D) {
> + if (D->isInvalidDecl() || D->hasAttr<OverrideAttr>())
> + return;
> + CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D);
> + if (!MD || MD->isImplicit() || MD->hasAttr<FinalAttr>() ||
> + isa<CXXDestructorDecl>(MD))
> + return;
> +
> + if (MD->size_overridden_methods() > 0) {
> + Diag(MD->getLocation(),
> diag::warn_function_marked_not_override_overriding)
> + << MD->getDeclName();
> + 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.
> @@ -4763,13 +4779,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->size_overridden_methods() > 0)
> + HasOverridingMethodWithoutOverrideControl = true;
> // Check whether the explicitly-defaulted special members are valid.
> if (!M->isInvalidDecl() && M->isExplicitlyDefaulted())
> CheckExplicitlyDefaultedSpecialMember(M);
> @@ -4788,6 +4809,13 @@ 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())
> + DiagnoseAbsenceOfOverrideControl(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/Parser/MicrosoftExtensions.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/MicrosoftExtensions.cpp?rev=220703&r1=220702&r2=220703&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Parser/MicrosoftExtensions.cpp (original)
> +++ cfe/trunk/test/Parser/MicrosoftExtensions.cpp Mon Oct 27 14:11: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/SemaCXX/MicrosoftExtensions.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp?rev=220703&r1=220702&r2=220703&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp (original)
> +++ cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp Mon Oct 27 14:11:51 2014
> @@ -372,14 +372,15 @@ 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();
> + // expected-error at +2 {{declaration of 'SealedFunction' overrides a
> 'sealed' function}}
> + // FIXME. warning can be suppressed if we're also issuing error for
> overriding a 'final' function.
> + 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;
>
>
> _______________________________________________
> 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/20141028/26593af8/attachment.html>
More information about the cfe-commits
mailing list