Hi all,
We have an enhancement request from our users to provide
warning if ‘override’ control is missing. This warning is off by default as it
will
be noisy (but it will show itself with -Weverything).
Is such a patch useful enough to go into the trunk? Also, comment on the patch
is appreciated.
I will provide ‘fixit’ later if this is a worthwhile patch.
- Fariborz
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td (revision 218382)
+++ include/clang/Basic/DiagnosticGroups.td (working copy)
@@ -146,6 +146,8 @@
def CXX11Narrowing : DiagGroup<"c++11-narrowing">;
+def CXX11WarnOverrideMethod :
DiagGroup<"warn-missing-override-on-overriding-method">;
+
// Original name of this warning in Clang
def : DiagGroup<"c++0x-narrowing", [CXX11Narrowing]>;
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td (revision 218382)
+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -1683,6 +1683,9 @@
"%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 is not marked 'override' but overrides a member functions">,
+ InGroup<CXX11WarnOverrideMethod>, DefaultIgnore;
def err_class_marked_final_used_as_base : Error<
"base %0 is marked '%select{final|sealed}1'">;
def warn_abstract_final_class : Warning<
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h (revision 218382)
+++ include/clang/Sema/Sema.h (working copy)
@@ -5070,6 +5070,10 @@
/// 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
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp (revision 218382)
+++ lib/Sema/SemaDeclCXX.cpp (working copy)
@@ -1895,6 +1895,27 @@
<< MD->getDeclName();
}
+void Sema::DiagnoseAbsenseOfOverrideControl(NamedDecl *D) {
+ if (D->isInvalidDecl())
+ return;
+ CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D);
+ if (!MD ||
Diags.isIgnored(diag::warn_function_marked_not_override_overriding,
+ MD->getLocation()))
+ return;
+ bool HasOverriddenMethods =
+ MD->begin_overridden_methods() != MD->end_overridden_methods();
+ if (HasOverriddenMethods) {
+ Diag(MD->getLocation(), diag::warn_function_marked_not_override_overriding)
+ << MD->getDeclName();
+ 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;
+ }
+ }
+}
+
/// CheckIfOverriddenFunctionIsMarkedFinal - Checks whether a virtual member
/// function overrides a virtual member function marked 'final', according to
/// C++11 [class.virtual]p4.
@@ -2182,6 +2203,9 @@
CheckOverrideControl(Member);
+ if (LangOpts.CPlusPlus11 && !Member->hasAttr<OverrideAttr>())
+ DiagnoseAbsenseOfOverrideControl(Member);
+
assert((Name || isInstField) && "No identifier for non-field ?");
if (isInstField) {
Index: test/SemaCXX/cxx11-warn-missing-override.cpp
===================================================================
--- test/SemaCXX/cxx11-warn-missing-override.cpp (revision 0)
+++ test/SemaCXX/cxx11-warn-missing-override.cpp (working copy)
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions
-Wwarn-missing-override-on-overriding-method -verify -std=c++11 %s
+// rdar://18295240
+
+struct A
+{
+ virtual void foo(); // expected-note {{overridden virtual function is
here}}
+ void bar();
+ virtual void gorf() {} // expected-note {{overridden virtual function is
here}}
+ virtual void g() = 0; // expected-note {{overridden virtual function is
here}}
+};
+
+struct B : A
+{
+ void foo(); // expected-warning {{'foo' is not marked 'override' but
overrides a member functions}}
+ void bar();
+};
+
+struct C : B
+{
+ virtual void gorf() {} // expected-warning {{'gorf' is not marked
'override' but overrides a member functions}}
+ virtual void g() = 0; // expected-warning {{'g' is not marked 'override'
but overrides a member functions}}
+};
Index: test/SemaCXX/virtuals.cpp
===================================================================
--- test/SemaCXX/virtuals.cpp (revision 218382)
+++ test/SemaCXX/virtuals.cpp (working copy)
@@ -1,8 +1,9 @@
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions
-Wwarn-missing-override-on-overriding-method -verify -std=c++11 %s
class A {
virtual void f();
- virtual void g() = 0; // expected-note{{unimplemented pure virtual method
'g' in 'A'}}
+ virtual void g() = 0; // expected-note{{unimplemented pure virtual method
'g' in 'A'}} \
+ // expected-note {{overridden virtual function is here}}
void h() = 0; // expected-error {{'h' is not virtual and cannot be declared
pure}}
void i() = 1; // expected-error {{initializer on function does not look like
a pure-specifier}}
@@ -19,7 +20,7 @@
class B : public A {
// Needs to recognize that overridden function is virtual.
- void g() = 0;
+ void g() = 0; // expected-warning {{'g' is not marked 'override' but
overrides a member functions}}
// Needs to recognize that function does not override.
void g(int) = 0; // expected-error{{'g' is not virtual and cannot be
declared pure}}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
