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

Reply via email to