[Bug c++/31397] New: Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[Bug c++/31397] New: Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[Thomas dot Lange at sun dot com]

When trying to build the OOo code warning-free we turned all useful warnings on
and get rid of them.
But there is one warning that would be really useful missing. It is not
required for code correctness or safety at all, but it would be most useful to
have better understandable code.

What I/we at OOo would like to have is a warning when a when a function in a
derived class is overloaded without specifing 'virtual'. 

This would allow to identify all the cases where this is missing and in the end
it would be clear just be looking at the specific declaration of a class which
functions are virtual and which not. Right now in our sometimes very old code
it is quite troublesome to always go up in the class hierarchy in order to
check this.

Example:
class A
{
  A();
  virtual ~A();
  virtual int f( int x );
};

class B : public A
{
  B( int y );
  ~B();
  int f( int x );
};

It would be nice if there would be warnings for ~B and f not using the
'virtual' keyword. Together with the switch to treat warnings as errors this
would help to produce better understandable code.


-- 
           Summary: Useful compiler warning missing (virtual functions in
                    derived classes used without 'virtual')
           Product: gcc
           Version: 3.4.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: Thomas dot Lange at sun dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[fang at csl dot cornell dot edu]

------- Comment #1 from fang at csl dot cornell dot edu  2007-03-31 00:43 -------
Do you mean -Woverloaded-virtual?  (see man page)
The diagostic reports when a derived class's method 'hides' the base class's.  


-- 

fang at csl dot cornell dot edu changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fang at csl dot cornell dot
                   |                            |edu


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[bangerth at dealii dot org]

------- Comment #2 from bangerth at dealii dot org  2007-04-08 00:58 -------
(In reply to comment #1)
> Do you mean -Woverloaded-virtual?  (see man page)
> The diagostic reports when a derived class's method 'hides' the base class's.  

No, he simply wants to know that "it doesn't matter from a semantic point,
but just to make the declaration clearer, why don't you add a 'virtual'
to your destructor and int f(int) functions, because they are implicitly
virtual anyway'.

Confirmed.

W.


-- 

bangerth at dealii dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bangerth at dealii dot org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
           Keywords|                            |diagnostic
   Last reconfirmed|0000-00-00 00:00:00         |2007-04-08 00:58:22
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[Thomas dot Lange at sun dot com]

------- Comment #3 from Thomas dot Lange at sun dot com  2009-07-08 10:23 -------
(In reply to comment #2)
> why don't you add a 'virtual'
> to your destructor and int f(int) functions, because they are implicitly
> virtual anyway'.

That is exactly the point this is about!
I want a way so the compiler enforces to add 'virtual' to ~B and B::f, and thus
help to write better understandable code.

This is in order for large derived trees or a big source code. It is for the
one looking at the declaration (and not anymore or not at all familiar with the
code!) to not miss that those functions are virtual.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[Thomas dot Lange at sun dot com]

------- Comment #4 from Thomas dot Lange at sun dot com  2009-07-08 10:30 -------
Side note: Of course having such an option is much more useful where the
declaration of class A and B are in different header files and probably even in
different modules. 
(For example: such cases are often found in the OpenOffice.org source code...
^_-)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[jwakely dot gcc at gmail dot com]

------- Comment #5 from jwakely dot gcc at gmail dot com  2009-07-13 11:48 -------
(In reply to comment #0)
> 
> What I/we at OOo would like to have is a warning when a when a function in a
> derived class is overloaded without specifing 'virtual'. 

To avoid further misunderstanding: you mean overridden, not overloaded.

This seems like the sort of thing that could be put in a plugin or other static
analysis tool rather than the compiler, because it warns about a preference in
coding style.


-- 

jwakely dot gcc at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jwakely dot gcc at gmail dot
                   |                            |com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[Thomas dot Lange at sun dot com]

------- Comment #6 from Thomas dot Lange at sun dot com  2009-07-13 11:56 -------
(In reply to comment #5)

No. I do mean overloaded!

It might be nice to have a warning for overloading virtual functions of base
classes as well. But my point is that the compiler should help to enforce that
every virtual function has the 'virtual' keyword set in its declaration.
Thus effectively removing those implicit virtual functions that are allowed in
C++. 
The goal is: Either a function should be explicitly declared virtual in its
declaration or not be virtual at all.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[rguenth at gcc dot gnu dot org]

------- Comment #7 from rguenth at gcc dot gnu dot org  2009-07-13 11:59 -------
Your example is overriding A::f, not overloading it.  Overloading would be

  int f(int x, int y);

do you want a warning for that as well?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[Thomas dot Lange at sun dot com]

------- Comment #8 from Thomas dot Lange at sun dot com  2009-07-13 12:02 -------
(In reply to comment #6)

Ooops... Sorry!
I just was told that I confused those two terms. >_<
(That might happen to non-native speakers)
My apologies!
Yes you are correct. It is about overwriting.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[Thomas dot Lange at sun dot com]

------- Comment #9 from Thomas dot Lange at sun dot com  2009-07-13 12:07 -------
(In reply to comment #6)
I'm not concerned about that case.

Thank you for your time!


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[jwakely dot gcc at gmail dot com]

------- Comment #10 from jwakely dot gcc at gmail dot com  2009-07-13 16:46 -------
(In reply to comment #8)
> 
> Ooops... Sorry!
> I just was told that I confused those two terms. >_<
> (That might happen to non-native speakers)
> My apologies!
> Yes you are correct. It is about overwriting.

Overriding :-)

I'm using the terminology of the C++ standard, but I do have the advantage that
it's written in my native language


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[jwakely dot gcc at gmail dot com]

------- Comment #11 from jwakely dot gcc at gmail dot com  2009-07-21 10:51 -------
There is a lot of overlap between this warning and the functionality described
by http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2009/n2852.html (which
will be updated by N2928 in a couple of weeks.)

It might make sense to add attributes rather than a -W switch, as that might
make it easier to implement the C++0x [[base_check]] and [[override]]
attributes.  It would also be easier for user code to transition from GNU-style
__attribute__((override)) to C++0x-style [[override]]


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[jwakely dot gcc at gmail dot com]

------- Comment #12 from jwakely dot gcc at gmail dot com  2009-08-11 14:16 -------
revised 'Explicit Virtual Function Overrides' paper:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2009/n2928.htm


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

--- Comment #13 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-06-21 16:12:42 UTC ---
G++ 4.7 implements C++0x explicit override control so you can add 'override'

class B : public A
{
  B( int y );
  ~B();
  int f( int x ) override;
};

and you'll get an error if it doesn't actually override a virtual function from
a base class.

It's not *quite* what you want, but you could use it to check whether B::f
overrides a virtual (and so you want to add 'virtual' to it) without checking
the declaration of A::f

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[ville.voutilainen at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

Ville Voutilainen <ville.voutilainen at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ville.voutilainen at gmail
                   |                            |dot com

--- Comment #14 from Ville Voutilainen <ville.voutilainen at gmail dot com> 2011-07-09 12:54:40 UTC ---
"[[base_check]]" or "explicit" on a class might've been helpful for this, but
even those facilities were never supposed to require annotations on
destructors.
The committee ended up removing that part because there were grammar problems
for "new" on members, and we didn't want to add base_check/explicit that
would check only overrides.

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[ethouris at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

Michal Malecki <ethouris at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ethouris at gmail dot com

--- Comment #15 from Michal Malecki <ethouris at gmail dot com> ---
But it would be nice to add a warning, valid in C++11 mode only, when a method
is overridden without 'override' keyword, or an overloaded method is defined,
which does not override without explicit 'new'. For example,
-Wimplicit-override. Additionally, if [[base_check]] has been withdrawn, the
same idea can be additionally added as __attribute__((warn_implicit_override))
(or, in C++11, [[gnu::warn_implicit_override]]).

I don't think forcing a 'virtual' keyword would do things any better -
'virtual' can always mean starting a new virtual method in this class, which
does not override anything.

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[tbsaunde at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

--- Comment #16 from tbsaunde at gcc dot gnu.org ---
Author: tbsaunde
Date: Tue Jan  6 02:02:47 2015
New Revision: 219213

URL: https://gcc.gnu.org/viewcvs?rev=219213&root=gcc&view=rev
Log:
implement -Wsuggest-override

c-family/

    PR c++/31397
    * c.opt (Wsuggest-override): New option.

cp/

    PR c++/31397
    * class.c (check_for_override): Warn when a virtual function is an
    override not marked override.

gcc/

    PR c++/31397
    * doc/invoke.texi: Document -Wsuggest-override.

Added:
    trunk/gcc/testsuite/g++.dg/warn/Wsuggest-override.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c.opt
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/class.c
    trunk/gcc/doc/invoke.texi

[Bug c++/31397] Useful compiler warning missing (virtual functions in derived classes used without 'virtual')

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=31397

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED
   Target Milestone|---                         |5.0

--- Comment #17 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed in GCC 5 with -Wsuggest-override.