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.133.124]) by sourceware.org (Postfix) with ESMTPS id 60D393858D1E for ; Thu, 22 Dec 2022 21:02:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 60D393858D1E 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=1671742960; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Nli+faweFZ8ihq3RErh67z3x+45NBzCbMf9se9+dH7g=; b=hHzCQ4lDRDNPk2ZcmajsJJ7EBvgNV7jAUYUJ4i0UVPrQ/JWtEh0QsOyMpkyV0XlCjWBtzn T93aUWyzQ9S1LAof3kshyufkQ7cY3P3wJoP9JrL3UJDe2bDJxkNRQRYuSQziFp4Y5ph/CC mP3xALv2KQ3P7TT9Vk+9T74S4jZX81w= Received: from mail-yb1-f200.google.com (mail-yb1-f200.google.com [209.85.219.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-126-NNOOzIOvMOCvtsxW1vcNGQ-1; Thu, 22 Dec 2022 16:02:38 -0500 X-MC-Unique: NNOOzIOvMOCvtsxW1vcNGQ-1 Received: by mail-yb1-f200.google.com with SMTP id a4-20020a5b0004000000b006fdc6aaec4fso3154200ybp.20 for ; Thu, 22 Dec 2022 13:02:38 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Nli+faweFZ8ihq3RErh67z3x+45NBzCbMf9se9+dH7g=; b=wvcuXPNIQ2E5hBUohJogWcUYOt/bq71CZRh9JQfAxsMmhvpRaRxDw+5auNHGC4xs5T aM74kcfPvij3EpGsi5vmjbAwXMIQAUdnW+2RbLPaihcpBh2g8DIUADk3mKEIfD0EgqIe 4aikmI1l6LVbQJaq1WXIz90wwomr/PoO1TNdwuLlLagVQ5qlwpIMhJ/QFKaqplvGO3O6 mdKrRdGj537raX8HGqMa1dCfZd+4bduWA9VDKv9LooCua+F9ZZ/Ib6yhItUt3ojZnPku yEurSY/AKnaHDBe5NOt/m/rLEDgXAqqzpE3vgfdigS+36x1JWT1TBQ6j5AxdJO4ZRrqC b9nw== X-Gm-Message-State: AFqh2kqX9eEwrjtcfJEVdrP5dj+mn55NBckjkz7yF9SoC90cm08daMQM cYZbaYW6h+KLsSNluBXyiJLaFUsVBHIdMrgJFbXJSZ/3qTUVQgJOBSFU8VvbfzWuAbskHkoBNCd HbYjwh6ATD2tIHctBVA== X-Received: by 2002:a05:7500:5e07:b0:d8:ad71:c8fb with SMTP id fe7-20020a0575005e0700b000d8ad71c8fbmr519948gab.1.1671742957624; Thu, 22 Dec 2022 13:02:37 -0800 (PST) X-Google-Smtp-Source: AMrXdXtZNapmVAjOqvTSDiS1RMu2jPE0xbekUpgMNVMHVwf1xaMm1fScl+h1vU5PPdWoZkQQXO38QA== X-Received: by 2002:a05:7500:5e07:b0:d8:ad71:c8fb with SMTP id fe7-20020a0575005e0700b000d8ad71c8fbmr519940gab.1.1671742957078; Thu, 22 Dec 2022 13:02:37 -0800 (PST) Received: from [192.168.1.108] (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id l16-20020a37f910000000b006ef1a8f1b81sm972645qkj.5.2022.12.22.13.00.40 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 22 Dec 2022 13:02:06 -0800 (PST) Message-ID: Date: Thu, 22 Dec 2022 16:00:39 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.1 Subject: Re: [PATCH] c++: template friend with variadic constraints [PR108066] To: Patrick Palka Cc: gcc-patches@gcc.gnu.org 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> From: Jason Merrill In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,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 12/22/22 12:34, Patrick Palka wrote: > 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. OK. >> 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;