On 9/11/23 09:49, waffl3x via Gcc-patches wrote:
Bootstrapped and tested on x86_64-linux with no regressions.

Hopefully I fixed all the issues. I also took the opportunity to remove the
small mistake present in v1, so that is no longer a concern.

Thanks again for all the patience.
   -Alex

Thank you, this is great!

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.

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

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

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

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

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

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

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

Reply via email to