From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) by sourceware.org (Postfix) with ESMTP id 34275385DC00 for ; Tue, 31 Mar 2020 19:50:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 34275385DC00 Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-339-y71F917yNyeKlJUoNn6EYA-1; Tue, 31 Mar 2020 15:50:18 -0400 X-MC-Unique: y71F917yNyeKlJUoNn6EYA-1 Received: by mail-qt1-f198.google.com with SMTP id w1so18930642qte.6 for ; Tue, 31 Mar 2020 12:50:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:date:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version; bh=dqRgeg7Sn2AwflGGbrYZZbYTJwvl1OxA0h0JtffO9Vw=; b=sbdRdUIqAzdkcvqOnMkqy3t6zXiGEE8GTS2Bigib0Z/27udk1n/ZltAwSZBUh43T5v ls3fzrehHh9V4PD7owgE97SUW7+PCaPyAXqs6SqeqBGfAK6mfp0XJLxmcl25de1r/36e Rs1YRgaUIrwhnhPWjCLYqNGCT3VYN5y8kAWB+wcePUYmx5zJqWHZJjuGPOKVYwWMMYAU vyiMglTqM0kNQIdQR3lBGQVBePqgPEJgTzIYhYjNzbiUrOR26XVSqPK4CvTbTlPKiwSU 1BdycGvhWCnDb6muAts572d3IgD2exCd4+PHZKWCAuDdJoRHlnA2g6ZbR/2O9KQFptlr tOEQ== X-Gm-Message-State: ANhLgQ3gFI/+ut6McCsza1qXwwOD5UqsPfH7Ias/KUIc7DdXSJbvmy3h 1PMO3axSZyuUB9Xn84CXQlVE6D/CqHUgvPQshUXlfOfxOfF1Q1LSVZnWq2K8kn2RmoIcNwcJauv M9rCjLJHhHCHfFNyVyQ== X-Received: by 2002:a05:620a:391:: with SMTP id q17mr6844754qkm.103.1585684217548; Tue, 31 Mar 2020 12:50:17 -0700 (PDT) X-Google-Smtp-Source: ADFU+vuqXwJAktliF8/oOkGqpeqCeF9zi58hfAl7HHycjjyxaCbf78haV+tOFfgMu0AwNbZhOj974A== X-Received: by 2002:a05:620a:391:: with SMTP id q17mr6844736qkm.103.1585684217222; Tue, 31 Mar 2020 12:50:17 -0700 (PDT) Received: from [192.168.1.130] (ool-457d493a.dyn.optonline.net. [69.125.73.58]) by smtp.gmail.com with ESMTPSA id v17sm13172418qkf.83.2020.03.31.12.50.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 31 Mar 2020 12:50:16 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Tue, 31 Mar 2020 15:50:15 -0400 (EDT) To: Jason Merrill cc: Patrick Palka , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010] In-Reply-To: Message-ID: References: <20200323012105.3692086-1-ppalka@redhat.com> <0c702b38-1fca-d0ac-c1ee-f7e080d9367c@redhat.com> User-Agent: Alpine 2.22 (DEB 413 2020-03-19) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-18.7 required=5.0 tests=BAYES_00, 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: Tue, 31 Mar 2020 19:50:21 -0000 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: > > > >=20 > > > > > 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 wild= card > > > > > > function > > > > > > parameter type. Concretely, the latent bug is that given the > > > > > > function > > > > > > template > > > > > >=20 > > > > > > template void foo(const T t); > > > > > >=20 > > > > > > one would expect the type of foo to be void(const int*),= but > > > > > > 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 substituti= on > > > > > > and > > > > > > decaying. > > > > > >=20 > > > > > > We still however correctly substitute into and decay the formal > > > > > > parameter > > > > > > type, > > > > > > obtaining const int* as the type of t after substitution. But = 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. > > > > > >=20 > > > > > > Assuming it's too late at this stage to fix the substitution bu= g, we > > > > > > can > > > > > > still > > > > > > relax the assertion like so. Tested on x86_64-pc-linux-gnu, do= es > > > > > > this > > > > > > look > > > > > > OK? > > > > >=20 > > > > > This is core issues 1001/1322, which have not been resolved. Cla= ng > > > > > does > > > > > the > > > > > substitution the way you suggest; EDG rejects the testcase becaus= e the > > > > > two > > > > > substitutions produce different results. I think it would make s= ense > > > > > to > > > > > follow the EDG behavior until this issue is actually resolved. > > > >=20 > > > > Here is what I have so far towards that end. When substituting int= o the > > > > PARM_DECLs of a function decl, we now additionally check if the > > > > aforementioned Core issues are relevant and issue a (fatal) diagnos= tic > > > > if so. This patch checks this in tsubst_decl rath= er > > > > 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. > > >=20 > > > 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? > >=20 > > 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. > >=20 > > If we were to implement this check in tsubst_function_decl, then since > > we have access to the instantiated function, it would presumably suffic= e > > to compare its substituted DECL_ARGUMENTS with its substituted > > TYPE_ARG_TYPES to see if they're consistent. Doing so would certainly > > catch the original testcase, i.e. > >=20 > > template > > void foo(const T); > > int main() { foo(0); } > >=20 > > because the DECL_ARGUMENTS of foo would be {const int*} and its > > TYPE_ARG_TYPES would be {int*}. But apparently it doesn't catch the > > corresponding testcase that uses a function parameter pack, i.e. > >=20 > > template > > void foo(const Ts...); > > int main() { foo(0); } > >=20 > > because it turns out we don't strip top-level cv-qualifiers from > > function parameter packs from TYPE_ARG_TYPES at declaration time, as we > > do with regular function parameters. 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. > >=20 > > 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. Or maybe there's yet another way to detect > > this? >=20 > I think let's go ahead with comparing TYPE_ARG_TYPES and DECL_ARGUMENTS; = the > problem comes when they disagree. If we're handling pack expansions wron= g, > 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. Take g++.dg/cpp2a/lambda-uneval3.C for example: template void spam(decltype([]{}) (*s)[sizeof(T)]) {} 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 struct ._anon_4[1] * and according to its DECL_ARGUMENTS the type of 's' is 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. I'm not sure how to work around this bug/false-positive..