From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by sourceware.org (Postfix) with ESMTP id 057F5385C017 for ; Tue, 31 Mar 2020 17:13:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 057F5385C017 Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-76-WXGgmzKgOQSuA0SY3lwa5g-1; Tue, 31 Mar 2020 13:13:05 -0400 X-MC-Unique: WXGgmzKgOQSuA0SY3lwa5g-1 Received: by mail-qk1-f200.google.com with SMTP id c130so18595201qke.20 for ; Tue, 31 Mar 2020 10:13:04 -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:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=4vAGGmpNqcycM93nGLP5HbJ7xfymqWTTPrwjThjqJe8=; b=CjfnvQ0PMHwvfJrQNEXExVvHnCy16Ej+kZKQF4rONC9JCMQSJQdFWhKxX7t9QgVJ1D /UyDCAKc/obkMDO5j1X7PCvz62Hr1uz/LinJ0C5YYJgvjMKf71Z+ofG/KXKM4XsnN4Aw Q57yGPtRsvHrGDQffCKGr8H9jeCQHVa9vhBDeNS1Z/DaEoMEWibtR1YnkHdsyxAEWXEc CXxoEdYN8viFHlpsnvp+o3bpWUfqHP8vpfHr9lLB9DOacZtD8GZa7wOJrS/FdXI/VX8Z Sv4uaXJxKAXwSkjNMZiFIx1300XZ2N8kIjV5uTg5txuaOuAeawKv46tgIm5tL4UJY5s4 /smQ== X-Gm-Message-State: ANhLgQ2ehEZWUGnKOTjbIHuScWEaHdInCYRY87vynBePv8OJEzf2X+Oj hYu9uObTYQyT2O3RjBACf7f2WAoxEcorr/vVMsr7hMRKL1/1qkWn+6vhhzAEOsyW6jLTH2ZfJT1 eWyIwglTdpplWJO9eUA== X-Received: by 2002:a05:6214:1408:: with SMTP id n8mr17325761qvx.14.1585674783799; Tue, 31 Mar 2020 10:13:03 -0700 (PDT) X-Google-Smtp-Source: ADFU+vt6bEs+BU9TyF0paJvnki6Il2cXKigFYrXh/Ih2KrhUxUwooptHYkDF/wQ9EC6afJbE8CpD/g== X-Received: by 2002:a05:6214:1408:: with SMTP id n8mr17325736qvx.14.1585674783397; Tue, 31 Mar 2020 10:13:03 -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 h143sm13106482qke.58.2020.03.31.10.13.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 31 Mar 2020 10:13:02 -0700 (PDT) Subject: Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010] To: Patrick Palka Cc: gcc-patches@gcc.gnu.org References: <20200323012105.3692086-1-ppalka@redhat.com> <0c702b38-1fca-d0ac-c1ee-f7e080d9367c@redhat.com> From: Jason Merrill Message-ID: Date: Tue, 31 Mar 2020 13:13:01 -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: 7bit X-Spam-Status: No, score=-19.0 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 17:13:08 -0000 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 wildcard >>>>> function >>>>> parameter type. Concretely, the latent bug is that given the function >>>>> template >>>>> >>>>> template void foo(const T t); >>>>> >>>>> 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 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. 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. >>>>> >>>>> Assuming it's too late at this stage to fix the substitution bug, we can >>>>> still >>>>> relax the assertion like so. Tested on x86_64-pc-linux-gnu, does this >>>>> look >>>>> OK? >>>> >>>> This is core issues 1001/1322, which have not been resolved. Clang does >>>> the >>>> substitution the way you suggest; EDG rejects the testcase because the two >>>> substitutions produce different results. I think it would make sense to >>>> follow the EDG behavior until this issue is actually resolved. >>> >>> Here is what I have so far towards that end. When substituting into the >>> PARM_DECLs of a function decl, we now additionally check if the >>> aforementioned Core issues are relevant and issue a (fatal) diagnostic >>> if so. 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 marginal >> optimization; how many function templates have so many parameters that 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 suffice > 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. > > template > void foo(const T); > int main() { foo(0); } > > 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. > > template > void foo(const Ts...); > 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 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. > > 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? 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 wrong, that's a separate issue. Jason