From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-40132.protonmail.ch (mail-40132.protonmail.ch [185.70.40.132]) by sourceware.org (Postfix) with ESMTPS id 91F6B3858D20 for ; Wed, 20 Sep 2023 00:31:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 91F6B3858D20 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=protonmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=protonmail.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1695169860; x=1695429060; bh=/eJumhRlOBqUBwBnxmnxJRqTOITLHqRd8TLBjdp94ww=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=b4xtapIjvOBE6xbXViTiFt1Ce+8Oz1g0B+u9ZkjJIcGZLK70YRJXgBj61TkzxcLGQ RkW+oxXqE8Hxna3RYjnNFz6FMkcSV34JqJiFci8VFZuN/aGoqxgNjKPIXDyiHtQz1Q 9qcfNkfkws58C3jz6uI+45+zFj5oYNR4Z7rYYUCQacNxIanoslogZOtFy/72z6heGI j0xjC9CPXC2DxR1OB0KsU8mUhci0KR9y4KLo8shLyB/WwTd7QT7GVwMs1vjoz1MCD4 5NWTVvgD9TIxL/TGK+jYwliLstNEJQ7HVhz74iWKgUrE7UTcu4dmzJzFtlv++rIfZ7 EJRSrQ7FpvXiw== Date: Wed, 20 Sep 2023 00:30:48 +0000 To: Jason Merrill From: waffl3x Cc: Jakub Jelinek , "gcc-patches@gcc.gnu.org" Subject: [PATCH v2 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609] Message-ID: <9evl-z9cAecBNAGVh82igdeO_HCFYbASO5fS0ngotJBqdpab09FTYaMiAjlZUliISedO0mV66BldzWQtylI4Dax0yC2gdKWuM55xDaG6RQM=@protonmail.com> In-Reply-To: <3ec4cf47-ccd8-fc55-c4fc-d97402552b92@redhat.com> References: <3ec4cf47-ccd8-fc55-c4fc-d97402552b92@redhat.com> Feedback-ID: 14591686:user:proton MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,KAM_SHORT,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: > 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. */ >=20 >=20 > 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 othe= r > > + decl types so checking is still justified I imagine. */ >=20 >=20 > 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) >=20 >=20 > Better to use a DECL_LANG_FLAG than claim one of the > language-independent flags for C++. >=20 > 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 =3D false; > > + auto get_xobj_parm =3D [](tree parm_list) >=20 >=20 > 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 > > + =3D get_xobj_parm (declarator->u.function.parameters); > > + is_xobj_member_function =3D xobj_parm; >=20 >=20 > 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) > > + =3D decl_spec_seq_has_spec_p (declspecs, ds_this); >=20 >=20 > 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. */ >=20 >=20 > 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?). >=20 > 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