<div dir="ltr">Cool!<div><br></div><div>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.)</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Oct 27, 2014 at 12:11 PM, Fariborz Jahanian <span dir="ltr"><<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: fjahanian<br>
Date: Mon Oct 27 14:11:51 2014<br>
New Revision: 220703<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=220703&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=220703&view=rev</a><br>
Log:<br>
c++11 patch to issue warning on missing 'override' on<br>
overriding methods. Patch review by Richard Smith.<br>
rdar://18295240<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Basic/DiagnosticGroups.td<br>
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
cfe/trunk/include/clang/Sema/Sema.h<br>
cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>
cfe/trunk/test/Parser/MicrosoftExtensions.cpp<br>
cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=220703&r1=220702&r2=220703&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=220703&r1=220702&r2=220703&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Oct 27 14:11:51 2014<br>
@@ -147,6 +147,8 @@ def CXX98CompatPedantic : DiagGroup<"c++<br>
<br>
def CXX11Narrowing : DiagGroup<"c++11-narrowing">;<br>
<br>
+def CXX11WarnOverrideMethod : DiagGroup<"inconsistent-missing-override">;<br>
+<br>
// Original name of this warning in Clang<br>
def : DiagGroup<"c++0x-narrowing", [CXX11Narrowing]>;<br>
<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=220703&r1=220702&r2=220703&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=220703&r1=220702&r2=220703&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Oct 27 14:11:51 2014<br>
@@ -1691,6 +1691,9 @@ def override_keyword_hides_virtual_membe<br>
"%select{function|functions}1">;<br>
def err_function_marked_override_not_overriding : Error<<br>
"%0 marked 'override' but does not override any member functions">;<br>
+def warn_function_marked_not_override_overriding : Warning <<br>
+ "%0 overrides a member function but is not marked 'override'">,<br>
+ InGroup<CXX11WarnOverrideMethod>;<br>
def err_class_marked_final_used_as_base : Error<<br>
"base %0 is marked '%select{final|sealed}1'">;<br>
def warn_abstract_final_class : Warning<<br>
<br>
Modified: cfe/trunk/include/clang/Sema/Sema.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=220703&r1=220702&r2=220703&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=220703&r1=220702&r2=220703&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Sema/Sema.h (original)<br>
+++ cfe/trunk/include/clang/Sema/Sema.h Mon Oct 27 14:11:51 2014<br>
@@ -5138,6 +5138,10 @@ public:<br>
<br>
/// CheckOverrideControl - Check C++11 override control semantics.<br>
void CheckOverrideControl(NamedDecl *D);<br>
+<br>
+ /// DiagnoseAbsenceOfOverrideControl - Diagnose if 'override' keyword was<br>
+ /// not used in the declaration of an overriding method.<br>
+ void DiagnoseAbsenceOfOverrideControl(NamedDecl *D);<br>
<br>
/// CheckForFunctionMarkedFinal - Checks whether a virtual member function<br>
/// overrides a virtual member function marked 'final', according to<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=220703&r1=220702&r2=220703&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=220703&r1=220702&r2=220703&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Oct 27 14:11:51 2014<br>
@@ -1897,6 +1897,22 @@ void Sema::CheckOverrideControl(NamedDec<br>
<< MD->getDeclName();<br>
}<br>
<br>
+void Sema::DiagnoseAbsenceOfOverrideControl(NamedDecl *D) {<br>
+ if (D->isInvalidDecl() || D->hasAttr<OverrideAttr>())<br>
+ return;<br>
+ CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D);<br>
+ if (!MD || MD->isImplicit() || MD->hasAttr<FinalAttr>() ||<br>
+ isa<CXXDestructorDecl>(MD))<br>
+ return;<br>
+<br>
+ if (MD->size_overridden_methods() > 0) {<br>
+ Diag(MD->getLocation(), diag::warn_function_marked_not_override_overriding)<br>
+ << MD->getDeclName();<br>
+ const CXXMethodDecl *OMD = *MD->begin_overridden_methods();<br>
+ Diag(OMD->getLocation(), diag::note_overridden_virtual_function);<br>
+ }<br>
+}<br>
+<br>
/// CheckIfOverriddenFunctionIsMarkedFinal - Checks whether a virtual member<br>
/// function overrides a virtual member function marked 'final', according to<br>
/// C++11 [class.virtual]p4.<br>
@@ -4763,13 +4779,18 @@ void Sema::CheckCompletedCXXClass(CXXRec<br>
}<br>
}<br>
<br>
+ bool HasMethodWithOverrideControl = false,<br>
+ HasOverridingMethodWithoutOverrideControl = false;<br>
if (!Record->isDependentType()) {<br>
for (auto *M : Record->methods()) {<br>
// See if a method overloads virtual methods in a base<br>
// class without overriding any.<br>
if (!M->isStatic())<br>
DiagnoseHiddenVirtualMethods(M);<br>
-<br>
+ if (M->hasAttr<OverrideAttr>())<br>
+ HasMethodWithOverrideControl = true;<br>
+ else if (M->size_overridden_methods() > 0)<br>
+ HasOverridingMethodWithoutOverrideControl = true;<br>
// Check whether the explicitly-defaulted special members are valid.<br>
if (!M->isInvalidDecl() && M->isExplicitlyDefaulted())<br>
CheckExplicitlyDefaultedSpecialMember(M);<br>
@@ -4788,6 +4809,13 @@ void Sema::CheckCompletedCXXClass(CXXRec<br>
}<br>
}<br>
<br>
+ if (HasMethodWithOverrideControl &&<br>
+ HasOverridingMethodWithoutOverrideControl) {<br>
+ // At least one method has the 'override' control declared.<br>
+ // Diagnose all other overridden methods which do not have 'override' specified on them.<br>
+ for (auto *M : Record->methods())<br>
+ DiagnoseAbsenceOfOverrideControl(M);<br>
+ }<br>
// C++11 [dcl.constexpr]p8: A constexpr specifier for a non-static member<br>
// function that is not a constructor declares that member function to be<br>
// const. [...] The class of which that function is a member shall be<br>
<br>
Modified: cfe/trunk/test/Parser/MicrosoftExtensions.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/MicrosoftExtensions.cpp?rev=220703&r1=220702&r2=220703&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/MicrosoftExtensions.cpp?rev=220703&r1=220702&r2=220703&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Parser/MicrosoftExtensions.cpp (original)<br>
+++ cfe/trunk/test/Parser/MicrosoftExtensions.cpp Mon Oct 27 14:11:51 2014<br>
@@ -208,12 +208,12 @@ extern TypenameWrongPlace<AAAA> PR16925;<br>
<br>
__interface MicrosoftInterface;<br>
__interface MicrosoftInterface {<br>
- void foo1() = 0;<br>
+ void foo1() = 0; // expected-note {{overridden virtual function is here}}<br>
virtual void foo2() = 0;<br>
};<br>
<br>
__interface MicrosoftDerivedInterface : public MicrosoftInterface {<br>
- void foo1();<br>
+ void foo1(); // expected-warning {{'foo1' overrides a member function but is not marked 'override'}}<br>
void foo2() override;<br>
void foo3();<br>
};<br>
<br>
Modified: cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp?rev=220703&r1=220702&r2=220703&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp?rev=220703&r1=220702&r2=220703&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp Mon Oct 27 14:11:51 2014<br>
@@ -372,14 +372,15 @@ struct SomeBase {<br>
<br>
// expected-note@+2 {{overridden virtual function is here}}<br>
// expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}<br>
- virtual void SealedFunction() sealed;<br>
+ virtual void SealedFunction() sealed; // expected-note {{overridden virtual function is here}}<br>
};<br>
<br>
// expected-note@+2 {{'SealedType' declared here}}<br>
// expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}<br>
struct SealedType sealed : SomeBase {<br>
- // expected-error@+1 {{declaration of 'SealedFunction' overrides a 'sealed' function}}<br>
- virtual void SealedFunction();<br>
+ // expected-error@+2 {{declaration of 'SealedFunction' overrides a 'sealed' function}}<br>
+ // FIXME. warning can be suppressed if we're also issuing error for overriding a 'final' function.<br>
+ virtual void SealedFunction(); // expected-warning {{'SealedFunction' overrides a member function but is not marked 'override'}}<br>
<br>
// expected-warning@+1 {{'override' keyword is a C++11 extension}}<br>
virtual void OverrideMe() override;<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>