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

[Jason Merrill <jason@redhat.com>]

On 9/19/23 20:30, waffl3x wrote:
>> 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.

That would make sense.

>>> +/* 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.

That's a good point, but the flag you chose seems even more general purpose.

A better option might be, instead of putting this flag on the PARM_DECL, 
to put it on the short-lived TREE_LIST which is only used for 
communication between cp_parser_parameter_declaration_list and 
grokparms, and have grokdeclarator grab it from 
declarator->u.function.parameters?

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

Generally the flags that aren't specifically specified to be 
language-specific are reserved for language-independent uses; even if 
only one front-end actually uses the feature, it should be for 
communication to language-independent code rather than communication 
within the particular front-end.  The patch modified tree-core.h to 
refer to a macro in cp-tree.h.

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

Ah, oops, I assumed that v2 completely replaced v1.

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

Yes, please.

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

Sounds good!

Jason