Bug 20423

Hmm, ICC gives:
t.cc(7): warning #654: overloaded virtual function "Foo::Func" is only partially overridden in class "Baz"
  class Baz: public Foo
        ^

t.cc(14): error #165: too few arguments in function call
    b->Func(1);
             ^

compilation aborted for t.cc (code 2)

The warning is correct because Func(int) is still hidden in the supper class and you cannot use it:
class Foo
{
public: virtual void Func(int);
        virtual void Func(int, int);
};

class Baz: public Foo
{
public: virtual void Func(int, int);
};

void g(Baz *b)
{
  b->Func(1);
}


Maybe we should do the warning which ICC is giving instead of the one which we give right now.

I think there a two different situations:

1) in the derived class you define a method with the same name but
   different parameters. 

This is typically a serious bug in your program which can result in the 
exectution of the code of the wrong (not overloaded) function.


2) in the derived class you define not all overloaded variants. 

In that case the not overloaded variants are hidden but no run-time-error 
occurs. If you try to call a hidden variant, the compiler complains.


If "-Woverloaded-virtual" only triggers in case 1, it could be enabled
with "-Wall" as it typically indicates a severe bug in your program.

If there would be a "-Wpartial-overloaded-virtual" you could enable if you like 
it and you can. I have to work with a large existing code basis where I would 
not enable it.

I think to give different messages like ICC is the way to go.

Michael Cieslinski

*** Bug 20683 has been marked as a duplicate of this bug. ***

Agreed with the reporter's analysis

I am looking at this but it is a bit hard to see how to make the distinction between the two kinds. I don't want to introduce yet another loop in warn_hidden but perhaps that is unavoidable. Any hints?

A really wild-guess patch. Comments?


Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c      (revision 121953)
+++ gcc/cp/class.c      (working copy)
@@ -2377,6 +2377,8 @@ warn_hidden (tree t)
       tree binfo;
       int j;

+      bool just_hidden = false;
+
       /* All functions in this slot in the CLASSTYPE_METHOD_VEC will
         have the same name.  Figure out what name that is.  */
       name = DECL_NAME (OVL_CURRENT (fns));
@@ -2408,8 +2410,14 @@ warn_hidden (tree t)
                /* If the method from the base class has the same
                   signature as the method from the derived class, it
                   has been overridden.  */
-               if (same_signature_p (fndecl, TREE_VALUE (*prev)))
-                 *prev = TREE_CHAIN (*prev);
+               if (same_signature_p (fndecl, TREE_VALUE (*prev)))
+                 {
+                   *prev = TREE_CHAIN (*prev);
+                   /* If at least one method has the same signature,
+                      the not overloaded variants are just
+                      hidden.  */
+                   just_hidden = true;
+                 }
                else
                  prev = &TREE_CHAIN (*prev);
            }
@@ -2419,9 +2427,17 @@ warn_hidden (tree t)
         as they are hidden.  */
       while (base_fndecls)
        {
-         /* Here we know it is a hider, and no overrider exists.  */
-         warning (0, "%q+D was hidden", TREE_VALUE (base_fndecls));
-         warning (0, "  by %q+D", fns);
+         /* If Here we know it is a hider, and no overrider exists.  */
+         if (just_hidden)
+           {
+             warning (OPT_Wpartial_overloaded_virtual, "%q+D was hidden", TREE_VALUE (base_fndecls));
+             warning (OPT_Wpartial_overloaded_virtual, "  by %q+D", fns);
+           }                                                                                           
+         else
+           {
+             warning (OPT_Woverloaded_virtual, "%q+D was hidden", TREE_VALUE (base_fndecls));
+             warning (OPT_Woverloaded_virtual, "  by %q+D", fns);
+           }
          base_fndecls = TREE_CHAIN (base_fndecls);
        }
     }

Any progress on this? 

This warning could be really useful if only 1) would be handled. In its current state I can't use it as I get too many "false" positives :-(

An old patch: http://gcc.gnu.org/ml/gcc-patches/1999-03n/msg00182.html

(In reply to comment #0)
> 
> This is also not conforming to the "specification" in
> http://gcc.gnu.org/ml/gcc-bugs/1999-08n/msg01069.html
> 
>   Warn when a derived class function declaration may be an error in
>   defining a virtual function.  In a derived class, the definitions of
>   virtual functions must match the type signature of a virtual
>   function declared in the base class.  With this option, the compiler
>   warns when you define a function with the same name as a virtual
>   function, but with a type signature that does not match any
>   declarations from the base class.

But that's not what it does. The description in the manual describes the current behaviour correctly, that description above doesn't.

(In reply to comment #6)
> A really wild-guess patch. Comments?
...
> +                   /* If at least one method has the same signature,
> +                      the not overloaded variants are just
> +                      hidden.  */
> +                   just_hidden = true;

Shouldn't this say "not overridden variants" ?

I don't like the name -Wpartial-overloaded-virtual for the same reason. The name should clearly distinguish whether it is triggered by overloading, overriding or hiding.

-Woverloaded-virtual is a reasonable name for the current behaviour; it triggers if there is an overload that hides a virtual (whether the overload is itself virtual or not.)  The proposed -Wpartial-overloaded-virtual name makes no sense.

as with bug 31937, there is overlap between this enhancement and the 'Explicit Virtual Function Overrides' paper, http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2009/n2928.htm

the attributes proposed in that paper allow the derived class to explicitly state whether a function should override or hide the same name in the base class, making it ill-formed if there is unintended hiding

Not working on this now.

The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:113844d68e94f4e9c0e946db351ba7d3d4a1335a

commit r13-1262-g113844d68e94f4e9c0e946db351ba7d3d4a1335a
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Jun 24 14:40:12 2022 -0400

    c++: Include -Woverloaded-virtual in -Wall [PR87729]
    
    This seems like a good warning to have in -Wall, as requested.  But as
    pointed out in PR20423, some users want a warning only when a derived
    function doesn't override any base function.  So let's put that lesser
    version in -Wall (and -Woverloaded-virtual=1) while leaving the semantics
    for the existing option the same.
    
            PR c++/87729
            PR c++/20423
    
    gcc/c-family/ChangeLog:
    
            * c.opt (Woverloaded-virtual): Add levels, include in -Wall.
    
    gcc/ChangeLog:
    
            * doc/invoke.texi: Document changes.
    
    gcc/cp/ChangeLog:
    
            * class.cc (warn_hidden): Handle -Woverloaded-virtual=1.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/warn/Woverloaded-virt1.C: New test.
            * g++.dg/warn/Woverloaded-virt2.C: New test.

For GCC 13 the requested semantics will be available in -Woverloaded-virtual=1 or -Wall.