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

[waffl3x <waffl3x@protonmail.com>]

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

I've posted the report here, I ended up doing a bit more investigation,
so the contents might interest you. I'm really starting to think that
we want to do a more thorough re-engineering of how using declarations
are handled, instead of handling it with little hacks like the one
added to more_specialized_fn.

As I note in the report, the addition of xobj member functions really
makes [over.match.funcs.general.4] a lot more relevant, and I don't
think we can get away with not following it more closely anymore. I
know I'm wording myself here as if that passage has existed forever,
but I recognize it might be as recent as C++23 that it was added. I
don't mean to imply anything with how I'm wording it, it's just way
easier to express it this way. Especially since we really could get
away with these kinds of hacks if xobj member functions did not exist.
Unfortunately, the type of the implicit object parameter is suddenly
relevant for cases like this.

Anyway, as I've stated a few times, I'm going to implement my function
that checks correspondence of the object parameter of iobj and xobj
member functions assuming that iobj member functions introduced by
using declarations are handled properly. I think that's the best option
for my patch right now.

Well that investigation took the majority of my day. I'm just glad I'm
certain of what direction to take now.

Alex


On Monday, January 1st, 2024 at 8:34 AM, waffl3x <waffl3x@protonmail.com> wrote:


> 
> 
> That was faster than I expected, the problem is exactly just that we
> aren't implementing [over.match.funcs.general.4] at all. The result of
> compparms for the 2 functions is false which I believe to be wrong. I
> believe we have a few choices here, but no matter what we go with it
> will be a bit of an overhaul. I will post a PR on bugzilla in a little
> bit as this problem feels somewhat out of the scope of my patch now.
> 
> I think what I will do is instead of comparing the xobj parameter to
> the DECL_CONTEXT of the xobj function, I will compare it to the type of
> the iobj member function's object parameter. If I do it like this, it
> will work as expected if we rewrite functions that are introduced with
> a using declaration.
> 
> This might still cause problems, I will look into how the this pointer
> for iobj member functions is determined again. Depending on how it is
> determined, it might be possible to change the function signature of
> iobj member functions without altering their behavior. It would be
> incorrect, and would change the meaning of code, if changing the
> function signature changed the type of the this pointer.
> 
> Anyhow, this is a fairly big change to consider so I won't pretend I
> know what the right call is. But the way I've decided to implement
> correspondence checking will be consistent with how GCC currently
> (incorrectly) treats constraints on iobj member functions introduced
> with a using declaration, so I think doing it this way is the right
> choice for now.
> 
> Some days feel really unproductive when the majority is investigation
> and thinking. This was one of them, but at least I'm confident that my
> conclusions are correct. Aren't edge cases fun?
> 
> Alex
> 
> On Monday, January 1st, 2024 at 8:17 AM, waffl3x waffl3x@protonmail.com wrote:
> 
> 
> 
> > 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