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 [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 38074385783A for ; Sat, 19 Sep 2020 00:42:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 38074385783A Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-168-zcE8EM2kOjmqRfBJ4r-VvA-1; Fri, 18 Sep 2020 20:42:42 -0400 X-MC-Unique: zcE8EM2kOjmqRfBJ4r-VvA-1 Received: by mail-qk1-f199.google.com with SMTP id m186so5417575qkf.12 for ; Fri, 18 Sep 2020 17:42:42 -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:mime-version; bh=nOOr2WCUL4RWtfxZsnKniSBjqbyc+7at7UNQvArkwcA=; b=L/w1GY/64Pb3SKKDwiajS+Z+pHN5tukzIV9QyZ7ie9oI0bhHoLkebOVPrxJRBKZiRx ZTSNfaL+Sq/523ynmCFnQCfrkUut5xg6hPBKWhf5miroeWymvhMMIvznrYB6Ssa834MA ClVS/KhEaPBokxU8Nj+M0Kchcms3uQz47rMUkEBh7ifH5BP5hh3sbWop2QNJHbaETbRx Lam5+q9kZkGkjCH8qNaOYzg1hkyb7Pu/QCrOWCNBIaELjR72W0Pu3vBCAkXvxbx5eGB2 01jRNG9xhIXEqt8/7ZXeEsi18qXEPPoMBP2fU7uI6+mhpI51wkx/QUjNGlYxOaaLe1fM gT/g== X-Gm-Message-State: AOAM533x8DL5vErffgRnno3bRe8gDjph94Qz4T1vApi6H4S6xarvpNmX Op0WUioHzK6J10Ei/2eE+el0wLbDlNN//4pnf+QYqTfgJQpksje0PJqcJ/6GIHqUFOJ3z9PIU02 Mpel/bMefH45Ef62w6w== X-Received: by 2002:a37:2782:: with SMTP id n124mr33974628qkn.15.1600476161116; Fri, 18 Sep 2020 17:42:41 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwdaakYlo+hH7iG4843lfGv/WM6MZ/uXS4gxBtWdbLnXS3JeezoaP/N65X1oV19EPypRpPaEA== X-Received: by 2002:a37:2782:: with SMTP id n124mr33974612qkn.15.1600476160715; Fri, 18 Sep 2020 17:42:40 -0700 (PDT) Received: from [192.168.1.130] (ool-457d493a.dyn.optonline.net. [69.125.73.58]) by smtp.gmail.com with ESMTPSA id l29sm3372791qtb.77.2020.09.18.17.42.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 18 Sep 2020 17:42:39 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Fri, 18 Sep 2020 20:42:38 -0400 (EDT) To: Patrick Palka cc: Jason Merrill , gcc-patches@gcc.gnu.org, nathan@acm.org Subject: Re: [PATCH] c++: Fix self-mapping in map_arguments [PR96531, PR97103] In-Reply-To: <1cc77a4-1c52-36-36c6-db9dc3e49e73@idea> Message-ID: <1d38dc95-dcf9-e7df-ff1d-a5daba68be59@idea> References: <20200918200726.2123779-1-ppalka@redhat.com> <1cc77a4-1c52-36-36c6-db9dc3e49e73@idea> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-17.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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: Sat, 19 Sep 2020 00:42:58 -0000 On Fri, 18 Sep 2020, Patrick Palka wrote: > On Fri, 18 Sep 2020, Jason Merrill wrote: > > > On 9/18/20 4:07 PM, Patrick Palka wrote: > > > With r10-8077 we stopped passing the argified current_template_parms to > > > normalize_constraint_expression from finish_nested_requirement, and > > > instead tweaked map_arguments to perform a self-mapping of parameters > > > when args is NULL. We're currently not handling parameter packs and > > > BOUND_TEMPLATE_TEMPLATE_PARMs properly during this self-mapping, which > > > leads to ICEs later during satisfaction. > > > > > > To fix the self-mapping of a parameter pack, this patch makes > > > map_arguments use template_parm_to_arg which already does the right > > > thing for parameter packs. > > > > > > Before r10-8077, a BOUND_TEMPLATE_TEMPLATE_PARM would get mapped to the > > > corresponding TEMPLATE_TEMPLATE_PARM. We could restore this behavior in > > > map_arguments, but since a BOUND_TEMPLATE_TEMPLATE_PARM is not really a > > > template parameter it seems better to make keep_template_parm not give > > > us a BOUND_TEMPLATE_TEMPLATE_PARM in the first place. I think what we > > > actually want is to map the TEMPLATE_TEMPLATE_PARM to itself, so this > > > patch adjusts keep_template_parm to give us the corresponding > > > TEMPLATE_TEMPLATE_PARM of a BOUND_TEMPLATE_TEMPLATE_PARM instead. > > > > > > Tested on x86_64-pc-linux-gnu, and also tested with the cmcstl2 library. > > > Does this look OK for trunk/10? > > > > > > gcc/cp/ChangeLog: > > > > > > PR c++/96531 > > > PR c++/97103 > > > * constraint.cc (map_arguments): Call template_parm_to_arg > > > appropriately when doing a self-mapping. > > > * pt.c (keep_template_parm): Don't record a > > > BOUND_TEMPLATE_TEMPLATE_PARM, instead record its corresponding > > > TEMPLATE_TEMPLATE_PARM. > > > > > > gcc/testsuite/ChangeLog: > > > > > > PR c++/96531 > > > PR c++/97103 > > > * g++.dg/cpp2a/concepts-ttp2.C: New test. > > > * g++.dg/cpp2a/concepts-variadic1.C: New test. > > > --- > > > gcc/cp/constraint.cc | 27 ++++++++++++------ > > > gcc/cp/pt.c | 5 ++++ > > > gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C | 11 ++++++++ > > > .../g++.dg/cpp2a/concepts-variadic1.C | 28 +++++++++++++++++++ > > > 4 files changed, 62 insertions(+), 9 deletions(-) > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-variadic1.C > > > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > > index 0aab3073cc1..43336d191d9 100644 > > > --- a/gcc/cp/constraint.cc > > > +++ b/gcc/cp/constraint.cc > > > @@ -546,15 +546,24 @@ static tree > > > map_arguments (tree parms, tree args) > > > { > > > for (tree p = parms; p; p = TREE_CHAIN (p)) > > > - if (args) > > > - { > > > - int level; > > > - int index; > > > - template_parm_level_and_index (TREE_VALUE (p), &level, &index); > > > - TREE_PURPOSE (p) = TMPL_ARG (args, level, index); > > > - } > > > - else > > > - TREE_PURPOSE (p) = TREE_VALUE (p); > > > + { > > > + tree parm = TREE_VALUE (p); > > > + if (args) > > > + { > > > + int level; > > > + int index; > > > + template_parm_level_and_index (parm, &level, &index); > > > + TREE_PURPOSE (p) = TMPL_ARG (args, level, index); > > > + } > > > + else > > > + { > > > + tree tpi = (TYPE_P (parm) > > > + ? TEMPLATE_TYPE_PARM_INDEX (parm) : parm); > > > + TREE_PURPOSE (p) > > > + = template_parm_to_arg (build_tree_list (NULL_TREE, > > > + TEMPLATE_PARM_DECL > > > (tpi))); > > > > Doesn't passing 'p' to template_parm_to_arg work? > > Unfortunately not, template_parm_to_arg expects a TREE_LIST node whose > TREE_VALUE is the corresponding *_DECL for the template parm (i.e. it > expects node from current_template_parms), and 'p' is a > TEMPLATE_TYPE_PARM, TEMPLATE_TEMPLATE PARM or TEMPLATE_PARM_INDEX (given > to us by find_template_parameters). Oops, 'p' is of course a TREE_LIST of a TEMPLATE_PARM_P node. But template_parm_to_arg wants a TREE_LIST of a DECL_TEMPLATE_PARM_P node. > > Would it be appropriate to adjust template_parm_to_arg to accept the > latter inputs as well? This version extends template_parm_to_arg to handle TEMPLATE_PARM_P nodes alongside its existing DECL_TEMPLATE_PARM_P handling, which allows us to simply pass 'p' to template_parm_to_arg from map_arguments. Bootstrapped and regtested on x86_64-pc-linux-gnu and tested on cmcstl2 and range-v3. -- >8 -- Subject: [PATCH] c++: Fix self-mapping in map_arguments [PR96531, PR97103] With r10-8077 we stopped passing the argified current_template_parms to normalize_constraint_expression from finish_nested_requirement, and instead made map_arguments perform a self-mapping of parameters when args is NULL. But we're currently not handling parameter packs and BOUND_TEMPLATE_TEMPLATE_PARMs properly during this self-mapping, which leads to ICEs later during satisfaction. To properly handle self-mapping of a parameter pack, this patch extends template_parm_to_arg to handle TEMPLATE_PARM_P nodes, and makes map_arguments use it. This change revealed that the call to template_parm_to_arg in convert_generic_types_to_packs is a no-op because 't' is never a TREE_LIST, so this patch additionally removes this call. As for bound ttps, map_arguments before r10-8077 would map a BOUND_TEMPLATE_TEMPLATE_PARM not to itself but to its underlying TEMPLATE_TEMPLATE_PARM. We could restore this behavior in map_arguments, but since a bound ttp is not really a template parameter it seems better to make keep_template_parm not give us a bound ttp in the first place. So this patch makes keep_template_parm return the underlying ttp instead of the bound ttp itself. gcc/cp/ChangeLog: PR c++/96531 PR c++/97103 * constraint.cc (map_arguments): Call template_parm_to_arg in the self-mapping case. (finish_shorthand_constraint): No need to build a TREE_LIST before calling template_parm_to_arg. * pt.c (template_parm_to_arg): Rewrite to handle TEMPLATE_PARM_P nodes as well as DECL_TEMPLATE_PARM_P nodes, and to make the overlying TREE_LIST node optional. (keep_template_parm): Don't record a BOUND_TEMPLATE_TEMPLATE_PARM, instead record its corresponding TEMPLATE_TEMPLATE_PARM. (convert_generic_types_to_packs): Don't call template_parm_to_arg. gcc/testsuite/ChangeLog: PR c++/96531 PR c++/97103 * g++.dg/cpp2a/concepts-ttp2.C: New test. * g++.dg/cpp2a/concepts-variadic1.C: New test. --- gcc/cp/constraint.cc | 4 +- gcc/cp/pt.c | 43 ++++++++++++------- gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C | 11 +++++ .../g++.dg/cpp2a/concepts-variadic1.C | 28 ++++++++++++ 4 files changed, 68 insertions(+), 18 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-variadic1.C diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index 0aab3073cc1..2827d3f91b3 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -554,7 +554,7 @@ map_arguments (tree parms, tree args) TREE_PURPOSE (p) = TMPL_ARG (args, level, index); } else - TREE_PURPOSE (p) = TREE_VALUE (p); + TREE_PURPOSE (p) = template_parm_to_arg (p); return parms; } @@ -1492,7 +1492,7 @@ finish_shorthand_constraint (tree decl, tree constr) /* Get the argument and overload used for the requirement and adjust it if we're going to expand later. */ - tree arg = template_parm_to_arg (build_tree_list (NULL_TREE, decl)); + tree arg = template_parm_to_arg (decl); if (apply_to_each_p && declared_pack_p) arg = PACK_EXPANSION_PATTERN (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg), 0)); diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index cfe5ff4a94f..93706543d93 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -4704,29 +4704,37 @@ end_template_decl (void) current_template_parms = TREE_CHAIN (current_template_parms); } -/* Takes a TREE_LIST representing a template parameter and convert it - into an argument suitable to be passed to the type substitution - functions. Note that If the TREE_LIST contains an error_mark - node, the returned argument is error_mark_node. */ +/* Takes a TEMPLATE_PARM_P or DECL_TEMPLATE_PARM_P node or a TREE_LIST + thereof, and converts it into an argument suitable to be passed to + the type substitution functions. Note that if the TREE_LIST contains + an error_mark node, the returned argument is error_mark_node. */ tree template_parm_to_arg (tree t) { - - if (t == NULL_TREE - || TREE_CODE (t) != TREE_LIST) + if (t == NULL_TREE) return t; - if (error_operand_p (TREE_VALUE (t))) - return error_mark_node; + if (TREE_CODE (t) == TREE_LIST) + t = TREE_VALUE (t); - t = TREE_VALUE (t); + if (error_operand_p (t)) + return error_mark_node; - if (TREE_CODE (t) == TYPE_DECL - || TREE_CODE (t) == TEMPLATE_DECL) + if (DECL_P (t) && DECL_TEMPLATE_PARM_P (t)) { - t = TREE_TYPE (t); + 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)); + if (TREE_CODE (t) == TEMPLATE_TYPE_PARM + || TREE_CODE (t) == TEMPLATE_TEMPLATE_PARM) + { if (TEMPLATE_TYPE_PARAMETER_PACK (t)) { /* Turn this argument into a TYPE_ARGUMENT_PACK @@ -4743,8 +4751,6 @@ template_parm_to_arg (tree t) } else { - t = DECL_INITIAL (t); - if (TEMPLATE_PARM_PARAMETER_PACK (t)) { /* Turn this argument into a NONTYPE_ARGUMENT_PACK @@ -10539,6 +10545,11 @@ keep_template_parm (tree t, void* data) if (level > ftpi->max_depth) return 0; + if (TREE_CODE (t) == BOUND_TEMPLATE_TEMPLATE_PARM) + /* We want the underlying TEMPLATE_TEMPLATE_PARM, not the + BOUND_TEMPLATE_TEMPLATE_PARM itself. */ + t = TREE_TYPE (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t)); + /* 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. */ @@ -29448,7 +29459,7 @@ convert_generic_types_to_packs (tree parm, int start_idx, int end_idx) if (tree constr = TEMPLATE_PARM_CONSTRAINTS (node)) { tree id = unpack_concept_check (constr); - TREE_VEC_ELT (TREE_OPERAND (id, 1), 0) = template_parm_to_arg (t); + TREE_VEC_ELT (TREE_OPERAND (id, 1), 0) = t; tree fold = finish_left_unary_fold_expr (constr, TRUTH_ANDIF_EXPR); TEMPLATE_PARM_CONSTRAINTS (node) = fold; diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C b/gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C new file mode 100644 index 00000000000..7f4883754dd --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C @@ -0,0 +1,11 @@ +// PR c++/97103 +// { dg-do compile { target c++20 } } + +template +class quantity {}; + +template typename Q> +inline constexpr bool valid_template_arguments = requires { + requires requires { typename Q; }; +}; +static_assert(valid_template_arguments); diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-variadic1.C b/gcc/testsuite/g++.dg/cpp2a/concepts-variadic1.C new file mode 100644 index 00000000000..deab028ca3c --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-variadic1.C @@ -0,0 +1,28 @@ +// PR c++/96531 +// { dg-do compile { target c++20 } } + +template +concept is_bool = __is_same(bool, T); + +template +concept C = requires { + requires (is_bool || ...); +}; + +template +concept D = requires { + requires (Bs || ...); +}; + +template +requires C +void bar() {} + +template +requires D +void baz() {} + +int main() { + bar(); + baz(); +} -- 2.28.0.497.g54e85e7af1 > > > > > > + } > > > + } > > > return parms; > > > } > > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > > > index cfe5ff4a94f..55d8060b911 100644 > > > --- a/gcc/cp/pt.c > > > +++ b/gcc/cp/pt.c > > > @@ -10539,6 +10539,11 @@ keep_template_parm (tree t, void* data) > > > if (level > ftpi->max_depth) > > > return 0; > > > + if (TREE_CODE (t) == BOUND_TEMPLATE_TEMPLATE_PARM) > > > + /* A BOUND_TEMPLATE_TEMPLATE_PARM isn't a template parameter. What we > > > + really want is the corresponding TEMPLATE_TEMPLATE_PARM. */ > > > + t = TREE_TYPE (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t)); > > > + > > > /* 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/cpp2a/concepts-ttp2.C > > > b/gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C > > > new file mode 100644 > > > index 00000000000..7f4883754dd > > > --- /dev/null > > > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C > > > @@ -0,0 +1,11 @@ > > > +// PR c++/97103 > > > +// { dg-do compile { target c++20 } } > > > + > > > +template > > > +class quantity {}; > > > + > > > +template typename Q> > > > +inline constexpr bool valid_template_arguments = requires { > > > + requires requires { typename Q; }; > > > +}; > > > +static_assert(valid_template_arguments); > > > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-variadic1.C > > > b/gcc/testsuite/g++.dg/cpp2a/concepts-variadic1.C > > > new file mode 100644 > > > index 00000000000..deab028ca3c > > > --- /dev/null > > > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-variadic1.C > > > @@ -0,0 +1,28 @@ > > > +// PR c++/96531 > > > +// { dg-do compile { target c++20 } } > > > + > > > +template > > > +concept is_bool = __is_same(bool, T); > > > + > > > +template > > > +concept C = requires { > > > + requires (is_bool || ...); > > > +}; > > > + > > > +template > > > +concept D = requires { > > > + requires (Bs || ...); > > > +}; > > > + > > > +template > > > +requires C > > > +void bar() {} > > > + > > > +template > > > +requires D > > > +void baz() {} > > > + > > > +int main() { > > > + bar(); > > > + baz(); > > > +} > > > > > > > >