[Bug c++/109740] New: -Woverloaded-virtual is too aggressive

[Bug c++/109740] New: -Woverloaded-virtual is too aggressive

[psmith at gnu dot org]

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

            Bug ID: 109740
           Summary: -Woverloaded-virtual is too aggressive
           Product: gcc
           Version: 13.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: psmith at gnu dot org
  Target Milestone: ---

I understand the impetus for the -Woverloaded-virtual warning but I think it
should be further constrained, or maybe broken into levels of severity that can
be enabled.

The issue I'm running into is that a superclass has a virtual method signature
with some number of arguments, and the subclass has an overloaded method with a
different number of arguments.  For example:

  struct A { virtual void foo(int); };
  struct B : public A { void foo(); };

We do this kind of thing all the time because the subclass's version of this
method has more knowledge, or whatever, and doesn't need the extra arguments.

However, this fires the overloaded-virtual warning.  I don't think this
warrants a warning: if I call B.foo() then it will always do the right thing. 
If I call B.foo(10) it will give me an error.  There's no way that the compiler
could ever call the wrong thing "behind my back" due to some sort of type
conversion that is not obvious from the code, because the number of arguments
are different: either it does exactly what I expect and there's no need for a
warning, or I get an error and there's no need for a warning.

This subclassing capability is (IMO) useful and shouldn't be restricted, but I
would prefer to keep the warning for in other situations that could potentially
cause misbehavior.

Would it be possible to suppress this warning if the overload can't possibly
conflict due to number of arguments?

There is also the possibility that a subclass method has the same number of
arguments but they are of incompatible types which cannot be converted between.
 There is an argument to be made that this, also, shouldn't cause a warning. 
I'm not sure how straightforward it is to determine "can't be converted"
although there are some obvious ones.

[Bug c++/109740] -Woverloaded-virtual is too aggressive

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The warning is specifically designed this way and even is documented to warn.
https://gcc.gnu.org/onlinedocs/gcc-13.1.0/gcc/C_002b_002b-Dialect-Options.html#index-Woverloaded-virtual

It is designed this way so you don't accidently have the wrong arguments (types
and/or count). The documentation even talks about that:
"In cases where the different signatures are not an accident,"

[Bug c++/109740] -Woverloaded-virtual is too aggressive

[psmith at gnu dot org]

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

--- Comment #2 from Paul Smith <psmith at gnu dot org> ---
What I'm trying to say is that it's not useful (to me) for GCC to warn about
code that I could maybe write in the future but didn't actually write, and
which if I did write it would generate a compiler error anyway, rather than
"doing the wrong thing".

On the other hand, it's very useful for GCC to warn me about situations where I
could be actually getting an unexpected result, without a compiler error.  For
example if the parent method takes an int and the child method takes a char,
then I might call B.foo(10) expecting to get the parent method but actually the
child method will be invoked.  That kind of warning is very helpful.

So, it would be nice to have a way to warn about things that might miscompile
silently in unexpected ways, without also warning about things that can't
possibly miscompile in unexpected ways.

I did read the description in the docs, and the suggestion of adding using
A::foo to the child class, but that inherits all the parent class's virtual
methods into the child class which I don't want to do in all cases.

[Bug c++/109740] -Woverloaded-virtual is too aggressive

[emerg.reanimator at gmail dot com]

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

Alexander Goomenuk <emerg.reanimator at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |emerg.reanimator at gmail dot com

--- Comment #3 from Alexander Goomenuk <emerg.reanimator at gmail dot com> ---
Let consider the following case:
- There is a base class that implement virtual assignment operator with
arbitrary input argument (not copy operator).
- There is a derived class that inherits the assignment operator from base
class.

g++ version >= 13 produces the warning in this case because implicitly-defined
copy operator of derived class pretends to hide the user defined assignment
operator of base class. The code is totally fine and the assignment operator
works as expected. 

Even worse, the warning is produced by compiler even if no implicitly-defined
copy operator is NOT generated by the compiler and the code fails to build due
to the lack of copy operator. So the warning is misleading and incorrect in
this case.

See
https://stackoverflow.com/questions/77378553/woverloaded-virtual-with-default-shallow-copy-operator
for more details.

[Bug c++/109740] -Woverloaded-virtual is too aggressive

[emerg.reanimator at gmail dot com]

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

--- Comment #4 from Alexander Goomenuk <emerg.reanimator at gmail dot com> ---
https://en.cppreference.com/w/cpp/language/using_declaration

# In class definition
...
If the derived class already has a member with the same name, parameter list,
and qualifications, *the derived class member hides or overrides (doesn't
conflict with) the member that is introduced from the base class*.

----

https://gcc.gnu.org/onlinedocs/gcc-13.1.0/gcc/C_002b_002b-Dialect-Options.html#index-Woverloaded-virtual

In cases where the different signatures are not an accident, the simplest
solution is to add a *using-declaration to the derived class to un-hide the
base function*, e.g. add using A::f; to B. 

----

These two statements contradict each other, isn't it?

[Bug c++/109740] -Woverloaded-virtual is too aggressive

[emerg.reanimator at gmail dot com]

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

--- Comment #5 from Alexander Goomenuk <emerg.reanimator at gmail dot com> ---
Created attachment 56495
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56495&action=edit
Overloaded virtual testcase

Another relevant issue with gcc (GCC) 13.2.1 20231011 (Red Hat 13.2.1-4) and
-Woverloaded-virtual.

Overloading method f() in Derived class affects the warning that is caused by
assignment operator! 

Woverloaded-virtual-test.cpp:6:28: warning: ‘virtual const Parent&
Parent::operator=(int&)’ was hidden [-Woverloaded-virtual=]
    6 |     virtual const Parent & operator=(int &x)
      |                            ^~~~~~~~
Woverloaded-virtual-test.cpp:15:8: note:   by ‘constexpr Derived&
Derived::operator=(const Derived&)’
   15 | struct Derived : public Parent
      |        ^~~~~~~