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 ECB4C3858D33 for ; Wed, 1 Mar 2023 21:33:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org ECB4C3858D33 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=1677706401; 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=Gbe6/XCXBTzBb2iuoUQg4XXlB2uYLvEImfha2QZ9eqE=; b=OyVwFXva8cU5Wi7sEwRzr5iLPuVNqnEOHBKS8ZFgwOiLsl8efKCE1YCukONjZBJXcysZC+ Tur1q4sblg+XwdWBL80g1LMdG8W0KR3TKncii6KmPiRS4Ci3Z4CzBb0kQFGyGn8H5xxwPG J3jyb5P9Aca7jt6vU9OGUrcy+SC0ihw= Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-646-Z6zN4F2gPfGtzMVK0nIOMw-1; Wed, 01 Mar 2023 16:33:20 -0500 X-MC-Unique: Z6zN4F2gPfGtzMVK0nIOMw-1 Received: by mail-qk1-f197.google.com with SMTP id z23-20020a05620a101700b0073b328e7d17so8838228qkj.9 for ; Wed, 01 Mar 2023 13:33:20 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677706400; 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=Gbe6/XCXBTzBb2iuoUQg4XXlB2uYLvEImfha2QZ9eqE=; b=yXuKmYQRro+TZECiOWx0BO+oUu3hREUmRLCJoTerDtX6Zk/9k/WjhaLR/I1WaCtZvq KeeqwWvus1+xzyQb4vkIFwUvBVM7hoIH+Rjh2QZQr+KP1ElZ9iZ57TSJILJ8KaCzNy1j Xj6yjthM2QsiOhyaghcSszmMqSMrD7RImTYQKe1mZIzPiOaNxhVVBVA+3MXHgfszr2Xz rs8lVjpYcNoqo2eor2srLQeHD25uaxV2tru1yFh2AeguLRyNmE2yJx/zOTDbMytf/sMf loMzKnk6rgrGNP6U/bjPFLg0pQqAW7nP02lCRzCwC9PlV8amfQ6aDQUYWQX3D2ia4Aaz Q/sA== X-Gm-Message-State: AO0yUKXMl2nriYk3L7vz9qEVu+DQhXc8svREFaHaaTLu9JVr3hwN6iMR GROetv4k9XiTvmvhs92fR703CbZ4AS8j8LFlgCJM7qMbc6yjEyWKTSo1Ijek7460sUNzFWeYly/ KJWiE3H9/2p++xzO8gA== X-Received: by 2002:a05:6214:2387:b0:570:bf43:47a with SMTP id fw7-20020a056214238700b00570bf43047amr13752596qvb.1.1677706399765; Wed, 01 Mar 2023 13:33:19 -0800 (PST) X-Google-Smtp-Source: AK7set/e/5E2JmnTE7661rZ3nYcCOzzWG6bZM6UnVen8p/WU+UI+rLMP9oFg6ea57Rl6mFBUASdxfg== X-Received: by 2002:a05:6214:2387:b0:570:bf43:47a with SMTP id fw7-20020a056214238700b00570bf43047amr13752565qvb.1.1677706399347; Wed, 01 Mar 2023 13:33:19 -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 w16-20020a05620a129000b0073bad2f9380sm9518613qki.14.2023.03.01.13.33.18 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 01 Mar 2023 13:33:18 -0800 (PST) Message-ID: <6ae69fba-f369-6a5c-54b2-0ccd1e800b93@redhat.com> Date: Wed, 1 Mar 2023 16:33:17 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution To: Patrick Palka Cc: gcc-patches@gcc.gnu.org References: <20230213172340.849204-1-ppalka@redhat.com> <1a709771-253a-780c-335e-a5819a063483@redhat.com> <56845a84-0f51-1d4e-beae-3993c3ef4164@idea> <8b0777ef-6814-a720-9d86-7a4b3ef5f4c1@idea> <1f9f7a1d-b968-5a4c-a93c-e20aecd27083@redhat.com> 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=-12.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,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 2/21/23 19:05, Patrick Palka wrote: > On Sun, 19 Feb 2023, Jason Merrill wrote: > >> On 2/15/23 12:11, Patrick Palka wrote: >>> On Wed, 15 Feb 2023, Jason Merrill wrote: >>> >>>> On 2/15/23 09:21, Patrick Palka wrote: >>>>> On Tue, 14 Feb 2023, Jason Merrill wrote: >>>>> >>>>>> On 2/13/23 09:23, Patrick Palka wrote: >>>>>>> [N.B. this is a corrected version of >>>>>>> https://gcc.gnu.org/pipermail/gcc-patches/2022-November/607443.html >>>>>>> ] >>>>>>> >>>>>>> This patch factors out the TYPENAME_TYPE case of tsubst into a >>>>>>> separate >>>>>>> function tsubst_typename_type. It also factors out the two tsubst >>>>>>> flags >>>>>>> controlling TYPENAME_TYPE substitution, tf_keep_type_decl and >>>>>>> tf_tst_ok, >>>>>>> into distinct boolean parameters of this new function (and of >>>>>>> make_typename_type). Consequently, callers which used to pass >>>>>>> tf_tst_ok >>>>>>> to tsubst now instead must directly call tsubst_typename_type when >>>>>>> appropriate. >>>>>> >>>>>> Hmm, I don't love how that turns 4 lines into 8 more complex lines in >>>>>> each >>>>>> caller. And the previous approach of saying "a CTAD placeholder is >>>>>> OK" >>>>>> seem >>>>>> like better abstraction than repeating the specific TYPENAME_TYPE >>>>>> handling >>>>>> in >>>>>> each place. >>>>> >>>>> Ah yeah, I see what you mean. I was thinking since tf_tst_ok is >>>>> specific to TYPENAME_TYPE handling and isn't propagated (i.e. it only >>>>> affects top-level TYPENAME_TYPEs), it seemed cleaner to encode the flag >>>>> as a bool parameter "template_ok" of tsubst_typename_type instead of as >>>>> a global tsubst_flag that gets propagated freely. >>>>> >>>>>> >>>>>>> In a subsequent patch we'll add another flag to >>>>>>> tsubst_typename_type controlling whether we want to ignore non-types >>>>>>> during the qualified lookup. >>>>> >>>>> As mentioned above, the second patch in this series would just add >>>>> another flag "type_only" alongside "template_ok", since this flag will >>>>> also only affects top-level TYPENAME_TYPEs and doesn't need to propagate >>>>> like tsubst_flags. >>>>> >>>>> Except, it turns it, this new flag _does_ need to propagate, namely when >>>>> expanding a variadic using: >>>>> >>>>> using typename Ts::type::m...; // from typename25a.C below >>>>> >>>>> Here we have a USING_DECL whose USING_DECL_SCOPE is a >>>>> TYPE_PACK_EXPANSION over TYPENAME_TYPE. In order to correctly >>>>> substitute this TYPENAME_TYPE, the USING_DECL case of tsubst_decl needs >>>>> to pass an appropriate tsubst_flag to tsubst_pack_expansion to be >>>>> propagated to tsubst (to be propagated to make_typename_type). >>>>> >>>>> So in light of this case it seems adding a new tsubst_flag is the >>>>> way to go, which means we can avoid this refactoring patch entirely. >>>>> >>>>> Like so? Bootstrapped and regtested on x86_64-pc-linux-gnu. >>>> >>>> OK, though I still wonder about adding a tsubst_scope function that would >>>> add >>>> the tf_qualifying_scope. >>> >>> Hmm, but we need to add tf_qualifying_scope to two tsubst_copy calls, >>> one tsubst call and one tsubst_aggr_type call (with entering_scope=true). >>> Would tsubst_scope call tsubst, tsubst_copy or tsubst_aggr_type? >> >> In general it would call tsubst. >> >> It's odd that anything is calling tsubst_copy with a type, that seems like a >> copy/paste error. But it just hands off to tsubst anyway, so the effect is >> the same. > > Ah, makes sense. And if we make tsubst_copy hand off to tsubst > immediately for TYPE_P trees we can make tsubst_copy oblivious to the > tf_qualifying_scope flag. I experimented with going a step further and > fixing callers that pass a type to tsubst_copy, but that sort of clean > up might be in vain given that we might be getting rid of tsubst_copy > in the next stage 1. > >> >> tsubst_aggr_type is needed when pushing into the scope of a declarator; I >> don't know offhand why we would need that when substituting the scope of a >> TYPENAME_TYPE. > > Apparently if we don't do entering_scope=true here then it breaks > > g++.dg/template/friend61.C > g++.dg/template/memfriend12.C > g++.dg/template/memfriend17.C > > I think it's because without entering_scope=true, dependent substitution > yields a dependent specialization A instead of the primary template > type A, but we need the latter to perform qualified name lookup from > such a substituted dependent scope. I left that call alone for now. > > How does this look? Bootstrapped and regtested on x86_64-pc-linux-gnu. OK. > -- >8 -- > > Subject: [PATCH] c++: clean up tf_qualifying_scope usage > > This patch introduces a convenience wrapper tsubst_scope for tsubst'ing > into a type with tf_qualifying_scope set, and makes suitable callers use > it instead of explicitly setting tf_qualifying_scope. This also makes > tsubst_copy immediately delegate to tsubst for all type trees, which > allows tsubst_copy to be oblivious to the tf_qualifying_scope flag. > > gcc/cp/ChangeLog: > > * pt.cc (tsubst_scope): Define. > (tsubst_decl) : Call tsubst_scope instead of > calling tsubst_scope with tf_qualifying_scope set. > (tsubst_qualified_id): Call tsubst_scope instead of > calling tsubst with tf_qualifying_scope set. > (tsubst_copy): Immediately delegate to tsubst for all TYPE_P > trees. Remove tf_qualifying_scope manipulation. > : Call tsubst_scope instead of calling > tsubst with tf_qualifying_scope set. > --- > gcc/cp/pt.cc | 43 +++++++++++++++++-------------------------- > 1 file changed, 17 insertions(+), 26 deletions(-) > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index d11d540ab44..6a22fac5b32 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -206,6 +206,7 @@ static bool dependent_template_arg_p (tree); > static bool dependent_type_p_r (tree); > static tree tsubst_copy (tree, tree, tsubst_flags_t, tree); > static tree tsubst_decl (tree, tree, tsubst_flags_t); > +static tree tsubst_scope (tree, tree, tsubst_flags_t, tree); > static void perform_instantiation_time_access_checks (tree, tree); > static tree listify (tree); > static tree listify_autos (tree, tree); > @@ -15004,9 +15005,7 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain) > variadic_p = true; > } > else > - scope = tsubst_copy (scope, args, > - complain | tf_qualifying_scope, > - in_decl); > + scope = tsubst_scope (scope, args, complain, in_decl); > > tree name = DECL_NAME (t); > if (IDENTIFIER_CONV_OP_P (name) > @@ -16619,6 +16618,16 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) > } > } > > +/* Convenience wrapper over tsubst for substituting into the LHS > + of the :: scope resolution operator. */ > + > +static tree > +tsubst_scope (tree t, tree args, tsubst_flags_t complain, tree in_decl) > +{ > + gcc_checking_assert (TYPE_P (t)); > + return tsubst (t, args, complain | tf_qualifying_scope, in_decl); > +} > + > /* OLDFNS is a lookup set of member functions from some class template, and > NEWFNS is a lookup set of member functions from NEWTYPE, a specialization > of that class template. Return the subset of NEWFNS which are > @@ -16883,7 +16892,7 @@ tsubst_qualified_id (tree qualified_id, tree args, > scope = TREE_OPERAND (qualified_id, 0); > if (args) > { > - scope = tsubst (scope, args, complain | tf_qualifying_scope, in_decl); > + scope = tsubst_scope (scope, args, complain, in_decl); > expr = tsubst_copy (name, args, complain, in_decl); > } > else > @@ -17129,8 +17138,8 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl) > if (t == NULL_TREE || t == error_mark_node || args == NULL_TREE) > return t; > > - tsubst_flags_t qualifying_scope_flag = (complain & tf_qualifying_scope); > - complain &= ~tf_qualifying_scope; > + if (TYPE_P (t)) > + return tsubst (t, args, complain, in_decl); > > if (tree d = maybe_dependent_member_ref (t, args, complain, in_decl)) > return d; > @@ -17605,8 +17614,7 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl) > > case SCOPE_REF: > { > - tree op0 = tsubst_copy (TREE_OPERAND (t, 0), args, > - complain | tf_qualifying_scope, in_decl); > + tree op0 = tsubst_scope (TREE_OPERAND (t, 0), args, complain, in_decl); > tree op1 = tsubst_copy (TREE_OPERAND (t, 1), args, complain, in_decl); > return build_qualified_name (/*type=*/NULL_TREE, op0, op1, > QUALIFIED_NAME_IS_TEMPLATE (t)); > @@ -17702,26 +17710,9 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl) > return tree_cons (purpose, value, chain); > } > > - case RECORD_TYPE: > - case UNION_TYPE: > - case ENUMERAL_TYPE: > - case INTEGER_TYPE: > - case TEMPLATE_TYPE_PARM: > - case TEMPLATE_TEMPLATE_PARM: > - case BOUND_TEMPLATE_TEMPLATE_PARM: > case TEMPLATE_PARM_INDEX: > - case POINTER_TYPE: > - case REFERENCE_TYPE: > - case OFFSET_TYPE: > - case FUNCTION_TYPE: > - case METHOD_TYPE: > - case ARRAY_TYPE: > - case TYPENAME_TYPE: > - case UNBOUND_CLASS_TEMPLATE: > - case TYPEOF_TYPE: > - case DECLTYPE_TYPE: > case TYPE_DECL: > - return tsubst (t, args, complain | qualifying_scope_flag, in_decl); > + return tsubst (t, args, complain, in_decl); > > case USING_DECL: > t = DECL_NAME (t);