From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-40133.protonmail.ch (mail-40133.protonmail.ch [185.70.40.133]) by sourceware.org (Postfix) with ESMTPS id ACF7C3858C27 for ; Thu, 21 Sep 2023 11:28:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org ACF7C3858C27 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=1695295709; x=1695554909; bh=imh1jAocJtAWkBJv649NiSVL0jrtj4gj0GqaIrpSuB0=; 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=GYOq6r+k8J6jdLztn75TXq5aE34BfSZeYLB2u0xep1GkQ9Bz3iQv0VB0s6q4Zolqz wGfLK6CR0lEp5b7P2MaKvPuK1WxhIaI67nVMPPd+RJCL+IQCuO0trRche7+SQyw5ZW uoEBP/KKOY55QUfX9diVgi1KZvJQxY8NuNauyAc4f+Zw2cviDMOQTZ3ZgO14Y6fMY7 qUquYTpHPB/nIHkEA8nYar6prT6L53gttVYfSHknWfBYWTKtyvmWaLrFItSWXXb00U JQj2AUOnMx10B0RLdtDT9QtTsTSwGFl978dOnhh9MWI+d3wRxzE4CU2ybNRSjD1L2G CYzF0kgCsNL5w== Date: Thu, 21 Sep 2023 11:28:19 +0000 To: Jason Merrill From: waffl3x Cc: Jakub Jelinek , "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH v2 1/2] c++: Initial support for P0847R7 (Deducing This) [PR102609] Message-ID: In-Reply-To: <09e57c81-5231-16e8-6e57-18c37663c325@redhat.com> References: <3ec4cf47-ccd8-fc55-c4fc-d97402552b92@redhat.com> <9evl-z9cAecBNAGVh82igdeO_HCFYbASO5fS0ngotJBqdpab09FTYaMiAjlZUliISedO0mV66BldzWQtylI4Dax0yC2gdKWuM55xDaG6RQM=@protonmail.com> <09e57c81-5231-16e8-6e57-18c37663c325@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=-3.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_MSPIKE_H5,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: > 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 purpo= se. 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. >=20 >=20 > 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