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

[waffl3x <waffl3x@protonmail.com>]

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

Bringing this back up, I recalled another detail regarding this.

I'm pretty sure that found_decl_spec can be false when parsing the
second or latter decl-specifier. I tested it quickly and I believe I am
correct. I raise this as my diagnostics patch introduces another
variable to track whether we are on the first decl-specifier, given the
results of my quick test, I believe that was the correct choice.

This kinda unclear machinery is what makes me really want to refactor
this code, but I've resisted as it would be inappropriate to try to do
so while implementing a feature. Once I am finished implementing
`deducing this` would you be open to me refactoring grokdeclarator and
it's various auxiliary functions?

As for where the complaining happens, I believe I implemented this
particular error in cp_parser_decl_specifier_seq, I don't plan to be
stubborn on any of the diagnostic code though as I'm pretty unhappy
with how it got scattered about. I intend to get more input on that
after I finish v2 of the diagnostic patch though.


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

Yeah, I had to just settle on it because I was bikeshedding it for a
couple hours despite being very unhappy with it.

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

That does sound ideal! I will look into doing it this way.

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

Ah okay, that makes perfect sense to me, understood.

> The patch modified tree-core.h to
> refer to a macro in cp-tree.h.

Yeah, I wasn't sure about doing that, I will refrain from that in the
future, (along with removing it from v3, but the other change you
suggested should eliminate the referred to macro anyway.)

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

I had intended to complete v2 of it quite some time ago, I've just been
busy. Today as well I got sidetracked with some job hunting, but I plan
on finishing v3 of the initial support patch (the one related to this
thread) tonight at the very least. I can't commit to diagnostics v2
tonight, but if it happens it happens. :)

I might even have to leave out communicating that a PARM_DECL is an
xobj parm cp_parser_parameter_declaration_list if I have too hard a
time figuring out how to work it in, if that is the case then I will
make that change in a v4.

Alex