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 [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 2F8A93858D1E for ; Thu, 22 Dec 2022 17:34:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2F8A93858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1671730472; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=xuXgJbbnQKfaXQrUJEaRSM8r40/8bhLUOqT15qULe38=; b=AbhnsTC5FMZb898nGrb61LthWFCHHit3kKchkrOCLEtA9jffVDRZXP/9ycTItBSylunuc5 I76gVBjm37JY0Z9Lly0QHyx5N0W7x61DV7wV+zTS1ul48mDmt85kG2WlNe1zhTr8JIxUSb tOvlWbHK24zuSIxR/sXLhHbjDr3F56M= Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-369-IXCVjiYIM963hgiarp1gLA-1; Thu, 22 Dec 2022 12:34:29 -0500 X-MC-Unique: IXCVjiYIM963hgiarp1gLA-1 Received: by mail-qv1-f69.google.com with SMTP id ld9-20020a056214418900b004d0ab346633so1248259qvb.11 for ; Thu, 22 Dec 2022 09:34:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:references:message-id:in-reply-to:subject:cc:to:date :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=xuXgJbbnQKfaXQrUJEaRSM8r40/8bhLUOqT15qULe38=; b=Ll2FjQx+DGIgMC4mkMPx+DBo6HsXRtVGboQiHQK2kwu9nwVAfRuYI9I4EdzA9aPQdC 0Rw5qGi4SLDXTz3OMC9h1HiSARr3hLKsKn0gTZUq6TUODwVWKuborCM3UMIZk/4WLpkC G0F5X4lIEAq64sDjBsDTCAywED/QJutT4aqzZpNH5pP6Mbiu3wxaJ5LCZz7RDFV9Y0h+ aDyq+HCBtfKfW7IldLuGqu+pOlqp6XnvDS69OMW2/lHtn5InDE8kFekWj7hBnXbvxQMJ tj9qh/TtIP7xwd6FE5iTCtczlmLiL7XHg21PpCMMrzdqgowQSvrFogekVoeoyn4L78Uh QZMg== X-Gm-Message-State: AFqh2kpXc68N53QyIuNKw0zM40aOmqnXf+vlBA4EsrbBf11yWoZ0lRGL njQNN/o6yjIUEyw8dmmFFCn+WuH9FEveZNShyUWDFEw2HPB885weAa/GrgCFzebf4OmzzTx8bMh lxp7wrqERuvjuHQxQ0g== X-Received: by 2002:ac8:5483:0:b0:3a8:10c2:1eaf with SMTP id h3-20020ac85483000000b003a810c21eafmr7989917qtq.2.1671730468658; Thu, 22 Dec 2022 09:34:28 -0800 (PST) X-Google-Smtp-Source: AMrXdXvIBcHv4s8q0Us/0+snYI0T7j51FfGRyc5X0Dm/S3TUA4+FrpcOefA/L0ucOmEup6H32A9Txg== X-Received: by 2002:ac8:5483:0:b0:3a8:10c2:1eaf with SMTP id h3-20020ac85483000000b003a810c21eafmr7989893qtq.2.1671730468246; Thu, 22 Dec 2022 09:34:28 -0800 (PST) Received: from [192.168.1.130] (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id i1-20020ac84881000000b003a7ec97c882sm665212qtq.6.2022.12.22.09.34.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 22 Dec 2022 09:34:27 -0800 (PST) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Thu, 22 Dec 2022 12:34:26 -0500 (EST) To: Jason Merrill cc: Patrick Palka , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c++: template friend with variadic constraints [PR108066] In-Reply-To: Message-ID: References: <20221212172057.3527670-1-ppalka@redhat.com> <0b5ba723-31cd-b12c-e3df-c25a1d074825@redhat.com> <2faaee96-6cc4-cff7-5cd5-bf3c1f5ce7b6@idea> <0626f4ed-dcd0-dc14-e985-2a9630f7fea0@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=-13.9 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_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, 15 Dec 2022, Jason Merrill wrote: > On 12/15/22 14:31, Patrick Palka wrote: > > On Thu, 15 Dec 2022, Patrick Palka wrote: > > > > > On Thu, 15 Dec 2022, Jason Merrill wrote: > > > > > > > On 12/12/22 12:20, Patrick Palka wrote: > > > > > When instantiating a constrained hidden template friend, we need to > > > > > substitute into its constraints for sake of declaration matching. > > > > > > > > Hmm, we shouldn't need to do declaration matching when there's only a > > > > single > > > > declaration. > > > > > > Oops, indeed. It looks like in this case we're not calling > > > maybe_substitute_reqs_for during declaration matching, but rather from > > > tsubst_friend_function and specifically for the non-trailing requirements: > > > > > > if (TREE_CODE (new_friend) == TEMPLATE_DECL) > > > { > > > DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (new_friend) = false; > > > DECL_USE_TEMPLATE (DECL_TEMPLATE_RESULT (new_friend)) = 0; > > > DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (new_friend)) > > > = DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (decl)); > > > > > > /* Substitute TEMPLATE_PARMS_CONSTRAINTS so that parameter levels > > > will > > > match in decls_match. */ > > > tree parms = DECL_TEMPLATE_PARMS (new_friend); > > > tree treqs = TEMPLATE_PARMS_CONSTRAINTS (parms); > > > treqs = maybe_substitute_reqs_for (treqs, new_friend); > > > if (treqs != TEMPLATE_PARMS_CONSTRAINTS (parms)) > > > { > > > TEMPLATE_PARMS_CONSTRAINTS (parms) = treqs; > > > /* As well as each TEMPLATE_PARM_CONSTRAINTS. */ > > > tsubst_each_template_parm_constraints (parms, args, > > > tf_warning_or_error); > > > } > > > } > > > > > > I'll adjust the commit message appropriately. > > > > > > > > > > > > For > > > > > this substitution we use a full argument vector whose outer levels > > > > > correspond to the class's arguments and innermost level corresponds to > > > > > the template's level-lowered generic arguments. > > > > > > > > > > But for A::f here, for which the relevant argument vector is > > > > > {{int}, {Us...}}, this substitution triggers the assert in > > > > > use_pack_expansion_extra_args_p since one argument is a pack expansion > > > > > and the other isn't. > > > > > > > > > > And for A::f, for which the relevant argument vector is > > > > > {{int, int}, {Us...}}, the use_pack_expansion_extra_args_p assert > > > > > would > > > > > also trigger but we first get a bogus "mismatched argument pack > > > > > lengths" > > > > > error from tsubst_pack_expansion. > > > > > > > > > > These might ultimately be bugs in tsubst_pack_expansion, but it seems > > > > > we can work around them by tweaking the constraint substitution in > > > > > maybe_substitute_reqs_for to only use the friend's outer arguments in > > > > > the case of a template friend. > > > > > > > > Yes, this is how we normally substitute a member template during class > > > > instantiation: with the class' template args, not its own. The assert > > > > seems > > > > to be enforcing that. > > > > > > Ah, makes snese. > > > > > > > > > > > > This should be otherwise equivalent to > > > > > substituting using the full arguments, since the template's innermost > > > > > arguments are just its generic arguments with level=1. > > > > > > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK > > > > > for > > > > > trunk/12? > > > > > > > > > > PR c++/108066 > > > > > PR c++/108067 > > > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > > > * constraint.cc (maybe_substitute_reqs_for): For a template > > > > > friend, substitute using only its outer arguments. Remove > > > > > dead uses_template_parms test. > > > > > > > > I don't see any removal? > > > > > > Oops, I reverted that change before posting the patch but forgot to adjust > > > the > > > ChangeLog as well. Removing the uses_template_parms test seems to be safe > > > but > > > it's not necessary to fix the bug. > > > > > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > > > * g++.dg/cpp2a/concepts-friend12.C: New test. > > > > > --- > > > > > gcc/cp/constraint.cc | 8 +++++++ > > > > > .../g++.dg/cpp2a/concepts-friend12.C | 22 > > > > > +++++++++++++++++++ > > > > > 2 files changed, 30 insertions(+) > > > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C > > > > > > > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > > > > index d4cd92ec3b4..f9d1009c9fe 100644 > > > > > --- a/gcc/cp/constraint.cc > > > > > +++ b/gcc/cp/constraint.cc > > > > > @@ -1352,6 +1352,14 @@ maybe_substitute_reqs_for (tree reqs, > > > > > const_tree > > > > > decl) > > > > > tree tmpl = DECL_TI_TEMPLATE (decl); > > > > > tree gargs = generic_targs_for (tmpl); > > > > > processing_template_decl_sentinel s; > > > > > + if (PRIMARY_TEMPLATE_P (tmpl)) > > > > > + { > > > > > + if (TEMPLATE_ARGS_DEPTH (gargs) == 1) > > > > > + return reqs; > > > > > + ++processing_template_decl; > > > > > + gargs = copy_node (gargs); > > > > > + --TREE_VEC_LENGTH (gargs); > > > > > > > > Maybe instead of messing with TEMPLATE_ARGS_DEPTH we want to grab the > > > > targs > > > > for DECL_FRIEND_CONTEXT instead of decl itself? > > > > > > IIUC DECL_FRIEND_CONTEXT wouldn't give us the right args if the template > > > friend > > > is declared inside a partial specialization: > > > > > > template > > > concept C = __is_same(T, U); > > > > > > template > > > struct A; > > > > > > template > > > struct A { > > > template > > > requires (__is_same(T, U)) > > > friend void f() { }; > > > }; > > > > > > template struct A; > > > > > > The DECL_FRIEND_CONTEXT of f is A, whose TYPE_TI_ARGS are {int*}, > > > relative to the primary template instead of the partial specialization. > > > So > > > we'd wrongly end up substituting T=int* instead of T=int into f's > > > TEMPLATE_PARMS_CONSTRAINTS. > > > > > > r12-6950-g76dc465aaf1b74 fixed a similar issue when substituting into > > > class-scope deduction guides declared inside a partial specialization. > > Ah, so this is a workaround for not being able to get at the template args > used in substituting A from A itself. > > Maybe factor this out from here and ctor_deduction_guides_for into an > outer_template_args function with a comment to that effect? Like so? Bootstrapped and regtested on x86_64-pc-linux-gnu. > > Maybe it would make sense to update CLASSTYPE_TEMPLATE_INFO for A after > we choose a partial specialization in instantiate_class_template, but I'm sure > a bunch of places would need to be adjusted to handle that. *nod* Having that information readily available instead of having to call most_specialized_partial_spec would be nice. -- >8 -- Subject: [PATCH] c++: template friend with variadic constraints [PR107853] PR c++/107853 gcc/cp/ChangeLog: * constraint.cc (maybe_substitute_reqs_for): For a template friend, substitute using only its outer arguments via outer_template_args. * cp-tree.h (outer_template_args): Declare. * pt.cc (outer_template_args): Define, factored out and generalized from ... (ctor_deduction_guides_for): ... here. gcc/testsuite/ChangeLog: * g++.dg/cpp2a/concepts-friend12.C: New test. * g++.dg/cpp2a/concepts-friend13.C: New test. --- gcc/cp/constraint.cc | 7 ++-- gcc/cp/cp-tree.h | 1 + gcc/cp/pt.cc | 35 +++++++++++++------ .../g++.dg/cpp2a/concepts-friend12.C | 21 +++++++++++ .../g++.dg/cpp2a/concepts-friend13.C | 20 +++++++++++ 5 files changed, 71 insertions(+), 13 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend13.C diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index 37eae03afdb..247a8278d40 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -1350,11 +1350,12 @@ maybe_substitute_reqs_for (tree reqs, const_tree decl) if (DECL_UNIQUE_FRIEND_P (decl) && DECL_TEMPLATE_INFO (decl)) { tree tmpl = DECL_TI_TEMPLATE (decl); - tree gargs = generic_targs_for (tmpl); + tree outer_args = outer_template_args (tmpl); processing_template_decl_sentinel s; - if (uses_template_parms (gargs)) + if (PRIMARY_TEMPLATE_P (tmpl) + || uses_template_parms (outer_args)) ++processing_template_decl; - reqs = tsubst_constraint (reqs, gargs, + reqs = tsubst_constraint (reqs, outer_args, tf_warning_or_error, NULL_TREE); } return reqs; diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 541d760492d..1f4967c2ba0 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7056,6 +7056,7 @@ extern tree maybe_set_retval_sentinel (void); extern tree template_parms_to_args (tree); extern tree template_parms_level_to_args (tree); extern tree generic_targs_for (tree); +extern tree outer_template_args (tree); /* in expr.cc */ extern tree cplus_expand_constant (tree); diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index e68c74913f5..c0593e9cac6 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -4958,6 +4958,29 @@ generic_targs_for (tree tmpl) return template_parms_to_args (DECL_TEMPLATE_PARMS (tmpl)); } +/* Return the template arguments corresponding to the template + parameters of TMPL's enclosing scope. When TMPL is a member + template of a partial specialization, this returns the arguments + for the partial specialization as opposed to those for the primary + template, which is the main difference between this function and + simply inspecting e.g. the TYPE_TI_ARGS of TMPL's DECL_CONTEXT. */ + +tree +outer_template_args (tree tmpl) +{ + tree ti = get_template_info (DECL_TEMPLATE_RESULT (tmpl)); + if (!ti) + return NULL_TREE; + tree args = TI_ARGS (ti); + if (!PRIMARY_TEMPLATE_P (tmpl)) + return args; + if (TMPL_ARGS_DEPTH (args) == 1) + return NULL_TREE; + args = copy_node (args); + --TREE_VEC_LENGTH (args); + return args; +} + /* Update the declared TYPE by doing any lookups which were thought to be dependent, but are not now that we know the SCOPE of the declarator. */ @@ -30081,16 +30104,8 @@ alias_ctad_tweaks (tree tmpl, tree uguides) static tree ctor_deduction_guides_for (tree tmpl, tsubst_flags_t complain) { - tree type = TREE_TYPE (tmpl); - tree outer_args = NULL_TREE; - if (DECL_CLASS_SCOPE_P (tmpl) - && CLASSTYPE_TEMPLATE_INSTANTIATION (DECL_CONTEXT (tmpl))) - { - outer_args = copy_node (CLASSTYPE_TI_ARGS (type)); - gcc_assert (TMPL_ARGS_DEPTH (outer_args) > 1); - --TREE_VEC_LENGTH (outer_args); - type = TREE_TYPE (most_general_template (tmpl)); - } + tree outer_args = outer_template_args (tmpl); + tree type = TREE_TYPE (most_general_template (tmpl)); tree cands = NULL_TREE; diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C new file mode 100644 index 00000000000..9687be5931a --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C @@ -0,0 +1,21 @@ +// PR c++/107853 +// { dg-do compile { target c++20 } } + +template +concept C = __is_same(T, U); + +template +struct A { + template + requires (C && ...) + friend void f(A, A) { } +}; + +int main() { + A x; + f(x, x); + A y; + f(y, y); + A z; + f(x, z); // { dg-error "no match" } +} diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend13.C b/gcc/testsuite/g++.dg/cpp2a/concepts-friend13.C new file mode 100644 index 00000000000..3cc4505a7ef --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend13.C @@ -0,0 +1,20 @@ +// Verify we substitute the correct outer template arguments +// when instantiating a constrained template friend declared +// inside a partial specialization. +// { dg-do compile { target c++20 } } + +template + requires __is_same(int*, U) +void f() { }; + +template +struct A; + +template +struct A { + template + requires __is_same(T, U) + friend void f() { } // { dg-bogus "redefinition" } +}; + +template struct A; -- 2.39.0.95.g7c2ef319c5