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

[waffl3x <waffl3x@protonmail.com>]

> Thank you, this is great!

Thanks!

> One legal hurdle to start with: our DCO policy
> (https://gcc.gnu.org/dco.html) requires real names in the sign-off, not
> pseudonyms. If you would prefer to contribute under this pseudonym, I
> encourage you to file a copyright assignment with the FSF, who are set
> up to handle that.

I will get on that right away.

> > +/* These need to moved to somewhere appropriate. */
> 
> 
> This isn't a bad spot for these macros, but you could also move them
> down lower, maybe near DECL_THIS_STATIC and DECL_ARRAY_PARAMETER_P for
> some thematic connection.

Sounds good, I will move them down.

> > +/* The flag is a member of base, but the value is meaningless for other
> > + decl types so checking is still justified I imagine. */
> 
> 
> Absolutely, we often reuse bits for other purposes if they're disjoint
> from the use they were added for.

Would it be more appropriate to give it a general name in base instead
then? If so, I can also change that.

> > +/* Not a lang_decl field, but still specific to c++. */
> > +#define DECL_PARM_XOBJ_FLAG(NODE) \
> > + (PARM_DECL_CHECK (NODE)->decl_common.decl_flag_3)
> 
> 
> Better to use a DECL_LANG_FLAG than claim one of the
> language-independent flags for C++.
> 
> There's a list at the top of cp-tree.h of the uses of LANG_FLAG on
> various kinds of tree node. DECL_LANG_FLAG_4 seems free on PARM_DECL.

Okay, I will switch to that instead, I didn't like using such a general
purpose flag for what is only relevant until the FUNC_DECL is created
and then never again.

If you don't mind answering right now, what are the consequences of
claiming language-independent flags for C++? Or to phrase it
differently, why would this be claiming it for C++? My guess was that
those flags could be used by any front ends and there wouldn't be any
conflicts, as you can't really have crossover between two front ends at
the same time. Or is that the thing, that kind of cross-over is
actually viable and claiming a language independent flag inhibits that
possibility? Like I eluded to, this is kinda off topic from the patch
so feel free to defer the answer to someone else but I just want to
clear up my understanding for the future.

> > + /* Only used for skipping over build_memfn_type, grokfndecl handles
> > + copying the flag to the correct field for a func_decl.
> > + There must be a better way to do this, but it isn't obvious how. */
> > + bool is_xobj_member_function = false;
> > + auto get_xobj_parm = [](tree parm_list)
> 
> 
> I guess you could add a flag to the declarator, but this is fine too.
> Though I'd move this lambda down into the cdk_function case or out to a
> separate function.

Okay, I will move the lambda.

> > case cdk_function:
> > {
> > + tree xobj_parm
> > + = get_xobj_parm (declarator->u.function.parameters);
> > + is_xobj_member_function = xobj_parm;
> 
> 
> I'd also move these down a few lines after the setting of 'raises'.

Will do.
Also, I forgot to mention it anywhere, the diagnostic patch utilizes
xobj_parm which is why it's a separate variable.

> > + /* Set the xobj flag for this parm, unfortunately
> > + I don't think there is a better way to do this. */
> > + DECL_PARM_XOBJ_FLAG (decl)
> > + = decl_spec_seq_has_spec_p (declspecs, ds_this);
> 
> 
> This seems like a fine way to handle this.

Okay good, I had my doubt's there.
> > + /* Special case for xobj parm, doesn't really belong up here
> > + (it applies to parm decls and those are mostly handled below
> > + the following specifiers) but I intend to refactor this function
> > + so I'm not worrying about it too much.
> > + The error diagnostics might be better elsewhere though. */
> 
> 
> This seems like a reasonable place for it since 'this' is supposed to
> precede the decl-specifiers, and since we are parsing initial attributes
> here rather than in the caller. You will want to give an error if
> found_decl_spec is set. And elsewhere complain about 'this' on
> parameters after the first (in cp_parser_parameter_declaration_list?),
> or in a non-member/lambda (in grokdeclarator?).
> 
> Jason

Yeah, I separated all the diagnostics out into the second patch. This
patch was meant to include the bare minimum of what was necessary to
get the feature functional. As for the diagnostics patch, I'm not happy
with how scattered about the code base it is, but you'll be able to
judge for yourself when I resubmit that patch, hopefully later today.
So not to worry, I didn't neglect diagnostics, it's just in a follow
up. The v1 of it was submitted on August 31st if you want to find it,
but I wouldn't recommend it. I misunderstood how some things were to be
formatted so it's probably best you just wait for me to finish a v2 of
it.

One last thing, I assume I should clean up the comments and replace
them with more typical ones right? I'm going to go forward with that
assumption in v3, I just want to mention it in advanced just in case I
have the wrong idea.

I will get started on v3 of this patch and v2 of the diagnostic patch
as soon as I have the ball rolling on legal stuff. I should have it all
finished tonight. Thanks for the detailed response, it cleared up a lot
of my doubts.

Alex