from:"Josh Gao"

[PATCH] Add diagnostic to require that virtual methods be tagged with C++11 'override'

2014-09-24 Thread Josh Gao

Hi all,

This patch adds a diagnostic that warns when virtual methods that override are 
not tagged 'override'.

commit c33643ab4f12148ed6cdc5a8a5b0be282c7b0451
Author: Josh Gao 
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.

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.
+
 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 %, but is not virtual", decl);
   if (DECL_OVERRIDE_P (decl) && !overrides_found)
 error ("%q+#D marked %, 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 %", decl);
 }
 
 /* Warn about hidden virtual functions that are not overridden in t.
diff --git a/gcc/testsuite/g++.dg/cpp0x/override5.C 
b/gcc/testsuite/g++.dg/cpp0x/override5.C
new file mode 100644
index 000..07131e2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/override5.C
@@ -0,0 +1,36 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wmissing-virtual-override" }
+
+struct B
+{
+  virtual ~B();
+  virtual void f() final {}
+  virtual void x() {}
+};
+
+struct D : B
+{
+  virtual void x() final {} // { dg-warning "overrides, but is not marked 
'override'" }
+};
+
+template  struct D2 : T
+{
+  void x() override {}
+};
+
+template  struct D3 : D2
+{
+  void x() {} // { dg-warning "overrides, but is not marked 'override'" }
+};
+
+template  struct D4 : D3
+{
+  void x() override {}
+};
+
+int main()
+{
+  D2 d2;
+  D3 d3;
+  D4 d4;
+}

Re: [PATCH] Add diagnostic to require that virtual methods be tagged with C++11 'override'

2014-09-24 Thread Josh Gao


On Sep 24, 2014, at 1:09 PM, Marek Polacek  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 
>> 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 %, but is not virtual", decl);
>>   if (DECL_OVERRIDE_P (decl) && !overrides_found)
>> error ("%q+#D marked %, 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 %", 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 
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  
+* doc/invoke.texi (-Wmissing-virtual-override): Add.
+
 2014-09-24  Jan Hubicka  
 
* 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  
+   * c.opt: Add -Wmissing-virtual-override.
+
 2014-09-24  Marek Polacek  
 
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  
+   * class.c: Add -Wmissing-virtual-override.
+
 2014-09-24  Aldy Hernandez  
 
* 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)

[PATCH] Add diagnostic to require that virtual methods be tagged with C++11 'override'

Re: [PATCH] Add diagnostic to require that virtual methods be tagged with C++11 'override'

2 matches

Site Navigation

Mail list logo

Footer information