From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 3F18C385780B for ; Tue, 22 Sep 2020 20:06:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3F18C385780B Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-65-yi3yiMwUOBKMG9P-7LwzEg-1; Tue, 22 Sep 2020 16:06:10 -0400 X-MC-Unique: yi3yiMwUOBKMG9P-7LwzEg-1 Received: by mail-qt1-f199.google.com with SMTP id y53so17177315qth.2 for ; Tue, 22 Sep 2020 13:06:10 -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=dfF0iwlzf2lzYzPAXnMq0rIiLSf52A+fkunWaqqI9a4=; b=bJt9hh0CW/PGekVpQLT1lZ8eed9gutPHcrh7SmAMH7yLtWiT5xVwEloLF9v3b4p80u QhE4FF4VTW8I5RT4+74VO9rVqIBfbUAO1+lyvN5V/tv7ItunbuGrEQ19gPuYJSWigHSe vyX/IsxlQUcH59I8ogGCz6DsTP8eK+SEkX+rJWuTXh36Dcdxn1n6pJ20WcfxkOK6TT+7 uaDrcsP0gcxRcUmuTE83BVkl4YXWYfVv9QOnJAXitHyOgx3X9JjC0dyGlKlw3tM9esyN 2NKUBu3UtTa71wVLYzCKxR3cDqkuVb54rruZ/Pg3J/RAalxaQMko2E1ytqgUclYTp74A VJ7Q== X-Gm-Message-State: AOAM533m4dcFr3haga8veablSeRE/Jmio/d8UcFULQAbrNiI7DmwRttz F51WAuyYgHZDjnVVV6xUrEFlp8ku2R9I7/O4gTiU00uH4f/RAJXNhiM48jQZV6mNXInoOPT7wlx 73OCdhhbKKEL/X1+1tg== X-Received: by 2002:a0c:f687:: with SMTP id p7mr7768811qvn.15.1600805168712; Tue, 22 Sep 2020 13:06:08 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx4m4NeKrMgQUM/UXWi/5vI7TBue0WZZfeWeiXL+pXTcoP59qt4sCOwr9wmxjKw0at9iZgslQ== X-Received: by 2002:a0c:f687:: with SMTP id p7mr7768782qvn.15.1600805168239; Tue, 22 Sep 2020 13:06:08 -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 a52sm13461317qtc.22.2020.09.22.13.06.07 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 22 Sep 2020 13:06:07 -0700 (PDT) Subject: Re: [PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310] To: Patrick Palka Cc: gcc-patches@gcc.gnu.org References: <20200919194952.3593695-1-ppalka@redhat.com> <45118c59-948d-c373-4622-6ae2bede8707@redhat.com> <8f5f5514-e8b0-b910-411e-334eb81797df@idea> From: Jason Merrill Message-ID: Date: Tue, 22 Sep 2020 16:06:06 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Spam-Status: No, score=-14.9 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, GIT_PATCH_0, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, 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, 22 Sep 2020 20:06:14 -0000 On 9/22/20 2:41 PM, Patrick Palka wrote: > On Tue, 22 Sep 2020, Patrick Palka wrote: > >> On Mon, 21 Sep 2020, Jason Merrill wrote: >> >>> On 9/19/20 3:49 PM, Patrick Palka wrote: >>>> In the testcase below, the dependent specializations iter_reference_t >>>> and iter_reference_t share the same tree due to specialization >>>> caching. So when find_template_parameters walks through the >>>> requires-expression (as part of normalization), it sees and includes the >>>> out-of-scope template parameter F in the list of template parameters >>>> it found within the requires-expression (along with Out and N). >>>> >>>> From a correctness perspective this is harmless since the parameter mapping >>>> routines only care about the level and index of each parameter, so F is >>>> no different from Out in this sense. (And it's also harmless that two >>>> parameters in the parameter mapping have the same level and index.) >>>> >>>> But having both Out and F in the parameter mapping is extra work for >>>> hash_atomic_constrant, tsubst_parameter_mapping and get_mapped_args; and >>>> it also means we print this irrelevant template parameter in the >>>> testcase's diagnostics (via pp_cxx_parameter_mapping): >>>> >>>> in requirements with ‘Out o’ [with N = (const int&)&a; F = const int*; >>>> Out = const int*] >>>> >>>> This patch makes keep_template_parm return only in-scope template >>>> parameters by looking into ctx_parms for the corresponding in-scope one. >>>> >>>> (That we sometimes print irrelevant template parameters in diagnostics is >>>> also the subject of PR99 and PR66968, so the above diagnostic issue >>>> could likely be fixed in a more general way, but this targeted fix to >>>> keep_template_parm is perhaps worthwhile on its own.) >>>> >>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested on >>>> cmcstl2 and range-v3. Does this look OK for trunk? >>>> >>>> gcc/cp/ChangeLog: >>>> >>>> PR c++/95310 >>>> * pt.c (keep_template_parm): Adjust the given template parameter >>>> to the corresponding in-scope one from ctx_parms. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> PR c++/95310 >>>> * g++.dg/concepts/diagnostic15.C: New test. >>>> * g++.dg/cpp2a/concepts-ttp2.C: New test. >>>> --- >>>> gcc/cp/pt.c | 19 +++++++++++++++++++ >>>> gcc/testsuite/g++.dg/concepts/diagnostic15.C | 16 ++++++++++++++++ >>>> 2 files changed, 35 insertions(+) >>>> create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic15.C >>>> >>>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c >>>> index fe45de8d796..c2c70ff02b9 100644 >>>> --- a/gcc/cp/pt.c >>>> +++ b/gcc/cp/pt.c >>>> @@ -10550,6 +10550,25 @@ keep_template_parm (tree t, void* data) >>>> BOUND_TEMPLATE_TEMPLATE_PARM itself. */ >>>> t = TREE_TYPE (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t)); >>>> + /* This template parameter might be an argument to a cached dependent >>>> + specalization that was formed earlier inside some other template, in >>>> which >>>> + case the parameter is not among the ones that are in-scope. Look in >>>> + CTX_PARMS to find the corresponding in-scope template parameter and >>>> + always return that instead. */ >>>> + tree cparms = ftpi->ctx_parms; >>>> + while (TMPL_PARMS_DEPTH (cparms) > level) >>>> + cparms = TREE_CHAIN (cparms); >>>> + gcc_assert (TMPL_PARMS_DEPTH (cparms) == level); >>>> + if (TREE_VEC_LENGTH (TREE_VALUE (cparms))) >>>> + { >>>> + t = TREE_VALUE (TREE_VEC_ELT (TREE_VALUE (cparms), index)); >>>> + /* As in template_parm_to_arg. */ >>>> + if (TREE_CODE (t) == TYPE_DECL || TREE_CODE (t) == TEMPLATE_DECL) >>>> + t = TREE_TYPE (t); >>>> + else >>>> + t = DECL_INITIAL (t); >>>> + } >>> >>> This seems like a useful separate function: given a parmlist and a single >>> template parm (or index+level), return the corresponding parm from the >>> parmlist. Basically the reverse of canonical_type_parameter. >> >> Sounds good. Like this? >> >> -- >8 -- >> >> gcc/cp/ChangeLog: >> >> PR c++/95310 >> * pt.c (corresponding_template_parameter): Define. >> (keep_template_parm): Use it to adjust the given template >> parameter to the corresponding in-scope one from ctx_parms. >> >> gcc/testsuite/ChangeLog: >> >> PR c++/95310 >> * g++.dg/concepts/diagnostic15.C: New test. >> * g++.dg/cpp2a/concepts-ttp2.C: New test. > > Whoops, consider this stray ChangeLog line removed. diagnostic15.C is > the only new test. OK. >> --- >> gcc/cp/pt.c | 44 ++++++++++++++++++++ >> gcc/testsuite/g++.dg/concepts/diagnostic15.C | 16 +++++++ >> 2 files changed, 60 insertions(+) >> create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic15.C >> >> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c >> index 44ca14afc4e..bec8396f9f4 100644 >> --- a/gcc/cp/pt.c >> +++ b/gcc/cp/pt.c >> @@ -10244,6 +10244,42 @@ lookup_and_finish_template_variable (tree templ, tree targs, >> return convert_from_reference (templ); >> } >> >> +/* If the set of template parameters PARMS contains a template with > > s/template with/template parameter at/ > >> + the given LEVEL and INDEX, then return this parameter. Otherwise >> + return NULL_TREE. */ >> + >> +static tree >> +corresponding_template_parameter (tree parms, int level, int index) >> +{ >> + while (TMPL_PARMS_DEPTH (parms) > level) >> + parms = TREE_CHAIN (parms); >> + >> + if (TMPL_PARMS_DEPTH (parms) != level >> + || TREE_VEC_LENGTH (TREE_VALUE (parms)) <= index) >> + return NULL_TREE; >> + >> + tree t = TREE_VALUE (TREE_VEC_ELT (TREE_VALUE (parms), index)); >> + /* As in template_parm_to_arg. */ >> + if (TREE_CODE (t) == TYPE_DECL || TREE_CODE (t) == TEMPLATE_DECL) >> + t = TREE_TYPE (t); >> + else >> + t = DECL_INITIAL (t); >> + >> + gcc_assert (TEMPLATE_PARM_P (t)); >> + return t; >> +} >> + >> +/* Return the template parameter from PARMS that positionally corresponds >> + to the template parameter PARM, or else return NULL_TREE. */ >> + >> +static tree >> +corresponding_template_parameter (tree parms, tree parm) >> +{ >> + int level, index; >> + template_parm_level_and_index (parm, &level, &index); >> + return corresponding_template_parameter (parms, level, index); >> +} >> + >> >> struct pair_fn_data >> { >> @@ -10550,6 +10586,14 @@ keep_template_parm (tree t, void* data) >> BOUND_TEMPLATE_TEMPLATE_PARM itself. */ >> t = TREE_TYPE (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t)); >> >> + /* This template parameter might be an argument to a cached dependent >> + specalization that was formed earlier inside some other template, in >> + which case the parameter is not among the ones that are in-scope. >> + Look in CTX_PARMS to find the corresponding in-scope template >> + parameter and use it instead. */ >> + if (tree in_scope = corresponding_template_parameter (ftpi->ctx_parms, t)) >> + t = in_scope; >> + >> /* Arguments like const T yield parameters like const T. This means that >> a template-id like X would yield two distinct parameters: >> T and const T. Adjust types to their unqualified versions. */ >> diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic15.C b/gcc/testsuite/g++.dg/concepts/diagnostic15.C >> new file mode 100644 >> index 00000000000..3acd9f67968 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/concepts/diagnostic15.C >> @@ -0,0 +1,16 @@ >> +// PR c++/95310 >> +// { dg-do compile { target concepts } } >> + >> +template >> +using iter_reference_t = decltype(*T{}); >> + >> +template >> +struct result { using type = iter_reference_t; }; >> + >> +template >> +concept indirectly_writable = requires(Out o) { // { dg-bogus "F =" } >> + iter_reference_t(*o) = N; >> +}; >> + >> +const int a = 0; >> +static_assert(indirectly_writable); // { dg-error "assert" } >> -- >> 2.28.0.497.g54e85e7af1