[Bug c++/113191] New: [10.1/11/12/13/14 Regression] Incorrect overload resolution when base class function introduced with a using declaration is more constrained than a function declared in the derived class

[Bug c++/113191] New: [10.1/11/12/13/14 Regression] Incorrect overload resolution when base class function introduced with a using declaration is more constrained than a function declared in the derived class

[waffl3x at protonmail dot com]

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

            Bug ID: 113191
           Summary: [10.1/11/12/13/14 Regression] Incorrect overload
                    resolution when base class function introduced with a
                    using declaration is more constrained than a function
                    declared in the derived class
           Product: gcc
           Version: 10.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: waffl3x at protonmail dot com
  Target Milestone: ---

https://godbolt.org/z/91esEGhj4

template<typename = void>
struct B {
  constexpr int f() requires true { return 5; }
};

template<typename = void>
struct S : B<> {
  using B::f;
  constexpr int f() { return 10; }
};

static_assert(S<>{}.f() == 5);

The bug does not occur in this case:
https://godbolt.org/z/dPM1Gfc1c

We do the right thing in more_specialized_fn (which is why the second
case works fine), but that doesn't apply in this case. Perhaps we
should be lifting that work from more_specialized_fn to joust?

Unfortunately, the changes in more_specialized_fn do not properly
handle the following case.

struct B {
  template<typename T>
    requires true
  int g(T) { return 5; }
};

struct S : B {
  using B::g;
  template<typename T>
  int g(this S&, T) { return 10; }
};

int main()
{
  S s{};
  s.g(0);
}

This case is ambiguous, I believe the main issue is that
more_specialized_fn does not implement [over.match.funcs.general.4].
This is kind of a separate bug but they are connected, and it's
relevant to how we decide to fix it. I'm mildly of the opinion that we
should be rewriting iobj member functions that are introduced with a
using declaration to have an object parameter matching that of the
class it was introduced into. This might open a can of worms, but it
more closely matches the behavior specified by the standard.

[over.match.funcs.general.4]
For non-conversion functions that are implicit object member
functions nominated by a using-declaration in a derived class, the
function is considered to be a member of the derived class for the purpose
of defining the type of the implicit object parameter.

This wasn't really as relevant before, but it does become relevant now
because of the following case.

https://godbolt.org/z/MjP5nrd8q

template<typename = void>
struct S;

template<typename = void>
struct B {
  constexpr int f(this S<> const&) { return 5; }
  constexpr int g() const { return 5; }
};

template<typename>
struct S : B<> {
  using B<>::f;
  using B<>::g;
  constexpr int f() const { return 10; }
  constexpr int g(this S const&) { return 10; }
};

inline constexpr S<> s{};
static_assert(s.f() == 5);
static_assert(s.g() == 5);

I am not 100% sure what the correct behavior here is, but my
interpretation is that the constraints should be taken into account.
Again, this is slightly unrelated to this bug report, but it's more
evidence that we should just overhaul everything with iobj member
functions, and follow the standard to the letter. I think it's going to
be simpler in the long run, trying to hack it in this way or that is
just going to keep introducing problems. With that said, I recognize
theres potential implementation difficulties with doing it this way
too. Ultimately, it's a big decision so I don't mean to declare that we
need to do it this way, I merely intend to present it as food for
thought.

My implementation currently does not do either of these correct at all,
and as you can see in the godbolt link, clang does not exhibit the
behavior I believe to be correct either.

One last note, despite this being a regression, I don't believe that
the previous implementation will be ideal (not that I've found the
divergence yet.) Previous versions had the liberty of making different
assumptions, and as demonstrated in the examples with xobj member
functions, we have some new issues we need to work around here as well.

I've spent the better part of 6 hours investigating this issue and the
issues related to it, trying to figure out how to handle it for my
patch. I have concluded that I'm not going to try to fix this bug for
xobj member functions, and instead going to wait for this bug to be
fixed to try to handle it. So the behavior for xobj member functions
and iobj member functions will both be equally incorrect. Anyway, since
I have spent so much time staring at this I might have made some
mistakes in this report, or it will just be more confusing and
disjointed than I hoped. Hopefully not though!

[Bug c++/113191] [11/12/13/14 Regression] Incorrect overload resolution when base class function introduced with a using declaration is more constrained than a function declared in the derived class

[ppalka at gcc dot gnu.org]

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

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
            Summary|[10.1/11/12/13/14           |[11/12/13/14 Regression]
                   |Regression] Incorrect       |Incorrect overload
                   |overload resolution when    |resolution when base class
                   |base class function         |function introduced with a
                   |introduced with a using     |using declaration is more
                   |declaration is more         |constrained than a function
                   |constrained than a function |declared in the derived
                   |declared in the derived     |class
                   |class                       |
   Target Milestone|---                         |11.5
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2024-01-02
      Known to work|                            |10.1.0
                 CC|                            |jason at gcc dot gnu.org,
                   |                            |ppalka at gcc dot gnu.org
      Known to fail|                            |10.5.0, 11.4.0, 12.3.0,
                   |                            |13.2.0, 14.0

--- Comment #1 from Patrick Palka <ppalka at gcc dot gnu.org> ---
We started rejecting the first testcase since r11-1571-g57b4daf8dc4ed7.

> We do the right thing in more_specialized_fn (which is why the second
case works fine), but that doesn't apply in this case. Perhaps we
should be lifting that work from more_specialized_fn to joust?

Looking at the above commit, joust already takes care to check more_constrained
for non-template functions, and only if their function parameters match
according to cand_parms_match.  But here cand_parms_match returns false due to
different implicit object parameters:

(gdb) frame
#0  cand_parms_match (c2=0x3402cc0, c1=0x3402d70) at gcc/cp/call.cc:12699
12699     if (DECL_FUNCTION_MEMBER_P (fn1)
(gdb) pct parms1

struct B *, void

(gdb) pct parms2

struct S *, void

In contrast more_specialized_fn skips over the implicit object parameter when
comparing two non-static memfns.  Maybe cand_parms_match should follow suit?

[Bug c++/113191] [11/12/13/14 Regression] Incorrect overload resolution when base class function introduced with a using declaration is more constrained than a function declared in the derived class

[waffl3x at protonmail dot com]

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

--- Comment #2 from waffl3x <waffl3x at protonmail dot com> ---
> Looking at the above commit, joust already takes care to check
> more_constrained for non-template functions, and only if their function
> parameters match according to cand_parms_match.  But here cand_parms_match
> returns false due to different implicit object parameters:

Okay yeah I definitely misunderstood something in more_specialized_fn,
what it does works but is arguably not the right thing.

> (gdb) frame
> #0  cand_parms_match (c2=0x3402cc0, c1=0x3402d70) at gcc/cp/call.cc:12699
> 12699     if (DECL_FUNCTION_MEMBER_P (fn1)
> (gdb) pct parms1
> 
> struct B *, void
> 
> (gdb) pct parms2
> 
> struct S *, void
> 
> In contrast more_specialized_fn skips over the implicit object parameter
> when comparing two non-static memfns.  Maybe cand_parms_match should follow
> suit?

We COULD do that, but it won't solve the problems with implementing the
xobj/iobj cases. With that said it probably does make more sense to
skip the object parameter when comparing two iobj member functions. Are
there ever any situations where both candidates are viable, but have
different object parameters? I'm pretty sure that will never be the
case, right?

I guess I'll have to open another PR for the xobj/iobj cases because I
think I agree with your evaluation of this.

[Bug c++/113191] [11/12/13/14 Regression] Incorrect overload resolution when base class function introduced with a using declaration is more constrained than a function declared in the derived class

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:61b493f17e6fea5a0fb45b6a050259ca326c13a7

commit r14-7157-g61b493f17e6fea5a0fb45b6a050259ca326c13a7
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Jan 9 05:15:01 2024 -0500

    c++: corresponding object parms [PR113191]

    As discussed, our handling of corresponding object parameters needed to
    handle the using-declaration case better.  And I took the opportunity to
    share code between the add_method and cand_parms_match uses.

    This patch specifically doesn't compare reversed parameters, but a
follow-up
    patch will.

            PR c++/113191

    gcc/cp/ChangeLog:

            * class.cc (xobj_iobj_parameters_correspond): Add context parm.
            (object_parms_correspond): Factor out of...
            (add_method): ...here.
            * method.cc (defaulted_late_check): Use it.
            * call.cc (class_of_implicit_object): New.
            (object_parms_correspond): Overload taking two candidates.
            (cand_parms_match): Use it.
            (joust): Check reversed before comparing constraints.
            * cp-tree.h (object_parms_correspond): Declare.

    gcc/testsuite/ChangeLog:

            * g++.dg/cpp2a/concepts-memfun4.C: New test.

[Bug c++/113191] [11/12/13/14 Regression] Incorrect overload resolution when base class function introduced with a using declaration is more constrained than a function declared in the derived class

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

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

--- Comment #4 from Jason Merrill <jason at gcc dot gnu.org> ---
Fixed for GCC 14.  I don't think this is worth trying to change on release
branches.