On Sep 24, 2014, at 1:09 PM, Marek Polacek <pola...@redhat.com> wrote:
> On Wed, Sep 24, 2014 at 12:59:17PM -0700, Josh Gao wrote:
>> Hi all,
>>
>> This patch adds a diagnostic that warns when virtual methods that override
>> are not tagged 'override'.
>>
>> commit c33643ab4f12148ed6cdc5a8a5b0be282c7b0451
>> Author: Josh Gao <jmg...@gmail.com>
>> Date: Wed Sep 24 12:29:39 2014 -0700
>>
>> Add diagnostic to require virtual methods to be tagged override.
>>
>> * c-family/c.opt: Add -Wmissing-virtual-override.
>> * cp/class.c: Implement -Wmissing-virtual-override.
>> * testsuite/g++.dg/cpp0x/override5.C: New test.
>
> Please drop the c-family/, cp/ and testsuite/ prefixes; these should
> go into ChangeLogs in appropriate subdirectories.
>
>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>> index 72ac2ed..db350d7 100644
>> --- a/gcc/c-family/c.opt
>> +++ b/gcc/c-family/c.opt
>> @@ -558,6 +558,10 @@ Wmissing-field-initializers
>> C ObjC C++ ObjC++ Var(warn_missing_field_initializers) Warning
>> EnabledBy(Wextra)
>> Warn about missing fields in struct initializers
>>
>> +Wmissing-virtual-override
>> +C++ ObjC++ Var(warn_missing_virtual_override) Warning
>> +Warn about missing \"override\" specifier in virtual methods that override.
>
> No dot at the end here.
>
>> Wsizeof-pointer-memaccess
>> C ObjC C++ ObjC++ Var(warn_sizeof_pointer_memaccess) Warning LangEnabledBy(C
>> ObjC C++ ObjC++,Wall)
>> Warn about suspicious length parameters to certain string functions if the
>> argument uses sizeof
>> diff --git a/gcc/cp/class.c b/gcc/cp/class.c
>> index 010ed25..f44cb82 100644
>> --- a/gcc/cp/class.c
>> +++ b/gcc/cp/class.c
>> @@ -2773,6 +2773,8 @@ check_for_override (tree decl, tree ctype)
>> error ("%q+#D marked %<final%>, but is not virtual", decl);
>> if (DECL_OVERRIDE_P (decl) && !overrides_found)
>> error ("%q+#D marked %<override%>, but does not override", decl);
>> + if (!DECL_OVERRIDE_P (decl) && overrides_found &&
>> !DECL_DESTRUCTOR_P(decl))
>> + warning (OPT_Wmissing_virtual_override, "%q+#D overrides, but is not
>> marked %<override%>", decl);
>
> These lines are too long, please wrap them. Also add a space before
> (.
>
> Please describe the new option in doc/invoke.texi.
>
> Marek
Sorry, I was cargoculting the commit message (this is my first time
contributing to gcc). Does this look better?
-Josh
--
commit b4d15d3cf660708dca088361801ebd56e018f986
Author: Josh Gao <j...@mobileiron.com>
Date: Wed Sep 24 14:01:16 2014 -0700
Add diagnostic to require virtual methods to be tagged override.
gcc/Changelog
* doc/invoke.texi (-Wmissing-virtual-override): Add.
gcc/c-family
* c.opt: Add -Wmissing-virtual-override.
gcc/cp
* class.c: Add -Wmissing-virtual-override.
gcc/testsuite
* g++.dg/cpp0x/override5.C: New test.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 666f1a6..9392e7b 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,6 @@
+2014-09-24 Josh Gao <jmg...@gmail.com>
+ * doc/invoke.texi (-Wmissing-virtual-override): Add.
+
2014-09-24 Jan Hubicka <hubi...@ucw.cz>
* ipa-utils.h (polymorphic_call_context): Add
diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index 2278e77..56ec987 100644
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,6 @@
+2014-09-24 Josh Gao <jmg...@gmail.com>
+ * c.opt: Add -Wmissing-virtual-override.
+
2014-09-24 Marek Polacek <pola...@redhat.com>
PR c/61405
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 72ac2ed..69597be 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -558,6 +558,10 @@ Wmissing-field-initializers
C ObjC C++ ObjC++ Var(warn_missing_field_initializers) Warning
EnabledBy(Wextra)
Warn about missing fields in struct initializers
+Wmissing-virtual-override
+C++ ObjC++ Var(warn_missing_virtual_override) Warning
+Warn about missing \"override\" specifier in virtual methods that override
+
Wsizeof-pointer-memaccess
C ObjC C++ ObjC++ Var(warn_sizeof_pointer_memaccess) Warning LangEnabledBy(C
ObjC C++ ObjC++,Wall)
Warn about suspicious length parameters to certain string functions if the
argument uses sizeof
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index ee5169f..a0934c0 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,6 @@
+2014-09-24 Josh Gao <jmg...@gmail.com>
+ * class.c: Add -Wmissing-virtual-override.
+
2014-09-24 Aldy Hernandez <al...@redhat.com>
* class.c, decl.c, optimize.c: Rename all instances of
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index c4ac61b..68a3052 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -2773,6 +2773,9 @@ check_for_override (tree decl, tree ctype)
error ("%q+#D marked %<final%>, but is not virtual", decl);
if (DECL_OVERRIDE_P (decl) && !overrides_found)
error ("%q+#D marked %<override%>, but does not override", decl);
+ if (!DECL_OVERRIDE_P (decl) && overrides_found && !DECL_DESTRUCTOR_P (decl))
+ warning (OPT_Wmissing_virtual_override,
+ "%q+#D overrides, but is not marked %<override%>", decl);
}
/* Warn about hidden virtual functions that are not overridden in t.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index eae4ab1..eedf7c0 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -2624,6 +2624,12 @@ In this case, @code{PRId64} is treated as a separate
preprocessing token.
This warning is enabled by default.
+@item -Wmissing-virtual-override @r{(C++ and Objective-C++ only)}
+@opindex Wmissing-virtual-override
+@opindex Wnomissing-virtual-override
+Warn when a virtual method that overrides is not tagged with the C++11 override
+keyword.
+
@item -Wnarrowing @r{(C++ and Objective-C++ only)}
@opindex Wnarrowing
@opindex Wno-narrowing
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 0c3e082..8220b45 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,6 @@
+2014-09-24 Josh Gao <jmg...@gmail.com>
+ * g++.dg/cpp0x/override5.C: New test.
+
2014-09-24 Bill Schmidt <wschm...@linux.vnet.ibm.com>
* gcc.target/powerpc/swaps-p8-17.c: New test.
