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 82A653856DF1 for ; Wed, 4 Oct 2023 16:08:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 82A653856DF1 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=1696435701; 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=uRm6jEr3SoIgYE7MnQj6rpIcqF/ujx+5rQPgdnBoYTE=; b=UXNqS2YfSyBnNoSPVVA5bXuGyBJ9+IjNe+l8Wp5tEth8KIzCTuSN6q9wDt7JKfZ6sXQuFL 5z519ILUhNSsNxBH1x2fd1UGFtHyBvNJe/+3ZbKxojrIRgb0rJrtP8KkRAzExYe2UQ1L93 yAvdsarP3kF/xQvMn5TMCPkKcVA1dN0= 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_256_GCM_SHA384) id us-mta-49-hNNU6SVkOSOTSW39FHzYvg-1; Wed, 04 Oct 2023 12:08:18 -0400 X-MC-Unique: hNNU6SVkOSOTSW39FHzYvg-1 Received: by mail-qv1-f69.google.com with SMTP id 6a1803df08f44-668f04867deso24047696d6.2 for ; Wed, 04 Oct 2023 09:08:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696435698; x=1697040498; 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=uRm6jEr3SoIgYE7MnQj6rpIcqF/ujx+5rQPgdnBoYTE=; b=ShNKX7Q2uip+7C5JgiWnsvgH0wk3rgYDXVyFV+y0fZkJ4wPRJ0sngZjSPcgdYifDfD 8lLX37jXwBa4B35Z7VHum3dX1KsbsWKXBNZwRjeLf8MTLGP1QozUREynKXcHfzDociMl S1kcRPI/3bDzwRYmV3Sd5fdj4ojek+w7pQ2TOMlzgk8UPHRThY8sm/2XWrjPSEQ/QgSQ DGkHax2nGAuJPbXRyHVnNkZVtko/tORuX/PDL0ssEoDDXMxyeAcgVJIOCF9Jrwf1wsiU vbsgP+Wvy/n7oK1EjNyROzbfFHJRnMpnEGwyNaJvSXqPqqescAJg3CqVaHC+wVU07XV5 GMew== X-Gm-Message-State: AOJu0Yy5K521D4yYqfZFPexDRf58sH0ZZju9DKql5f0KA1COwPB02F+o NEHMYq2dobiJ1zNplL8xZOkTDQ33HUSiBukQd68yHttYLU7XGOMbbbV+IsIobSE6Zm+zbkTI3ql GZtge3CKae/8sI1bOcQ== X-Received: by 2002:a0c:c44a:0:b0:658:2037:717c with SMTP id t10-20020a0cc44a000000b006582037717cmr2390743qvi.45.1696435697067; Wed, 04 Oct 2023 09:08:17 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG8KwbSOCmOyWAjB/cZQBfmD+NfwUEP2Abt9L9PemrwfTWDc2LMiqq/wi24x2+G6tsloWkSkg== X-Received: by 2002:a0c:c44a:0:b0:658:2037:717c with SMTP id t10-20020a0cc44a000000b006582037717cmr2390681qvi.45.1696435695984; Wed, 04 Oct 2023 09:08:15 -0700 (PDT) Received: from [192.168.1.130] (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id b5-20020a0cf045000000b0065b08bb01afsm1421773qvl.124.2023.10.04.09.08.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Oct 2023 09:08:15 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Wed, 4 Oct 2023 12:08:14 -0400 (EDT) To: Jason Merrill cc: Patrick Palka , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c++: merge tsubst_copy into tsubst_copy_and_build In-Reply-To: Message-ID: <662bb1db-fa27-1c59-2fa5-5986458cb074@idea> References: <20230925204302.1277285-2-ppalka@redhat.com> <20231002193738.3900509-1-ppalka@redhat.com> 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.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,WEIRD_PORT 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 Tue, 3 Oct 2023, Jason Merrill wrote: > On 10/3/23 08:41, Patrick Palka wrote: > > On Mon, 2 Oct 2023, Patrick Palka wrote: > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look > > > OK for trunk? > > > > > > -- >8 -- > > > > > > The relationship between tsubst_copy_and_build and tsubst_copy (two of > > > the main template argument substitution routines for expression trees) > > > is rather hazy. The former is mostly a superset of the latter, with > > > some differences. > > > > > > The main difference is that they handle many tree codes differently, but > > > much of the tree code handling in tsubst_copy appears to be dead code[1]. > > > This is because tsubst_copy only gets directly called in a few places > > > and mostly on id-expressions. The interesting exceptions are PARM_DECL, > > > VAR_DECL, BIT_NOT_EXPR, SCOPE_REF, TEMPLATE_ID_EXPR and IDENTIFIER_NODE: > > > > > > * for PARM_DECL and VAR_DECL, tsubst_copy_and_build calls tsubst_copy > > > followed by doing some extra handling of its own > > > * for BIT_NOT_EXPR tsubst_copy implicitly handles unresolved destructor > > > calls (i.e. the first operand is an identifier or a type) > > > * for SCOPE_REF, TEMPLATE_ID_EXPR and IDENTIFIER_NODE tsubst_copy > > > refrains from doing name lookup of the terminal name > > > > > > Other more minor differences are that tsubst_copy exits early when > > > 'args' is null, and it calls maybe_dependent_member_ref > > That's curious, since what that function does seems like name lookup; I > wouldn't think we would want to call it when tf_no_name_lookup. Ah, that makes sense I think. > > > > and finally it dispatches to tsubst for type trees. > > And it looks like you fix the callers to avoid that? Yes, I'll make a note of that in the commit message. > > > > Thus tsubst_copy is (at this point) similar enough to > > > tsubst_copy_and_build > > > that it makes sense to merge the two functions, with the main difference > > > being the name lookup behavior[2]. So this patch merges tsubst_copy into > > > tsubst_copy_and_build via a new tsubst tf_no_name_lookup which controls > > > name lookup and resolution of a (top-level) id-expression. > > > > > > [1]: http://thrifty.mooo.com:8008/gcc-lcov/gcc/cp/pt.cc.gcov.html#17231 > > > [2]: I don't know the history of tsubst_copy but I would guess it was > > > added before we settled on using processing_template_decl to control > > > whether our AST building routines perform semantic checking and return > > > non-templated trees, and so we needed a separate tsubst routine that > > > avoids semantic checking and always returns a templated tree for e.g. > > > partial substitution. > > > > Oops, this is wrong -- tsubst_copy_and_build came after tsubst_copy, > > and was introduced as an optimization with the intent of getting rid > > of tsubst_copy eventually: > > https://gcc.gnu.org/pipermail/gcc-patches/2003-January/093659.html > > I wonder if we want to add a small tsubst_name wrapper to call > tsubst_copy_and_build with tf_no_name_lookup? Good idea, that'll complement tsubst_scope nicely. > > Can we also merge in tsubst_expr and use that name instead of the unwieldy > tsubst_copy_and_build? That'd be nice. Another idea would be to rename tsubst_expr to tsubst_stmt and make it disjoint from tsubst_copy_and_build, and then rename tsubst_copy_and_build to tsubst_expr, to draw a distinction between statement-like trees (the substitution of which typically has side effects like calling add_stmt) and expression-like trees (which don't usually have such side effects). I can work on that as a follow-up patch. Here's v2 which guards the call to maybe_dependent_member_ref and adds tsubst_name, bootstrapped and regtested on x86_64-pc-linux-gnu: -- >8 -- Subject: [PATCH] c++: merge tsubst_copy into tsubst_copy_and_build gcc/cp/ChangeLog: * cp-tree.h (enum tsubst_flags): Add tf_no_name_lookup. * pt.cc (tsubst_copy): (tsubst_pack_expansion): Use tsubst for substituting BASES_TYPE. (tsubst_decl) : Use tsubst_name instead of tsubst_copy. (tsubst) : Use tsubst_copy_and_build instead of tsubst_copy for substituting CLASS_PLACEHOLDER_TEMPLATE. : Use tsubst_name instead of tsubst_copy for substituting TYPENAME_TYPE_FULLNAME. (tsubst_name): Define. (tsubst_qualified_id): Use tsubst_name instead of tsubst_copy for substituting the component name of a SCOPE_REF. (tsubst_copy): Remove. (tsubst_copy_and_build): Clear tf_no_name_lookup at the start, and remember if it was set. Call maybe_dependent_member_ref if tf_no_name_lookup was not set. : Don't do name lookup if tf_no_name_lookup was set. : If tf_no_name_lookup was set, use tsubst_name instead of tsubst_copy_and_build to substitute the template and don't finish the template-id. : Handle identifier and type operand (if tf_no_name_lookup was set). : Avoid trying to resolve a SCOPE_REF if tf_no_name_lookup was set by calling build_qualified_name directly instead of tsubst_qualified_id. : Handling of sizeof... copied from tsubst_copy. : Use tsubst_name instead of tsubst_copy to substitute a TEMPLATE_ID_EXPR callee naming an unresolved template. : Likewise to substitute the member. : Copied from tsubst_copy and merged with ... : ... these. Initial handling copied from tsubst_copy. Optimize local variable substitution by trying retrieve_local_specialization before checking uses_template_parms. : Copied from tsubst_copy. : Likewise. : Likewise. : Likewise. : Likewise. : Likewise. : Likewise. : Likewise. : Likewise. : Likewise. : Likewise. : Likewise. : Likewise. : Likewise. : Likewise. : Use tsubst and tsubst_copy_and_build instead of tsubst_copy. : Copied from tsubst_copy. (tsubst_initializer_list): Use tsubst and tsubst_copy_and_build instead of tsubst_copy. --- gcc/cp/cp-tree.h | 3 + gcc/cp/pt.cc | 1186 +++++++++++++++++----------------------------- 2 files changed, 445 insertions(+), 744 deletions(-) diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 8b9a7d58462..919eab34803 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -5619,6 +5619,9 @@ enum tsubst_flags { tf_qualifying_scope = 1 << 14, /* Substituting the LHS of the :: operator. Affects TYPENAME_TYPE resolution from make_typename_type. */ + tf_no_name_lookup = 1 << 15, /* Don't look up the terminal name of an + outermost id-expression, or resolve its + constituent template-ids or qualified-ids. */ /* Convenient substitution flags combinations. */ tf_warning_or_error = tf_warning | tf_error }; diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index 4400d429b6f..e801a224d1b 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -204,9 +204,9 @@ static void copy_default_args_to_explicit_spec (tree); static bool invalid_nontype_parm_type_p (tree, tsubst_flags_t); 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, bool = true); static tree tsubst_scope (tree, tree, tsubst_flags_t, tree); +static tree tsubst_name (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); @@ -13373,15 +13373,11 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain, if (TREE_CODE (parm_pack) == BASES) { gcc_assert (parm_pack == pattern); + tree type = tsubst (BASES_TYPE (parm_pack), args, complain, in_decl); if (BASES_DIRECT (parm_pack)) - return calculate_direct_bases (tsubst_expr (BASES_TYPE (parm_pack), - args, complain, - in_decl), - complain); + return calculate_direct_bases (type, complain); else - return calculate_bases (tsubst_expr (BASES_TYPE (parm_pack), - args, complain, in_decl), - complain); + return calculate_bases (type, complain); } else if (builtin_pack_call_p (parm_pack)) { @@ -15171,7 +15167,7 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain, variadic_p = true; } else - name = tsubst_copy (name, args, complain, in_decl); + name = tsubst_name (name, args, complain, in_decl); int len; if (!variadic_p) @@ -16108,7 +16104,7 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) if (template_placeholder_p (t)) { tree tmpl = CLASS_PLACEHOLDER_TEMPLATE (t); - tmpl = tsubst_copy (tmpl, args, complain, in_decl); + tmpl = tsubst_copy_and_build (tmpl, args, complain, in_decl); if (TREE_CODE (tmpl) == TEMPLATE_TEMPLATE_PARM) tmpl = TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (tmpl); @@ -16592,7 +16588,7 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) if (ctx == error_mark_node) return error_mark_node; - tree f = tsubst_copy (TYPENAME_TYPE_FULLNAME (t), args, + tree f = tsubst_name (TYPENAME_TYPE_FULLNAME (t), args, complain, in_decl); if (f == error_mark_node) return error_mark_node; @@ -16780,6 +16776,15 @@ tsubst_scope (tree t, tree args, tsubst_flags_t complain, tree in_decl) return tsubst (t, args, complain | tf_qualifying_scope, in_decl); } +/* Convenience wrapper over tsubst for substituting into an id-expression + without resolving its terminal name. */ + +static tree +tsubst_name (tree t, tree args, tsubst_flags_t complain, tree in_decl) +{ + return tsubst_copy_and_build (t, args, complain | tf_no_name_lookup, 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 @@ -17045,7 +17050,7 @@ tsubst_qualified_id (tree qualified_id, tree args, if (args) { scope = tsubst_scope (scope, args, complain, in_decl); - expr = tsubst_copy (name, args, complain, in_decl); + expr = tsubst_name (name, args, complain, in_decl); } else expr = name; @@ -17277,707 +17282,6 @@ maybe_dependent_member_ref (tree t, tree args, tsubst_flags_t complain, TREE_CODE (t) == TEMPLATE_DECL); } -/* Like tsubst, but deals with expressions. This function just replaces - template parms; to finish processing the resultant expression, use - tsubst_copy_and_build or tsubst_expr. */ - -static tree -tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl) -{ - enum tree_code code; - tree r; - - if (t == NULL_TREE || t == error_mark_node || args == NULL_TREE) - return t; - - 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; - - code = TREE_CODE (t); - - switch (code) - { - case PARM_DECL: - r = retrieve_local_specialization (t); - - if (r == NULL_TREE) - { - /* We get here for a use of 'this' in an NSDMI. */ - if (DECL_NAME (t) == this_identifier && current_class_ptr) - return current_class_ptr; - - /* This can happen for a parameter name used later in a function - declaration (such as in a late-specified return type). Just - make a dummy decl, since it's only used for its type. */ - gcc_assert (cp_unevaluated_operand); - r = tsubst_decl (t, args, complain); - /* Give it the template pattern as its context; its true context - hasn't been instantiated yet and this is good enough for - mangling. */ - DECL_CONTEXT (r) = DECL_CONTEXT (t); - } - - if (TREE_CODE (r) == ARGUMENT_PACK_SELECT) - r = argument_pack_select_arg (r); - if (!mark_used (r, complain) && !(complain & tf_error)) - return error_mark_node; - return r; - - case CONST_DECL: - { - tree enum_type; - tree v; - - if (DECL_TEMPLATE_PARM_P (t)) - return tsubst_copy (DECL_INITIAL (t), args, complain, in_decl); - if (!uses_template_parms (DECL_CONTEXT (t))) - return t; - - /* Unfortunately, we cannot just call lookup_name here. - Consider: - - template int f() { - enum E { a = I }; - struct S { void g() { E e = a; } }; - }; - - When we instantiate f<7>::S::g(), say, lookup_name is not - clever enough to find f<7>::a. */ - enum_type - = tsubst_aggr_type (DECL_CONTEXT (t), args, complain, in_decl, - /*entering_scope=*/0); - - for (v = TYPE_VALUES (enum_type); - v != NULL_TREE; - v = TREE_CHAIN (v)) - if (TREE_PURPOSE (v) == DECL_NAME (t)) - return TREE_VALUE (v); - - /* We didn't find the name. That should never happen; if - name-lookup found it during preliminary parsing, we - should find it again here during instantiation. */ - gcc_unreachable (); - } - return t; - - case FIELD_DECL: - if (DECL_CONTEXT (t)) - { - tree ctx; - - ctx = tsubst_aggr_type (DECL_CONTEXT (t), args, complain, in_decl, - /*entering_scope=*/1); - if (ctx != DECL_CONTEXT (t)) - { - tree r = lookup_field (ctx, DECL_NAME (t), 0, false); - if (!r) - { - if (complain & tf_error) - error ("using invalid field %qD", t); - return error_mark_node; - } - return r; - } - } - - return t; - - case VAR_DECL: - case FUNCTION_DECL: - if (DECL_LANG_SPECIFIC (t) && DECL_TEMPLATE_INFO (t)) - r = tsubst (t, args, complain, in_decl); - else if (DECL_LOCAL_DECL_P (t)) - { - /* Local specialization will usually have been created when - we instantiated the DECL_EXPR_DECL. */ - r = retrieve_local_specialization (t); - if (!r) - { - /* We're in a generic lambda referencing a local extern - from an outer block-scope of a non-template. */ - gcc_checking_assert (LAMBDA_FUNCTION_P (current_function_decl)); - r = t; - } - } - else if (local_variable_p (t) - && uses_template_parms (DECL_CONTEXT (t))) - { - r = retrieve_local_specialization (t); - if (r == NULL_TREE) - { - /* First try name lookup to find the instantiation. */ - r = lookup_name (DECL_NAME (t)); - if (r) - { - if (!VAR_P (r)) - { - /* During error-recovery we may find a non-variable, - even an OVERLOAD: just bail out and avoid ICEs and - duplicate diagnostics (c++/62207). */ - gcc_assert (seen_error ()); - return error_mark_node; - } - if (!is_capture_proxy (r)) - { - /* Make sure the one we found is the one we want. */ - tree ctx = enclosing_instantiation_of (DECL_CONTEXT (t)); - if (ctx != DECL_CONTEXT (r)) - r = NULL_TREE; - } - } - - if (r) - /* OK */; - else - { - /* This can happen for a variable used in a - late-specified return type of a local lambda, or for a - local static or constant. Building a new VAR_DECL - should be OK in all those cases. */ - r = tsubst_decl (t, args, complain); - if (local_specializations) - /* Avoid infinite recursion (79640). */ - register_local_specialization (r, t); - if (decl_maybe_constant_var_p (r)) - { - /* We can't call cp_finish_decl, so handle the - initializer by hand. */ - tree init = tsubst_init (DECL_INITIAL (t), r, args, - complain, in_decl); - if (!processing_template_decl) - init = maybe_constant_init (init); - if (processing_template_decl - ? potential_constant_expression (init) - : reduced_constant_expression_p (init)) - DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (r) - = TREE_CONSTANT (r) = true; - DECL_INITIAL (r) = init; - if (tree auto_node = type_uses_auto (TREE_TYPE (r))) - TREE_TYPE (r) - = do_auto_deduction (TREE_TYPE (r), init, auto_node, - complain, adc_variable_type); - } - gcc_assert (cp_unevaluated_operand - || processing_contract_condition - || TREE_STATIC (r) - || decl_constant_var_p (r) - || seen_error ()); - if (!processing_template_decl - && !TREE_STATIC (r)) - r = process_outer_var_ref (r, complain); - } - /* Remember this for subsequent uses. */ - if (local_specializations) - register_local_specialization (r, t); - } - if (TREE_CODE (r) == ARGUMENT_PACK_SELECT) - r = argument_pack_select_arg (r); - } - else - r = t; - if (!mark_used (r, complain)) - return error_mark_node; - return r; - - case NAMESPACE_DECL: - return t; - - case OVERLOAD: - return t; - - case BASELINK: - return tsubst_baselink (t, current_nonlambda_class_type (), - args, complain, in_decl); - - case TEMPLATE_DECL: - if (DECL_TEMPLATE_TEMPLATE_PARM_P (t)) - return tsubst (TREE_TYPE (DECL_TEMPLATE_RESULT (t)), - args, complain, in_decl); - else if (DECL_FUNCTION_TEMPLATE_P (t) && DECL_MEMBER_TEMPLATE_P (t)) - return tsubst (t, args, complain, in_decl); - else if (DECL_CLASS_SCOPE_P (t) - && uses_template_parms (DECL_CONTEXT (t))) - { - /* Template template argument like the following example need - special treatment: - - template