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

[waffl3x <waffl3x@protonmail.com>]

> > 
> > Based on what you've said, I assume that OFFSET_REF handles static
> > member functions that are overloaded. But as I've said this seems to
> > contradict the comments I'm reading, so I'm not sure that I'm
> > understanding you correctly.
> 
> 
> That's right. For instance,
> 
> struct A {
> static void g();
> static void g(int);
> };
> 
> void (*p)(int) = &A::g; // cp_build_addr_expr_1 gets an OFFSET_REF
> 
> > > > I think we need the OFFSET_REF for an explicit-object member function
> > > > because it expresses that the code satisfies the requirement "If the
> > > > operand names an explicit object member function, the operand shall be a
> > > > qualified-id."
> > 
> > I do agree here, but it does reinforce that OFFSET_REF is no longer
> > just for members represented by pointer to member type. So that might
> > be something to take into consideration.
> 
> 
> An OFFSET_REF that isn't type_unknown_p, agreed.
> 
> > > > It might simplify things to remove the optimization in build_offset_ref
> > > > so we get an OFFSET_REF even for a single static member function, and
> > > > add support for that to cp_build_addr_expr_1.
> > 
> > I don't think this should be necessary, the "right thing" should just
> > be done for explicit-object member functions. With all the stuff going
> > on here that I missed I'm starting to wonder how function overloads
> > ever worked at all in my patch. On the other hand though, this
> > optimization probably could be documented better, but I very well might
> > have missed it even if it were.
> > 
> > Hell, maybe it needs a greater redesign altogether, it seems strange to
> > me to bundle overload information in with a construct for a specific
> > expression. (Assuming that's whats happening of course, I still don't
> > fully understand it.) It's not like this has rules unique to it for how
> > overload resolution is decided, right? Initializing a param/variable of
> > pointer to function type with an overloaded function resolves that with
> > similar rules, I think? Maybe it is a little different now that I write
> > it out loud.
> > 
> > I wasn't going to finish my musings about that, but it made me realize
> > that it might not actually be correct for address of explicit-object
> > member functions to be wrapped by OFFSET_REF. I mean surely it's fine
> > because based on what you've said static member functions are also
> > wrapped by OFFSET_REF, so it's likely fully implemented, especially
> > considering things worked before. But now that there are 2 different
> > varieties of class members that the address of them can be taken, it
> > might make sense to split things up a bit? Then again, why were static
> > member functions ever handled the same way? Taking the address of other
> > static members isn't handled in the same way here is it?
> 
> 
> Functions are different because of overloading; in general we can't
> decide what an expression that names a function actually means until we
> have enough context to decide which function, exactly. So we represent
> the id-expression largely as lookup+syntax until overload resolution
> turns it into a specific function. The type_unknown_p check earlier in
> cp_build_addr_expr_1 is for that case.

Yeah this all makes sense, but that's why I was confused by the
following documentation from cp-tree.def.

```
/* An OFFSET_REF is used in two situations:

   1. An expression of the form `A::m' where `A' is a class and `m' is
      a non-static member.  In this case, operand 0 will be a TYPE
      (corresponding to `A') and operand 1 will be a FIELD_DECL,
      BASELINK, or TEMPLATE_ID_EXPR (corresponding to `m').

      The expression is a pointer-to-member if its address is taken,
      but simply denotes a member of the object if its address is not
      taken.
```

> An OFFSET_REF that isn't type_unknown_p, agreed.
But I suppose that's what this might have been referring to.

So is that the case then? OFFSET_REF might be for a regular address of
member expression unless it's type_unknown_p?

> 
> An id-expression that names a single non-template function
> (!really_overloaded_fn) is handled somewhat differently, as we don't
> need to defer everything. But that means various special-case code.
> 
> Currently build_offset_ref special-cases &A::f for a single static
> member function, but we can't use the same special case for single
> explicit object member functions because we need to distinguish between
> &A::f and &f somehow to check the requirement I quoted above. So it
> seems to me we'll need to add support for single explicit object member
> functions in the OFFSET_REF handling in cp_build_addr_expr_1. And I
> thought if we're doing that, perhaps we want to move the single static
> handling over there as well, but that's not necessary.
> 
> Jason

I'm done for today but it does seem like that special case is what is
causing my crash. I found that it only jumps into the section that it
crashes in when there are no overloads. I'm kinda close to fixing it
probably but I've spent too long on it for today, it's melting together.

> Currently build_offset_ref special-cases &A::f for a single static
> member function, but we can't use the same special case for single
> explicit object member functions because we need to distinguish between
> &A::f and &f somehow to check the requirement I quoted above.

I don't understand what this means exactly, under what circumstances
would &f find the member function. Oh, I guess while in the body of
it's class, I hadn't considered that. Is that what you're referring to?

Well either way, I'm going to pick back up here tomorrow and see if I
can finish figuring out exactly whats causing the problems. I'm pretty
certain it's the special case that causes it, I'm just not sure what
part exactly is causing the difference, and whether there aren't more
hiding issues. But for now I need to get some rest. Thanks for the
detailed responses, they helped a lot, this part of the code has been
particularly difficult to figure out.

Alex