From: Jason Merrill <jason@redhat.com>
To: Patrick Palka <ppalka@redhat.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution
Date: Tue, 14 Feb 2023 17:49:51 -0500 [thread overview]
Message-ID: <9e5eb102-39ca-e327-23bb-6c154ee16940@redhat.com> (raw)
In-Reply-To: <1a709771-253a-780c-335e-a5819a063483@redhat.com>
On 2/14/23 14:15, 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.
tsubst_maybe_ctad_type?
>> 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.
>>
>> gcc/cp/ChangeLog:
>>
>> * cp-tree.h (enum tsubst_flags): Remove tf_keep_type_decl
>> and tf_tst_ok.
>> (make_typename_type): Add two trailing boolean parameters
>> defaulted to false.
>> * decl.cc (make_typename_type): Replace uses of
>> tf_keep_type_decl and tf_tst_ok with the corresponding new
>> boolean parameters.
>> * pt.cc (tsubst_typename_type): New, factored out from tsubst
>> and adjusted after removing tf_keep_type_decl and tf_tst_ok.
>> (tsubst_decl) <case VAR_DECL>: Conditionally call
>> tsubst_typename_type directly instead of using tf_tst_ok.
>> (tsubst) <case TYPENAME_TYPE>: Call tsubst_typename_type.
>> (tsubst_copy) <case CAST_EXPR>: Conditionally call
>> tsubst_typename_type directly instead of using tf_tst_ok.
>> (tsubst_copy_and_build) <case CAST_EXPR>: Likewise.
>> <case CONSTRUCTOR>: Likewise.
>> ---
>> gcc/cp/cp-tree.h | 9 +-
>> gcc/cp/decl.cc | 17 ++--
>> gcc/cp/pt.cc | 223 +++++++++++++++++++++++++----------------------
>> 3 files changed, 134 insertions(+), 115 deletions(-)
>>
>> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
>> index 06bc64a6b8d..a7c5765fc33 100644
>> --- a/gcc/cp/cp-tree.h
>> +++ b/gcc/cp/cp-tree.h
>> @@ -5573,8 +5573,7 @@ enum tsubst_flags {
>> tf_error = 1 << 0, /* give error messages */
>> tf_warning = 1 << 1, /* give warnings too */
>> tf_ignore_bad_quals = 1 << 2, /* ignore bad cvr qualifiers */
>> - tf_keep_type_decl = 1 << 3, /* retain typedef type decls
>> - (make_typename_type use) */
>> + /* 1 << 3 available */
>> tf_ptrmem_ok = 1 << 4, /* pointers to member ok (internal
>> instantiate_type use) */
>> tf_user = 1 << 5, /* found template must be a user template
>> @@ -5594,8 +5593,7 @@ enum tsubst_flags {
>> (build_target_expr and friends) */
>> tf_norm = 1 << 11, /* Build diagnostic information during
>> constraint normalization. */
>> - tf_tst_ok = 1 << 12, /* Allow a typename-specifier to name
>> - a template (C++17 or later). */
>> + /* 1 << 12 available */
>> tf_dguide = 1 << 13, /* Building a deduction guide from a
>> ctor. */
>> /* Convenient substitution flags combinations. */
>> tf_warning_or_error = tf_warning | tf_error
>> @@ -6846,7 +6844,8 @@ extern tree declare_local_label (tree);
>> extern tree define_label (location_t, tree);
>> extern void check_goto (tree);
>> extern bool check_omp_return (void);
>> -extern tree make_typename_type (tree, tree, enum
>> tag_types, tsubst_flags_t);
>> +extern tree make_typename_type (tree, tree, enum
>> tag_types, tsubst_flags_t,
>> + bool = false, bool = false);
>> extern tree build_typename_type (tree, tree, tree,
>> tag_types);
>> extern tree make_unbound_class_template (tree, tree, tree,
>> tsubst_flags_t);
>> extern tree make_unbound_class_template_raw (tree, tree, tree);
>> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
>> index d606b31d7a7..430533606b0 100644
>> --- a/gcc/cp/decl.cc
>> +++ b/gcc/cp/decl.cc
>> @@ -4228,14 +4228,17 @@ build_typename_type (tree context, tree name,
>> tree fullname,
>> /* Resolve `typename CONTEXT::NAME'. TAG_TYPE indicates the tag
>> provided to name the type. Returns an appropriate type, unless an
>> error occurs, in which case error_mark_node is returned. If we
>> - locate a non-artificial TYPE_DECL and TF_KEEP_TYPE_DECL is set, we
>> + locate a non-artificial TYPE_DECL and KEEP_TYPE_DECL is true, we
>> return that, rather than the _TYPE it corresponds to, in other
>> - cases we look through the type decl. If TF_ERROR is set, complain
>> - about errors, otherwise be quiet. */
>> + cases we look through the type decl. If TEMPLATE_OK is true and
>> + we found a TEMPLATE_DECL then we return a CTAD placeholder for the
>> + TEMPLATE_DECL. If TF_ERROR is set, complain about errors, otherwise
>> + be quiet. */
>> tree
>> make_typename_type (tree context, tree name, enum tag_types tag_type,
>> - tsubst_flags_t complain)
>> + tsubst_flags_t complain, bool keep_type_decl /* = false */,
>> + bool template_ok /* = false */)
>> {
>> tree fullname;
>> tree t;
>> @@ -4352,8 +4355,8 @@ make_typename_type (tree context, tree name,
>> enum tag_types tag_type,
>> }
>> if (!want_template && TREE_CODE (t) != TYPE_DECL)
>> {
>> - if ((complain & tf_tst_ok) && cxx_dialect >= cxx17
>> - && DECL_TYPE_TEMPLATE_P (t))
>> + if (template_ok && DECL_TYPE_TEMPLATE_P (t)
>> + && cxx_dialect >= cxx17)
>> /* The caller permits this typename-specifier to name a template
>> (because it appears in a CTAD-enabled context). */;
>> else
>> @@ -4383,7 +4386,7 @@ make_typename_type (tree context, tree name,
>> enum tag_types tag_type,
>> t = TYPE_NAME (t);
>> }
>> - if (DECL_ARTIFICIAL (t) || !(complain & tf_keep_type_decl))
>> + if (DECL_ARTIFICIAL (t) || !keep_type_decl)
>> t = TREE_TYPE (t);
>> maybe_record_typedef_use (t);
>> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
>> index e89dbf47097..bc47bf15d38 100644
>> --- a/gcc/cp/pt.cc
>> +++ b/gcc/cp/pt.cc
>> @@ -13949,6 +13949,94 @@ tsubst_aggr_type_1 (tree t,
>> return t;
>> }
>> +/* Substitute ARGS into the TYPENAME_TYPE T. The flag TEMPLATE_OK
>> + is passed to make_typename_type. */
>> +
>> +static tree
>> +tsubst_typename_type (tree t, tree args, tsubst_flags_t complain,
>> tree in_decl,
>> + bool template_ok = false)
>> +{
>> + tree ctx = TYPE_CONTEXT (t);
>> + if (TREE_CODE (ctx) == TYPE_PACK_EXPANSION)
>> + {
>> + ctx = tsubst_pack_expansion (ctx, args, complain, in_decl);
>> + if (ctx == error_mark_node
>> + || TREE_VEC_LENGTH (ctx) > 1)
>> + return error_mark_node;
>> + if (TREE_VEC_LENGTH (ctx) == 0)
>> + {
>> + if (complain & tf_error)
>> + error ("%qD is instantiated for an empty pack",
>> + TYPENAME_TYPE_FULLNAME (t));
>> + return error_mark_node;
>> + }
>> + ctx = TREE_VEC_ELT (ctx, 0);
>> + }
>> + else
>> + ctx = tsubst_aggr_type (ctx, args, complain, in_decl,
>> + /*entering_scope=*/1);
>> + if (ctx == error_mark_node)
>> + return error_mark_node;
>> +
>> + tree f = tsubst_copy (TYPENAME_TYPE_FULLNAME (t), args,
>> + complain, in_decl);
>> + if (f == error_mark_node)
>> + return error_mark_node;
>> +
>> + if (!MAYBE_CLASS_TYPE_P (ctx))
>> + {
>> + if (complain & tf_error)
>> + error ("%qT is not a class, struct, or union type", ctx);
>> + return error_mark_node;
>> + }
>> + else if (!uses_template_parms (ctx) && !TYPE_BEING_DEFINED (ctx))
>> + {
>> + /* Normally, make_typename_type does not require that the CTX
>> + have complete type in order to allow things like:
>> +
>> + template <class T> struct S { typename S<T>::X Y; };
>> +
>> + But, such constructs have already been resolved by this
>> + point, so here CTX really should have complete type, unless
>> + it's a partial instantiation. */
>> + if (!complete_type_or_maybe_complain (ctx, NULL_TREE, complain))
>> + return error_mark_node;
>> + }
>> +
>> + f = make_typename_type (ctx, f, typename_type, complain,
>> + /*keep_type_decl=*/true, template_ok);
>> + if (f == error_mark_node)
>> + return f;
>> + if (TREE_CODE (f) == TYPE_DECL)
>> + {
>> + complain |= tf_ignore_bad_quals;
>> + f = TREE_TYPE (f);
>> + }
>> +
>> + if (TREE_CODE (f) != TYPENAME_TYPE)
>> + {
>> + if (TYPENAME_IS_ENUM_P (t) && TREE_CODE (f) != ENUMERAL_TYPE)
>> + {
>> + if (complain & tf_error)
>> + error ("%qT resolves to %qT, which is not an enumeration type",
>> + t, f);
>> + else
>> + return error_mark_node;
>> + }
>> + else if (TYPENAME_IS_CLASS_P (t) && !CLASS_TYPE_P (f))
>> + {
>> + if (complain & tf_error)
>> + error ("%qT resolves to %qT, which is not a class type",
>> + t, f);
>> + else
>> + return error_mark_node;
>> + }
>> + }
>> +
>> + return cp_build_qualified_type
>> + (f, cp_type_quals (f) | cp_type_quals (t), complain);
>> +}
>> +
>> /* Map from a FUNCTION_DECL to a vec of default argument
>> instantiations,
>> indexed in reverse order of the parameters. */
>> @@ -15193,10 +15281,13 @@ tsubst_decl (tree t, tree args,
>> tsubst_flags_t complain)
>> && VAR_HAD_UNKNOWN_BOUND (t)
>> && type != error_mark_node)
>> type = strip_array_domain (type);
>> - tsubst_flags_t tcomplain = complain;
>> - if (VAR_P (t))
>> - tcomplain |= tf_tst_ok;
>> - type = tsubst (type, args, tcomplain, in_decl);
>> + if (VAR_P (t)
>> + && TREE_CODE (type) == TYPENAME_TYPE
>> + && !typedef_variant_p (type))
>> + type = tsubst_typename_type (type, args, complain, in_decl,
>> + /*template_ok=*/true);
>> + else
>> + type = tsubst (type, args, complain, in_decl);
>> /* Substituting the type might have recursively instantiated
>> this
>> same alias (c++/86171). */
>> if (gen_tmpl && DECL_ALIAS_TEMPLATE_P (gen_tmpl)
>> @@ -15889,9 +15980,6 @@ tsubst (tree t, tree args, tsubst_flags_t
>> complain, tree in_decl)
>> bool fndecl_type = (complain & tf_fndecl_type);
>> complain &= ~tf_fndecl_type;
>> - bool tst_ok = (complain & tf_tst_ok);
>> - complain &= ~tf_tst_ok;
>> -
>> if (type
>> && code != TYPENAME_TYPE
>> && code != TEMPLATE_TYPE_PARM
>> @@ -16424,89 +16512,7 @@ tsubst (tree t, tree args, tsubst_flags_t
>> complain, tree in_decl)
>> }
>> case TYPENAME_TYPE:
>> - {
>> - tree ctx = TYPE_CONTEXT (t);
>> - if (TREE_CODE (ctx) == TYPE_PACK_EXPANSION)
>> - {
>> - ctx = tsubst_pack_expansion (ctx, args, complain, in_decl);
>> - if (ctx == error_mark_node
>> - || TREE_VEC_LENGTH (ctx) > 1)
>> - return error_mark_node;
>> - if (TREE_VEC_LENGTH (ctx) == 0)
>> - {
>> - if (complain & tf_error)
>> - error ("%qD is instantiated for an empty pack",
>> - TYPENAME_TYPE_FULLNAME (t));
>> - return error_mark_node;
>> - }
>> - ctx = TREE_VEC_ELT (ctx, 0);
>> - }
>> - else
>> - ctx = tsubst_aggr_type (ctx, args, complain, in_decl,
>> - /*entering_scope=*/1);
>> - if (ctx == error_mark_node)
>> - return error_mark_node;
>> -
>> - tree f = tsubst_copy (TYPENAME_TYPE_FULLNAME (t), args,
>> - complain, in_decl);
>> - if (f == error_mark_node)
>> - return error_mark_node;
>> -
>> - if (!MAYBE_CLASS_TYPE_P (ctx))
>> - {
>> - if (complain & tf_error)
>> - error ("%qT is not a class, struct, or union type", ctx);
>> - return error_mark_node;
>> - }
>> - else if (!uses_template_parms (ctx) && !TYPE_BEING_DEFINED (ctx))
>> - {
>> - /* Normally, make_typename_type does not require that the CTX
>> - have complete type in order to allow things like:
>> -
>> - template <class T> struct S { typename S<T>::X Y; };
>> -
>> - But, such constructs have already been resolved by this
>> - point, so here CTX really should have complete type, unless
>> - it's a partial instantiation. */
>> - if (!complete_type_or_maybe_complain (ctx, NULL_TREE, complain))
>> - return error_mark_node;
>> - }
>> -
>> - tsubst_flags_t tcomplain = complain | tf_keep_type_decl;
>> - if (tst_ok)
>> - tcomplain |= tf_tst_ok;
>> - f = make_typename_type (ctx, f, typename_type, tcomplain);
>> - if (f == error_mark_node)
>> - return f;
>> - if (TREE_CODE (f) == TYPE_DECL)
>> - {
>> - complain |= tf_ignore_bad_quals;
>> - f = TREE_TYPE (f);
>> - }
>> -
>> - if (TREE_CODE (f) != TYPENAME_TYPE)
>> - {
>> - if (TYPENAME_IS_ENUM_P (t) && TREE_CODE (f) != ENUMERAL_TYPE)
>> - {
>> - if (complain & tf_error)
>> - error ("%qT resolves to %qT, which is not an enumeration
>> type",
>> - t, f);
>> - else
>> - return error_mark_node;
>> - }
>> - else if (TYPENAME_IS_CLASS_P (t) && !CLASS_TYPE_P (f))
>> - {
>> - if (complain & tf_error)
>> - error ("%qT resolves to %qT, which is not a class type",
>> - t, f);
>> - else
>> - return error_mark_node;
>> - }
>> - }
>> -
>> - return cp_build_qualified_type
>> - (f, cp_type_quals (f) | cp_type_quals (t), complain);
>> - }
>> + return tsubst_typename_type (t, args, complain, in_decl);
>> case UNBOUND_CLASS_TEMPLATE:
>> {
>> @@ -17391,10 +17397,14 @@ tsubst_copy (tree t, tree args,
>> tsubst_flags_t complain, tree in_decl)
>> case IMPLICIT_CONV_EXPR:
>> CASE_CONVERT:
>> {
>> - tsubst_flags_t tcomplain = complain;
>> - if (code == CAST_EXPR)
>> - tcomplain |= tf_tst_ok;
>> - tree type = tsubst (TREE_TYPE (t), args, tcomplain, in_decl);
>> + tree type = TREE_TYPE (t);
>> + if (code == CAST_EXPR
>> + && TREE_CODE (type) == TYPENAME_TYPE
>> + && !typedef_variant_p (type))
>> + type = tsubst_typename_type (type, args, complain, in_decl,
>> + /*template_ok=*/true);
>> + else
>> + type = tsubst (type, args, complain, in_decl);
>> tree op0 = tsubst_copy (TREE_OPERAND (t, 0), args, complain,
>> in_decl);
>> return build1 (code, type, op0);
>> }
>> @@ -20430,13 +20440,16 @@ tsubst_copy_and_build (tree t,
>> case DYNAMIC_CAST_EXPR:
>> case STATIC_CAST_EXPR:
>> {
>> - tree type;
>> + tree type = TREE_TYPE (t);
>> tree op, r = NULL_TREE;
>> - tsubst_flags_t tcomplain = complain;
>> - if (TREE_CODE (t) == CAST_EXPR)
>> - tcomplain |= tf_tst_ok;
>> - type = tsubst (TREE_TYPE (t), args, tcomplain, in_decl);
>> + if (TREE_CODE (t) == CAST_EXPR
>> + && TREE_CODE (type) == TYPENAME_TYPE
>> + && !typedef_variant_p (type))
>> + type = tsubst_typename_type (type, args, complain, in_decl,
>> + /*template_ok=*/true);
>> + else
>> + type = tsubst (type, args, complain, in_decl);
>> op = RECUR (TREE_OPERAND (t, 0));
>> @@ -21421,10 +21434,14 @@ tsubst_copy_and_build (tree t,
>> bool need_copy_p = false;
>> tree r;
>> - tsubst_flags_t tcomplain = complain;
>> - if (COMPOUND_LITERAL_P (t))
>> - tcomplain |= tf_tst_ok;
>> - tree type = tsubst (TREE_TYPE (t), args, tcomplain, in_decl);
>> + tree type = TREE_TYPE (t);
>> + if (COMPOUND_LITERAL_P (t)
>> + && TREE_CODE (type) == TYPENAME_TYPE
>> + && !typedef_variant_p (type))
>> + type = tsubst_typename_type (type, args, complain, in_decl,
>> + /*template_ok=*/true);
>> + else
>> + type = tsubst (type, args, complain, in_decl);
>> if (type == error_mark_node)
>> RETURN (error_mark_node);
>
next prev parent reply other threads:[~2023-02-14 22:49 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-13 17:23 Patrick Palka
2023-02-13 17:23 ` [PATCH 2/2] c++: TYPENAME_TYPE lookup ignoring non-types [PR107773] Patrick Palka
2023-02-14 22:16 ` Jason Merrill
2023-02-14 22:15 ` [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution Jason Merrill
2023-02-14 22:49 ` Jason Merrill [this message]
2023-02-15 17:21 ` Patrick Palka
2023-02-15 19:26 ` Jason Merrill
2023-02-15 20:11 ` Patrick Palka
2023-02-20 3:49 ` Jason Merrill
2023-02-22 0:05 ` Patrick Palka
2023-03-01 21:33 ` Jason Merrill
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9e5eb102-39ca-e327-23bb-6c154ee16940@redhat.com \
--to=jason@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=ppalka@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).