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 42FB0388CA7E for ; Thu, 15 Dec 2022 19:31:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 42FB0388CA7E 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=1671132678; 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=0HxOZUHxrOGmxMFsZXlRO2Su0illiI7b8mCvId77V5o=; b=JtaOfJjB/SSbaIw0vtBuc8ITIk6IZmWLGMDBKb0F1uO0mZbAYBaSEhODnSTHZG8b/xvY7u hcKRKmfWM7I084wozRwx7p49PrwhWKUVT9u7UkqoRmWj99RqYEmG2NtQw8cyh+yaqf4j0X /lXri0pq5p5PhqYeKkCeHTsAxoAHgKo= Received: from mail-vs1-f71.google.com (mail-vs1-f71.google.com [209.85.217.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-617-LA568vlcM--nbLEMOBUzhw-1; Thu, 15 Dec 2022 14:31:17 -0500 X-MC-Unique: LA568vlcM--nbLEMOBUzhw-1 Received: by mail-vs1-f71.google.com with SMTP id t125-20020a675f83000000b003b152cdaea7so39921vsb.15 for ; Thu, 15 Dec 2022 11:31:17 -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=0HxOZUHxrOGmxMFsZXlRO2Su0illiI7b8mCvId77V5o=; b=fYADwCRqtBheZAswW9EMpYI2V/R1jHD4TZ2kz2PFlWv4iwelh6c/g9I6mEifvtR4JK sSpLB7I4W8iWcLspIVwyAF95CSqx7UQQYmays1L25UNdixOOLII+MRKqdQfA8mstFboR pcnaw2I6znoTOMSq73P7zKbYDDL2STLkSDmnYdptwAIarn+Y1gjSxyresVZPQ5ZE+PGl g4ScsGOCQS/k6vGCDWZz/EK1eY2oSl3seOu4uAdeZCzqnGyWV+wm/+oANDqZbiPoBome HfEUovLeoXhJskhabU4XH6C3HW35ygoAXpICtb4siigQNdvb2iPtoe0NhXyZs3a6Ls3R wllg== X-Gm-Message-State: ANoB5pkcp8tsqKMDmCJWcgcVjR0vDOhGmgtraDNnqfQL/kEYcCeka8q5 sYTn6aRpDQ5Oo+7+uIUr9rKTSQyT4XBTN68LR/JvDofKcXOgjEr8s/tENJ/JzUuwGd/SMUCT6VB +BH47Ona9T84vqd3tcA== X-Received: by 2002:a1f:430f:0:b0:3bd:bd79:dd95 with SMTP id q15-20020a1f430f000000b003bdbd79dd95mr18937996vka.11.1671132676714; Thu, 15 Dec 2022 11:31:16 -0800 (PST) X-Google-Smtp-Source: AA0mqf6OSMyhaHVu1SipFeZyhuiH8aLfoVqKBRlH0IM1/lNxZIJqmRS0Z6Q2VMgQp01NhY+Tu2RwFw== X-Received: by 2002:a1f:430f:0:b0:3bd:bd79:dd95 with SMTP id q15-20020a1f430f000000b003bdbd79dd95mr18937943vka.11.1671132676305; Thu, 15 Dec 2022 11:31:16 -0800 (PST) Received: from [192.168.1.130] (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id g22-20020a05620a40d600b006fc3fa1f589sm13187462qko.114.2022.12.15.11.31.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Dec 2022 11:31:15 -0800 (PST) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Thu, 15 Dec 2022 14:31:15 -0500 (EST) To: Patrick Palka cc: Jason Merrill , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c++: template friend with variadic constraints [PR108066] In-Reply-To: <2faaee96-6cc4-cff7-5cd5-bf3c1f5ce7b6@idea> Message-ID: <0626f4ed-dcd0-dc14-e985-2a9630f7fea0@idea> References: <20221212172057.3527670-1-ppalka@redhat.com> <0b5ba723-31cd-b12c-e3df-c25a1d074825@redhat.com> <2faaee96-6cc4-cff7-5cd5-bf3c1f5ce7b6@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, 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. Here's a concrete testcase that we'd begin to reject if we used the args from DECL_FRIEND_CONTEXT for substitution: // Verify we substitute the correct outer template arguments // when instantiating a constrained template friend. // { 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; > > > > > > + } > > > if (uses_template_parms (gargs)) > > > ++processing_template_decl; > > > reqs = tsubst_constraint (reqs, gargs, > > > 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..95973842afb > > > --- /dev/null > > > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C > > > @@ -0,0 +1,22 @@ > > > +// PR c++/108066 > > > +// PR c++/108067 > > > +// { 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" } > > > +} > > > > >