Bug 109918 - [12/13/14/15 Regression] Unexpected -Woverloaded-virtual with virtual conversion operators
Summary: [12/13/14/15 Regression] Unexpected -Woverloaded-virtual with virtual convers...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 14.0
: P2 normal
Target Milestone: 15.0
Assignee: Simon Martin
URL:
Keywords: diagnostic
Depends on: 117114 117117
Blocks:
  Show dependency treegraph
 
Reported: 2023-05-20 18:28 UTC by Romain Geissler
Modified: 2025-02-04 10:05 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 15.0, 7.1.0, 7.5.0
Known to fail: 8.1.0
Last reconfirmed: 2023-05-20 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Romain Geissler 2023-05-20 18:28:04 UTC
Hi,

Now that the -Woverloaded-virtual=1 is enabled by default with -Wall, the following code raises warning while I think it should not (for the specific case of conversion operators):

#include <iostream>

struct A
{
    virtual operator int() { return 42; }

    virtual operator char() = 0;
};

struct B : public A
{
    operator char() override { return 'A'; }
};

int main()
{
    B b;

    std::cout << static_cast<int>(b) << std::endl; // int conversion was not hiden, contrary to what -Woverloaded-virtual claims
    std::cout << static_cast<char>(b) << std::endl;
}


Godbolt link: https://godbolt.org/z/4Wb9rxbMP

Compiled with -Wall, it raises this warning:

<source>:5:13: warning: 'virtual A::operator int()' was hidden [-Woverloaded-virtual=]
    5 |     virtual operator int() { return 42; }
      |             ^~~~~~~~
<built-in>: note:   by 'operator'
ASM generation compiler returned: 0
<source>:5:13: warning: 'virtual A::operator int()' was hidden [-Woverloaded-virtual=]
    5 |     virtual operator int() { return 42; }
      |             ^~~~~~~~
<built-in>: note:   by 'operator'
Execution build compiler returned: 0
Program returned: 0
42
A


I have hit this issue in a real code base, while migrating to gcc 13.

Cheers,
Romain
Comment 1 Andrew Pinski 2023-05-20 18:41:12 UTC
Confirmed. GCC 7.5 didn't warn with -Woverloaded-virtual but GCC 8 does.

Maybe introduced by r8-2977-g940ab2e08e2414 .
Comment 2 Richard Biener 2023-07-07 10:45:22 UTC
GCC 10 branch is being closed.
Comment 3 Martin Jambor 2024-01-10 17:17:51 UTC
The testcase with -Werror=overloaded-virtual started failing with commit r8-2669-gbff8b385e997a8 (Nathan Sidwell: Conversion operators have a special name).
Comment 4 Richard Biener 2024-07-19 13:20:28 UTC
GCC 11 branch is being closed.
Comment 5 Simon Martin 2024-08-23 14:17:32 UTC
Working on this one.
Comment 6 GCC Commits 2024-10-12 07:51:33 UTC
The master branch has been updated by Simon Martin <simartin@gcc.gnu.org>:

https://gcc.gnu.org/g:60163c85730e6b7c566e219222403ac87ddbbddd

commit r15-4282-g60163c85730e6b7c566e219222403ac87ddbbddd
Author: Simon Martin <simon@nasilyan.com>
Date:   Fri Oct 11 10:16:26 2024 +0200

    c++: Fix overeager Woverloaded-virtual with conversion operators [PR109918]
    
    We currently emit an incorrect -Woverloaded-virtual warning upon the
    following test case
    
    === cut here ===
    struct A {
      virtual operator int() { return 42; }
      virtual operator char() = 0;
    };
    struct B : public A {
      operator char() { return 'A'; }
    };
    === cut here ===
    
    The problem is that when iterating over ovl_range (fns), warn_hidden
    gets confused by the conversion operator marker, concludes that
    seen_non_override is true and therefore emits a warning for all
    conversion operators in A that do not convert to char, even if
    -Woverloaded-virtual is 1 (e.g. with -Wall, the case reported).
    
    A second set of problems is highlighted when -Woverloaded-virtual is 2.
    
    First, with the same test case, since base_fndecls contains all
    conversion operators in A (except the one to char, that's been removed
    when iterating over ovl_range (fns)), we emit a spurious warning for
    the conversion operator to int, even though it's unrelated.
    
    Second, in case there are several conversion operators with different
    cv-qualifiers to the same type in A, we rightfully emit a warning,
    however the note uses the location of the conversion operator marker
    instead of the right one; location_of should go over conv_op_marker.
    
    This patch fixes all these by explicitly keeping track of (1) base
    methods that are overriden, as well as (2) base methods that are hidden
    but not overriden (and by what), and warning about methods that are in
    (2) but not (1). It also ignores non virtual base methods, per
    "definition" of -Woverloaded-virtual.
    
            PR c++/109918
    
    gcc/cp/ChangeLog:
    
            * class.cc (warn_hidden): Keep track of overloaded and of hidden
            base methods. Mention the actual hiding function in the warning,
            not the first overload.
            * error.cc (location_of): Skip over conv_op_marker.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/warn/Woverloaded-virt1.C: Check that no warning is
            emitted for non virtual base methods.
            * g++.dg/warn/Woverloaded-virt5.C: New test.
            * g++.dg/warn/Woverloaded-virt6.C: New test.
            * g++.dg/warn/Woverloaded-virt7.C: New test.
            * g++.dg/warn/Woverloaded-virt8.C: New test.
            * g++.dg/warn/Woverloaded-virt9.C: New test.
Comment 7 Simon Martin 2024-10-12 07:53:09 UTC
Fixed on trunk.
Comment 8 Jonathan Wakely 2024-10-12 10:09:12 UTC
As this is a regression, it would usually be fixed on all open branches. Was closing it without backports intentional?
Comment 9 Andreas Schwab 2024-10-13 13:47:19 UTC
This breaks building the rust compiler.

In file included from ../../gcc/rust/typecheck/rust-tyty.h:28,
                 from ../../gcc/rust/backend/rust-mangle.h:21,
                 from ../../gcc/rust/backend/rust-mangle.cc:19:
../../gcc/rust/typecheck/rust-tyty-subst.h:408:21: error: 'virtual Rust::TyTy::BaseType* Rust::TyTy::SubstitutionRef::handle_substitions(Rust::TyTy::Substitut\
ionArgumentMappings&)' was hidden [-Werror=overloaded-virtual=]
  408 |   virtual BaseType *handle_substitions (SubstitutionArgumentMappings &mappings)
      |                     ^~~~~~~~~~~~~~~~~~
../../gcc/rust/typecheck/rust-tyty.h:761:3: note:   by 'virtual Rust::TyTy::ADTType* Rust::TyTy::ADTType::handle_substitions(Rust::TyTy::SubstitutionArgumentMappings&)'
  761 |   handle_substitions (SubstitutionArgumentMappings &mappings) override final;
      |   ^~~~~~~~~~~~~~~~~~
../../gcc/rust/typecheck/rust-tyty-subst.h:408:21: error: 'virtual Rust::TyTy::BaseType* Rust::TyTy::SubstitutionRef::handle_substitions(Rust::TyTy::SubstitutionArgumentMappings&)' was hidden [-Werror=overloaded-virtual=]
  408 |   virtual BaseType *handle_substitions (SubstitutionArgumentMappings &mappings)
      |                     ^~~~~~~~~~~~~~~~~~
../../gcc/rust/typecheck/rust-tyty.h:873:3: note:   by 'virtual Rust::TyTy::FnType* Rust::TyTy::FnType::handle_substitions(Rust::TyTy::SubstitutionArgumentMappings&)'
  873 |   handle_substitions (SubstitutionArgumentMappings &mappings) override final;
      |   ^~~~~~~~~~~~~~~~~~
../../gcc/rust/typecheck/rust-tyty-subst.h:408:21: error: 'virtual Rust::TyTy::BaseType* Rust::TyTy::SubstitutionRef::handle_substitions(Rust::TyTy::SubstitutionArgumentMappings&)' was hidden [-Werror=overloaded-virtual=]
  408 |   virtual BaseType *handle_substitions (SubstitutionArgumentMappings &mappings)
      |                     ^~~~~~~~~~~~~~~~~~
../../gcc/rust/typecheck/rust-tyty.h:1033:3: note:   by 'virtual Rust::TyTy::ClosureType* Rust::TyTy::ClosureType::handle_substitions(Rust::TyTy::SubstitutionArgumentMappings&)'
 1033 |   handle_substitions (SubstitutionArgumentMappings &mappings) override final;
      |   ^~~~~~~~~~~~~~~~~~
../../gcc/rust/typecheck/rust-tyty-subst.h:408:21: error: 'virtual Rust::TyTy::BaseType* Rust::TyTy::SubstitutionRef::handle_substitions(Rust::TyTy::SubstitutionArgumentMappings&)' was hidden [-Werror=overloaded-virtual=]
  408 |   virtual BaseType *handle_substitions (SubstitutionArgumentMappings &mappings)
      |                     ^~~~~~~~~~~~~~~~~~
../../gcc/rust/typecheck/rust-tyty.h:1576:3: note:   by 'virtual Rust::TyTy::ProjectionType* Rust::TyTy::ProjectionType::handle_substitions(Rust::TyTy::SubstitutionArgumentMappings&)'
 1576 |   handle_substitions (SubstitutionArgumentMappings &mappings) override final;
      |   ^~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
make[3]: *** [../../gcc/rust/Make-lang.in:433: rust/rust-mangle.o] Error 1
Comment 10 Sam James 2024-10-13 13:59:12 UTC
(In reply to Andreas Schwab from comment #9)
> This breaks building the rust compiler.
> 

That's PR117114.
Comment 11 Simon Martin 2024-10-13 16:04:20 UTC
Reopening since this broke the rust build and I don't have an immediate fix.
Comment 12 Simon Martin 2024-10-13 16:05:25 UTC
Revert done via r15-4301-ga4eec6c712ec16
Comment 13 Simon Martin 2025-01-03 09:48:00 UTC
Patch submitted in https://gcc.gnu.org/pipermail/gcc-patches/2024-October/665650.html
Comment 14 GCC Commits 2025-02-04 10:02:52 UTC
The master branch has been updated by Simon Martin <simartin@gcc.gnu.org>:

https://gcc.gnu.org/g:d346af2af88caf1e21a5cc1f0afa33596198fb60

commit r15-7350-gd346af2af88caf1e21a5cc1f0afa33596198fb60
Author: Simon Martin <simon@nasilyan.com>
Date:   Tue Feb 4 10:58:17 2025 +0100

    c++: Fix overeager Woverloaded-virtual with conversion operators [PR109918]
    
    We currently emit an incorrect -Woverloaded-virtual warning upon the
    following test case
    
    === cut here ===
    struct A {
      virtual operator int() { return 42; }
      virtual operator char() = 0;
    };
    struct B : public A {
      operator char() { return 'A'; }
    };
    === cut here ===
    
    The problem is that when iterating over ovl_range (fns), warn_hidden
    gets confused by the conversion operator marker, concludes that
    seen_non_override is true and therefore emits a warning for all
    conversion operators in A that do not convert to char, even if
    -Woverloaded-virtual is 1 (e.g. with -Wall, the case reported).
    
    A second set of problems is highlighted when -Woverloaded-virtual is 2.
    
    First, with the same test case, since base_fndecls contains all
    conversion operators in A (except the one to char, that's been removed
    when iterating over ovl_range (fns)), we emit a spurious warning for
    the conversion operator to int, even though it's unrelated.
    
    Second, in case there are several conversion operators with different
    cv-qualifiers to the same type in A, we rightfully emit a warning,
    however the note uses the location of the conversion operator marker
    instead of the right one; location_of should go over conv_op_marker.
    
    This patch fixes all these by explicitly keeping track of (1) base
    methods that are overriden, as well as (2) base methods that are hidden
    but not overriden (and by what), and warning about methods that are in
    (2) but not (1). It also ignores non virtual base methods, per
    "definition" of -Woverloaded-virtual.
    
    Co-authored-by: Jason Merrill <jason@redhat.com>
    
            PR c++/117114
            PR c++/109918
    
    gcc/cp/ChangeLog:
    
            * class.cc (warn_hidden): Keep track of overloaded and of hidden
            base methods.
            * error.cc (location_of): Skip over conv_op_marker.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/warn/Woverloaded-virt1.C: Check that no warning is
            emitted for non virtual base methods.
            * g++.dg/warn/Woverloaded-virt10.C: New test.
            * g++.dg/warn/Woverloaded-virt11.C: New test.
            * g++.dg/warn/Woverloaded-virt12.C: New test.
            * g++.dg/warn/Woverloaded-virt13.C: New test.
            * g++.dg/warn/Woverloaded-virt5.C: New test.
            * g++.dg/warn/Woverloaded-virt6.C: New test.
            * g++.dg/warn/Woverloaded-virt7.C: New test.
            * g++.dg/warn/Woverloaded-virt8.C: New test.
            * g++.dg/warn/Woverloaded-virt9.C: New test.
Comment 15 Simon Martin 2025-02-04 10:05:47 UTC
Fixed in GCC 15.