Re: [PATCH v7 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]

[waffl3x <waffl3x@protonmail.com>]

I've been at this for a while, and I'm not sure what the proper way to
fix this is.

```
struct S;

struct B {
  void f(this S&) {}
  void g() {}
};

struct S : B {
  using B::f;
  using B::g;
  void f() {}
  void g(this S&) {}
};

int main()
{
  S s{};
  s.f();
  s.g();
}
```

In short, the call to f is ambiguous, but the call to g is not. I
already know where the problem is, but since I share this code in
places that don't know about whether a function was introduced by a
using declaration (cand_parms_match), I don't want to rely on that to
solve the problem.

```
  /* An iobj member function's object parameter can't be an unrelated type, if
     the xobj member function's object parameter is an unrelated type we know
     they must not correspond to each other.  If the iobj member function was
     introduced with a using declaration, then the type of its object parameter
     is still that of the class we are currently adding a member function to,
     so this assumption holds true in that case as well.

     [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.

     We don't get to bail yet out even if the xobj parameter is by-value as
     elaborated on below.

     This also implicitly handles xobj parameters of type pointer.  */
  if (DECL_CONTEXT (xobj_fn) != TYPE_MAIN_VARIANT (non_reference (xobj_param)))
    return false;
```

I feel like what we are actually supposed to be doing to be to the
letter of the standard is to be creating a new function entirely, with
a decl_context of the original class, which sounds omega silly, and
might bring a new set of problems.

I think I might have came up with an unfortunately fairly convoluted
way to solve this just now, but I don't know if it brings another new
set of problems. The assumptions I had when I originally implemented
this in add_method bled through when I broke it out into it's own
function. At the very least I need to better document how the function
is intended to be used, at worst I'll need to consider whether it makes
sense to be reusing this logic if the use cases are subtly different.

I don't think the latter is the case now though, I'm noticing GCC just
has a bug in general with constraints and using declarations.

https://godbolt.org/z/EbGvjfG7E

So it might actually just be better to be rewriting functions that are
introduced by using declarations, I have a feeling that will be what
introduces the least pain.

I'm not sure where exactly GCC is deciding that a function introduced
by a using declaration is different from an otherwise corresponding one
declared directly in that class, but I have a feeling on where it is.
Obviously it's in joust, but I'm not sure the object parameters are
actually being compared.

I'll investigate this bug and get back to you, I imagine fixing it is
going to be key to actually implementing the xobj case without hacks.

Finding both these issues has slowed down my next revision as I noticed
the problem while cleaning up my implementation of CWG2789. I want to
note, I am implementing it as if it specifies corresponding object
arguments, not object arguments of the same type, as we previously
discussed, I believe that to be the right resolution as there are
really bad edge cases with the current wording.

Alex

On Tuesday, December 26th, 2023 at 9:37 AM, Jason Merrill <jason@redhat.com> wrote:


> 
> 
> On 12/23/23 02:10, waffl3x wrote:
> 
> > On Friday, December 22nd, 2023 at 10:26 AM, Jason Merrill jason@redhat.com wrote:
> > 
> > > On 12/22/23 04:01, waffl3x wrote:
> > > 
> > > > int n = 0;
> > > > auto f = []<typename Self>(this Self){
> > > > static_assert(__is_same (decltype(n), int));
> > > > decltype((n)) a; // { dg-error {is not captured} }
> > > > };
> > > > f();
> > > > 
> > > > Could you clarify if this error being removed was intentional. I do
> > > > recall that Patrick Palka wanted to remove this error in his patch, but
> > > > it seemed to me like you stated it would be incorrect to allow it.
> > > > Since the error is no longer present I assume I am misunderstanding the
> > > > exchange.
> > > > 
> > > > In any case, let me know if I need to modify my test case or if this
> > > > error needs to be added back in.
> > > 
> > > Removing the error was correct under
> > > https://eel.is/c++draft/expr.prim#id.unqual-3
> > > Naming n in that lambda would not refer a capture by copy, so the
> > > decltype is the same as outside the lambda.
> > 
> > Alright, I've fixed my tests to reflect that.
> > 
> > I've got defaulting assignment operators working. Defaulting equality
> > and comparison operators seemed to work out of the box somehow, so I
> > just have to make some fleshed out tests for those cases.
> > 
> > There can always be more tests, I have a few ideas for what still needs
> > to be covered, mostly with dependent lambdas. Tests for xobj conversion
> > operators definitely need to be more fleshed out. I also need to
> > formulate some tests to make sure constraints are not being taking into
> > account when the object parameters should not correspond, but that's a
> > little more tough to test for than the valid cases.
> > 
> > Other than tests though, is there anything you can think of that the
> > patch is missing? Other than the aforementioned tests, I'm pretty
> > confident everything is done.
> > 
> > To recap, I have CWG2789 implemented on my end with the change we
> > discussed to require corresponding object parameters instead of the
> > same type, and I have CWG2586 implemented. I can't recall what other
> > outstanding issues we had, and my notes don't mention anything other
> > than tests. So I'm assuming everything is good.
> 
> 
> Sounds good! Did you mean to include the updated patch?
> 
> Jason