From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-2.mimecast.com [205.139.110.61]) by sourceware.org (Postfix) with ESMTP id 9DCF2385BF92 for ; Wed, 1 Apr 2020 22:38:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9DCF2385BF92 Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-254-Ns658zOLPomPCn7ifYqJyg-1; Wed, 01 Apr 2020 18:37:59 -0400 X-MC-Unique: Ns658zOLPomPCn7ifYqJyg-1 Received: by mail-qk1-f198.google.com with SMTP id r64so1284145qkc.17 for ; Wed, 01 Apr 2020 15:37:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=bj31nGCLb/2lBZ5YFZgCC3H3nEyEu62tAij3CHyzyjk=; b=nrTXKC352/b2fmq3RYQfpdrIK47XJmbO9jU3MJ5T/68q5gUy+DI9YqvaIb2YUici0s VDAes7+ZtaKRV33GXK7wpyDCGIZMclgc/E6zgZ6rdwfswnkw6/FyrVwF0ubOSLc0QIzs TR5K7G7i8oASpYkVLkq095ZeGkyljfcHywSuzVlYWj2PXTBNWAxoSqlAYBbitK37yYbt AXPW/+2kbFB2Jngjw7KStWWpOOLg41xmHIIb6ceuAwH6NwFdQMouNWqNcv2Zht8O22lX Xg5PjWWPPx7+reeBukG30Y++VLl/WoDI9bGNzdVsWpetyUs9HHF8OVTLIcBSk0/buf3K dC+g== X-Gm-Message-State: AGi0PuZOjxAjnVNHK2cL+v5b4Hze1MrPqQ0V0uRM06Fa1LBWejsNSvr8 9ajjbq+EELEmGHok9c6TQiNeTyyusSD4ZGLwcDdQY2fd3S1Jg4FBomUeXrbiqepOHcoR0aQ4YXZ ekckMmX3baYSzbHH0kQ== X-Received: by 2002:ac8:12cb:: with SMTP id b11mr45633qtj.384.1585780678701; Wed, 01 Apr 2020 15:37:58 -0700 (PDT) X-Google-Smtp-Source: APiQypJU+O1SG3AG2vtTJ0kqwZmLrbJmtY6HhD4rzn+Nw22evkWyGQENNrJubYfHu15dOwvD6gRRTA== X-Received: by 2002:ac8:12cb:: with SMTP id b11mr45620qtj.384.1585780678335; Wed, 01 Apr 2020 15:37:58 -0700 (PDT) Received: from [192.168.1.148] (209-6-216-142.s141.c3-0.smr-cbr1.sbo-smr.ma.cable.rcncustomer.com. [209.6.216.142]) by smtp.gmail.com with ESMTPSA id g29sm2529014qte.0.2020.04.01.15.37.57 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 01 Apr 2020 15:37:57 -0700 (PDT) Subject: Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010] From: Jason Merrill To: Patrick Palka Cc: gcc-patches@gcc.gnu.org References: <20200323012105.3692086-1-ppalka@redhat.com> <0c702b38-1fca-d0ac-c1ee-f7e080d9367c@redhat.com> Message-ID: Date: Wed, 1 Apr 2020 18:37:57 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.3.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-16.9 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Apr 2020 22:38:03 -0000 On 4/1/20 6:29 PM, Jason Merrill wrote: > On 3/31/20 3:50 PM, Patrick Palka wrote: >> On Tue, 31 Mar 2020, Jason Merrill wrote: >> >>> On 3/30/20 6:46 PM, Patrick Palka wrote: >>>> On Mon, 30 Mar 2020, Jason Merrill wrote: >>>>> On 3/30/20 3:58 PM, Patrick Palka wrote: >>>>>> On Thu, 26 Mar 2020, Jason Merrill wrote: >>>>>> >>>>>>> On 3/22/20 9:21 PM, Patrick Palka wrote: >>>>>>>> This patch relaxes an assertion in tsubst_default_argument that >>>>>>>> exposes >>>>>>>> a >>>>>>>> latent >>>>>>>> bug in how we substitute an array type into a cv-qualified wildcar= d >>>>>>>> function >>>>>>>> parameter type.=C2=A0 Concretely, the latent bug is that given the >>>>>>>> function >>>>>>>> template >>>>>>>> >>>>>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 template void foo(const= T t); >>>>>>>> >>>>>>>> one would expect the type of foo to be void(const int*), bu= t >>>>>>>> we >>>>>>>> (seemingly prematurely) strip function parameter types of their >>>>>>>> top-level >>>>>>>> cv-qualifiers when building the function's TYPE_ARG_TYPES, and >>>>>>>> instead >>>>>>>> end >>>>>>>> up >>>>>>>> obtaining void(int*) as the type of foo after substitution >>>>>>>> and >>>>>>>> decaying. >>>>>>>> >>>>>>>> We still however correctly substitute into and decay the formal >>>>>>>> parameter >>>>>>>> type, >>>>>>>> obtaining const int* as the type of t after substitution.=C2=A0 Bu= t this >>>>>>>> then >>>>>>>> leads >>>>>>>> to us tripping over the assert in tsubst_default_argument that >>>>>>>> verifies >>>>>>>> the >>>>>>>> formal parameter type and the function type are consistent. >>>>>>>> >>>>>>>> Assuming it's too late at this stage to fix the substitution=20 >>>>>>>> bug, we >>>>>>>> can >>>>>>>> still >>>>>>>> relax the assertion like so.=C2=A0 Tested on x86_64-pc-linux-gnu, = does >>>>>>>> this >>>>>>>> look >>>>>>>> OK? >>>>>>> >>>>>>> This is core issues 1001/1322, which have not been resolved.=C2=A0 = Clang >>>>>>> does >>>>>>> the >>>>>>> substitution the way you suggest; EDG rejects the testcase=20 >>>>>>> because the >>>>>>> two >>>>>>> substitutions produce different results.=C2=A0 I think it would mak= e=20 >>>>>>> sense >>>>>>> to >>>>>>> follow the EDG behavior until this issue is actually resolved. >>>>>> >>>>>> Here is what I have so far towards that end.=C2=A0 When substituting= =20 >>>>>> into the >>>>>> PARM_DECLs of a function decl, we now additionally check if the >>>>>> aforementioned Core issues are relevant and issue a (fatal)=20 >>>>>> diagnostic >>>>>> if so.=C2=A0 This patch checks this in tsubst_decl = rather >>>>>> than in tsubst_function_decl for efficiency reasons, so that we don'= t >>>>>> have to perform another traversal over the DECL_ARGUMENTS / >>>>>> TYPE_ARG_TYPES just to implement this check. >>>>> >>>>> Hmm, this seems like writing more complicated code for a very margina= l >>>>> optimization; how many function templates have so many parameters tha= t >>>>> walking >>>>> over them once to compare types will have any effect on compile time? >>>> >>>> Good point... though I just tried implementing this check in >>>> tsubst_function_decl, and it seems it might be just as complicated to >>>> implement it there instead, at least if we want to handle function >>>> parameter packs correctly. >>>> >>>> If we were to implement this check in tsubst_function_decl, then since >>>> we have access to the instantiated function, it would presumably=20 >>>> suffice >>>> to compare its substituted DECL_ARGUMENTS with its substituted >>>> TYPE_ARG_TYPES to see if they're consistent.=C2=A0 Doing so would cert= ainly >>>> catch the original testcase, i.e. >>>> >>>> =C2=A0=C2=A0=C2=A0 template >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 void foo(const T); >>>> =C2=A0=C2=A0=C2=A0 int main() { foo(0); } >>>> >>>> because the DECL_ARGUMENTS of foo would be {const int*} and its >>>> TYPE_ARG_TYPES would be {int*}.=C2=A0 But apparently it doesn't catch = the >>>> corresponding testcase that uses a function parameter pack, i.e. >>>> >>>> =C2=A0=C2=A0=C2=A0 template >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 void foo(const Ts...); >>>> =C2=A0=C2=A0=C2=A0 int main() { foo(0); } >>>> >>>> because it turns out we don't strip top-level cv-qualifiers from >>>> function parameter packs from TYPE_ARG_TYPES at declaration time, as w= e >>>> do with regular function parameters.=C2=A0 So in this second testcase = both >>>> DECL_ARGUMENTS and TYPE_ARG_TYPES of foo would be {const int*}, >>>> and yet we would (presumably) want to reject this instantiation too. >>>> >>>> So it seems comparing TYPE_ARG_TYPES and DECL_ARGUMENTS from >>>> tsubst_function_decl would not suffice, and we would still need to do = a >>>> variant of the trick that's done in this patch, i.e. substitute into >>>> each dependent parameter type stripped of its top-level cv-qualifiers, >>>> to see if these cv-qualifiers make a material difference in the >>>> resulting function type.=C2=A0 Or maybe there's yet another way to det= ect >>>> this? >>> >>> I think let's go ahead with comparing TYPE_ARG_TYPES and=20 >>> DECL_ARGUMENTS; the >>> problem comes when they disagree.=C2=A0 If we're handling pack expansio= ns=20 >>> wrong, >>> that's a separate issue. >> >> Hm, comparing TYPE_ARG_TYPES and DECL_ARGUMENTS for compatibility seems >> to be exposing a latent bug with how we handle lambdas that appear in >> function parameter types.=C2=A0 Take g++.dg/cpp2a/lambda-uneval3.C for >> example: >> >> =C2=A0=C2=A0=C2=A0=C2=A0 template void spam(decltype([]{}) (*s= )[sizeof(T)]) {} >> =C2=A0=C2=A0=C2=A0=C2=A0 int main() { spam(nullptr); } >> >> According to tsubst_function_decl in current trunk, the type of the >> function paremeter 's' of spam according to its TYPE_ARG_TYPES is >> =C2=A0=C2=A0=C2=A0=C2=A0 struct ._anon_4[1] * >> and according to its DECL_ARGUMENTS the type of 's' is >> =C2=A0=C2=A0=C2=A0=C2=A0 struct ._anon_5[1] * >> >> The disagreement happens because we call tsubst_lambda_expr twice during >> substitution and thereby generate two distinct lambda types, one when >> substituting into the TYPE_ARG_TYPES and another when substituting into >> the DECL_ARGUMENTS.=C2=A0 I'm not sure how to work around this >> bug/false-positive.. >=20 > Oof. >=20 > I think probably the right answer is to rebuild TYPE_ARG_TYPES from=20 > DECL_ARGUMENTS if they don't match. ...and treat that as a resolution of 1001/1322, so not giving an error. Jason