* [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution @ 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:15 ` [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution Jason Merrill 0 siblings, 2 replies; 11+ messages in thread From: Patrick Palka @ 2023-02-13 17:23 UTC (permalink / raw) To: gcc-patches; +Cc: jason, Patrick Palka [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. 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); -- 2.39.1.433.g23c56f7bd5 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] c++: TYPENAME_TYPE lookup ignoring non-types [PR107773] 2023-02-13 17:23 [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution Patrick Palka @ 2023-02-13 17:23 ` 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 1 sibling, 1 reply; 11+ messages in thread From: Patrick Palka @ 2023-02-13 17:23 UTC (permalink / raw) To: gcc-patches; +Cc: jason, Patrick Palka [N.B. this is a corrected version of https://gcc.gnu.org/pipermail/gcc-patches/2022-November/607443.html ] Currently when resolving a TYPENAME_TYPE for 'typename T::m' via make_typename_type, we consider only type bindings of 'm' and ignore non-type ones. But [temp.res.general]/3 says, in a note, "the usual qualified name lookup ([basic.lookup.qual]) applies even in the presence of 'typename'", and qualified name lookup doesn't discriminate between type and non-type bindings. So when resolving such a TYPENAME_TYPE we want the lookup to consider all bindings. An exception is when we have a TYPENAME_TYPE corresponding to the qualifying scope appearing before the :: scope resolution operator, such as 'T::type' in 'typename T::type::m'. In that case, [basic.lookup.qual]/1 applies, and lookup for such a TYPENAME_TYPE must ignore non-type bindings. So in order to correctly handle all cases, make_typename_type needs an additional flag controlling whether lookup should ignore non-types or not. To that end this patch adds a type_only flag to make_typename_type and defaults it to false (do not ignore non-types). In contexts where we do want to ignore non-types (when substituting into the scope of a TYPENAME_TYPE, SCOPE_REF, USING_DECL) we call tsubst_typename_type directly with type_only=true. Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk? PR c++/107773 gcc/cp/ChangeLog: * cp-tree.h (make_typename_type): Add another boolean parameter that defaults to false. * decl.cc (make_typename_type): Use lookup_member instead of lookup_field. Pass want_type=type_only instead of =false to lookup_member. Generalize format specifier in diagnostic to handle both type and non-type bindings. * pt.cc (tsubst_typename_type): Add another boolean parameter that defaults to false and pass it to make_typename_type. If TYPE_CONTEXT is a TYPENAME_TYPE recurse with type_only=true instead of substituting it via tsubst. (tsubst_decl) <case USING_DECL>: If the scpoe is a TYPENAME_TYPE call tsubst_typename_type directly with type_only=true instead of substituting it via tsubst. (tsubst_qualified_id): Likewise. * search.cc (lookup_member): Document default argument. gcc/testsuite/ChangeLog: * g++.dg/template/typename24.C: New test. * g++.dg/template/typename25.C: New test. * g++.dg/template/typename26.C: New test. --- gcc/cp/cp-tree.h | 2 +- gcc/cp/decl.cc | 14 ++++----- gcc/cp/pt.cc | 24 +++++++++++---- gcc/cp/search.cc | 2 +- gcc/testsuite/g++.dg/template/typename24.C | 18 ++++++++++++ gcc/testsuite/g++.dg/template/typename25.C | 34 ++++++++++++++++++++++ gcc/testsuite/g++.dg/template/typename26.C | 20 +++++++++++++ 7 files changed, 100 insertions(+), 14 deletions(-) create mode 100644 gcc/testsuite/g++.dg/template/typename24.C create mode 100644 gcc/testsuite/g++.dg/template/typename25.C create mode 100644 gcc/testsuite/g++.dg/template/typename26.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index a7c5765fc33..1241dbf8037 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6845,7 +6845,7 @@ 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, - bool = false, bool = false); + bool = false, 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 430533606b0..c741dc23d99 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -4232,13 +4232,14 @@ build_typename_type (tree context, tree name, tree fullname, return that, rather than the _TYPE it corresponds to, in other 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. */ + TEMPLATE_DECL. If TYPE_ONLY is true, lookup of NAME in CONTEXT + ignores non-type bindings. 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, bool keep_type_decl /* = false */, - bool template_ok /* = false */) + bool template_ok /* = false */, bool type_only /* = false */) { tree fullname; tree t; @@ -4308,9 +4309,8 @@ make_typename_type (tree context, tree name, enum tag_types tag_type, member of the current instantiation or a non-dependent base; lookup will stop when we hit a dependent base. */ if (!dependent_scope_p (context)) - /* We should only set WANT_TYPE when we're a nested typename type. - Then we can give better diagnostics if we find a non-type. */ - t = lookup_field (context, name, 2, /*want_type=*/true); + t = lookup_member (context, name, /*protect=*/2, + /*want_type=*/type_only, complain); else t = NULL_TREE; @@ -4362,7 +4362,7 @@ make_typename_type (tree context, tree name, enum tag_types tag_type, else { if (complain & tf_error) - error ("%<typename %T::%D%> names %q#T, which is not a type", + error ("%<typename %T::%D%> names %q#D, which is not a type", context, name, t); return error_mark_node; } diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index bc47bf15d38..27fc0b93484 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -13949,12 +13949,12 @@ 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. */ +/* Substitute ARGS into the TYPENAME_TYPE T. The flags TEMPLATE_OK and + TYPE_ONLY are 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) + bool template_ok = false, bool type_only = false) { tree ctx = TYPE_CONTEXT (t); if (TREE_CODE (ctx) == TYPE_PACK_EXPANSION) @@ -13972,6 +13972,10 @@ tsubst_typename_type (tree t, tree args, tsubst_flags_t complain, tree in_decl, } ctx = TREE_VEC_ELT (ctx, 0); } + else if (TREE_CODE (ctx) == TYPENAME_TYPE + && !typedef_variant_p (ctx)) + ctx = tsubst_typename_type (ctx, args, complain, in_decl, + /*template_ok=*/false, /*type_only=*/true); else ctx = tsubst_aggr_type (ctx, args, complain, in_decl, /*entering_scope=*/1); @@ -14004,7 +14008,7 @@ tsubst_typename_type (tree t, tree args, tsubst_flags_t complain, tree in_decl, } f = make_typename_type (ctx, f, typename_type, complain, - /*keep_type_decl=*/true, template_ok); + /*keep_type_decl=*/true, template_ok, type_only); if (f == error_mark_node) return f; if (TREE_CODE (f) == TYPE_DECL) @@ -15094,6 +15098,11 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain) scope = tsubst_pack_expansion (scope, args, complain, in_decl); variadic_p = true; } + else if (TREE_CODE (scope) == TYPENAME_TYPE + && !typedef_variant_p (scope)) + scope = tsubst_typename_type (scope, args, complain, in_decl, + /*template_ok=*/false, + /*type_only=*/true); else scope = tsubst_copy (scope, args, complain, in_decl); @@ -16885,7 +16894,12 @@ tsubst_qualified_id (tree qualified_id, tree args, scope = TREE_OPERAND (qualified_id, 0); if (args) { - scope = tsubst (scope, args, complain, in_decl); + if (TREE_CODE (scope) == TYPENAME_TYPE + && !typedef_variant_p (scope)) + scope = tsubst_typename_type (scope, args, complain, in_decl, + /*template_ok=*/false, /*type_only=*/true); + else + scope = tsubst (scope, args, complain, in_decl); expr = tsubst_copy (name, args, complain, in_decl); } else diff --git a/gcc/cp/search.cc b/gcc/cp/search.cc index f3f19cafec6..e472a97679d 100644 --- a/gcc/cp/search.cc +++ b/gcc/cp/search.cc @@ -1109,7 +1109,7 @@ build_baselink (tree binfo, tree access_binfo, tree functions, tree optype) tree lookup_member (tree xbasetype, tree name, int protect, bool want_type, - tsubst_flags_t complain, access_failure_info *afi) + tsubst_flags_t complain, access_failure_info *afi /* = NULL */) { tree rval, rval_binfo = NULL_TREE; tree type = NULL_TREE, basetype_path = NULL_TREE; diff --git a/gcc/testsuite/g++.dg/template/typename24.C b/gcc/testsuite/g++.dg/template/typename24.C new file mode 100644 index 00000000000..8b2b3718442 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/typename24.C @@ -0,0 +1,18 @@ +// PR c++/107773 +// Verify lookup for a non-neste TYPENAME_TYPE correctly considers +// non-types. + +struct a { + typedef void get; +}; + +struct b : a { + int get(int i) const; +}; + +template<class T> +void f() { + typedef typename T::get type; // { dg-error "'int b::get\\(int\\) const', which is not a type" } +} + +template void f<b>(); diff --git a/gcc/testsuite/g++.dg/template/typename25.C b/gcc/testsuite/g++.dg/template/typename25.C new file mode 100644 index 00000000000..924330ee8d4 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/typename25.C @@ -0,0 +1,34 @@ +// PR c++/107773 +// Verify lookup for TYPENAME_TYPE appearing to the left of the scope (::) +// operator in various contexts correctly ignores non-types. + +struct a { + typedef void type; +}; + +struct c { + struct b : a { + typedef b self; + static int m; + }; + int b; +}; + +template<class T> +void f() { + // T::b::type is a TYPENAME_TYPE whose TYPE_CONTEXT is a nested + // TYPENAME_TYPE. + typedef typename T::b::type type; + // T::b::m is a SCOPE_REF whose first operand is a TYPENAME_TYPE. + int m = T::b::m; +} + +template void f<c>(); + +template<class T> +struct d : T::b::self { + // The using is a USING_DECL whose USING_DECL_SCOPE is a TYPENAME_TYPE. + using typename T::b::type; +}; + +template struct d<c>; diff --git a/gcc/testsuite/g++.dg/template/typename26.C b/gcc/testsuite/g++.dg/template/typename26.C new file mode 100644 index 00000000000..4e6b764a97b --- /dev/null +++ b/gcc/testsuite/g++.dg/template/typename26.C @@ -0,0 +1,20 @@ +// Example 4 from [temp.res.general]/3. + +struct A { + struct X { }; + int X; +}; +struct B { + struct X { }; +}; +template<class T> void f(T t) { + typename T::X x; // { dg-error "'int A::X', which is not a type" } +} +void foo() { + A a; + B b; + f(b); // OK, T::X refers to B::X + // { dg-bogus "" "" { target *-*-* } .-1 } + f(a); // error: T::X refers to the data member A::X not the struct A::X + // { dg-message "required from here" "" { target *-*-* } .-1 } +} -- 2.39.1.433.g23c56f7bd5 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] c++: TYPENAME_TYPE lookup ignoring non-types [PR107773] 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 0 siblings, 0 replies; 11+ messages in thread From: Jason Merrill @ 2023-02-14 22:16 UTC (permalink / raw) To: Patrick Palka, gcc-patches 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 ] > > Currently when resolving a TYPENAME_TYPE for 'typename T::m' via > make_typename_type, we consider only type bindings of 'm' and ignore > non-type ones. But [temp.res.general]/3 says, in a note, "the usual > qualified name lookup ([basic.lookup.qual]) applies even in the presence > of 'typename'", and qualified name lookup doesn't discriminate between > type and non-type bindings. So when resolving such a TYPENAME_TYPE > we want the lookup to consider all bindings. > > An exception is when we have a TYPENAME_TYPE corresponding to the > qualifying scope appearing before the :: scope resolution operator, such > as 'T::type' in 'typename T::type::m'. In that case, [basic.lookup.qual]/1 > applies, and lookup for such a TYPENAME_TYPE must ignore non-type > bindings. So in order to correctly handle all cases, make_typename_type > needs an additional flag controlling whether lookup should ignore > non-types or not. > > To that end this patch adds a type_only flag to make_typename_type and > defaults it to false (do not ignore non-types). In contexts where we do > want to ignore non-types (when substituting into the scope of a > TYPENAME_TYPE, SCOPE_REF, USING_DECL) we call tsubst_typename_type > directly with type_only=true. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk? > > PR c++/107773 > > gcc/cp/ChangeLog: > > * cp-tree.h (make_typename_type): Add another boolean parameter > that defaults to false. > * decl.cc (make_typename_type): Use lookup_member instead of > lookup_field. Pass want_type=type_only instead of =false to > lookup_member. Generalize format specifier in diagnostic to > handle both type and non-type bindings. > * pt.cc (tsubst_typename_type): Add another boolean parameter > that defaults to false and pass it to make_typename_type. If > TYPE_CONTEXT is a TYPENAME_TYPE recurse with type_only=true > instead of substituting it via tsubst. > (tsubst_decl) <case USING_DECL>: If the scpoe is a TYPENAME_TYPE > call tsubst_typename_type directly with type_only=true instead > of substituting it via tsubst. > (tsubst_qualified_id): Likewise. > * search.cc (lookup_member): Document default argument. > > gcc/testsuite/ChangeLog: > > * g++.dg/template/typename24.C: New test. > * g++.dg/template/typename25.C: New test. > * g++.dg/template/typename26.C: New test. > --- > gcc/cp/cp-tree.h | 2 +- > gcc/cp/decl.cc | 14 ++++----- > gcc/cp/pt.cc | 24 +++++++++++---- > gcc/cp/search.cc | 2 +- > gcc/testsuite/g++.dg/template/typename24.C | 18 ++++++++++++ > gcc/testsuite/g++.dg/template/typename25.C | 34 ++++++++++++++++++++++ > gcc/testsuite/g++.dg/template/typename26.C | 20 +++++++++++++ > 7 files changed, 100 insertions(+), 14 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/template/typename24.C > create mode 100644 gcc/testsuite/g++.dg/template/typename25.C > create mode 100644 gcc/testsuite/g++.dg/template/typename26.C > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index a7c5765fc33..1241dbf8037 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -6845,7 +6845,7 @@ 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, > - bool = false, bool = false); > + bool = false, 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 430533606b0..c741dc23d99 100644 > --- a/gcc/cp/decl.cc > +++ b/gcc/cp/decl.cc > @@ -4232,13 +4232,14 @@ build_typename_type (tree context, tree name, tree fullname, > return that, rather than the _TYPE it corresponds to, in other > 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. */ > + TEMPLATE_DECL. If TYPE_ONLY is true, lookup of NAME in CONTEXT > + ignores non-type bindings. 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, bool keep_type_decl /* = false */, > - bool template_ok /* = false */) > + bool template_ok /* = false */, bool type_only /* = false */) > { > tree fullname; > tree t; > @@ -4308,9 +4309,8 @@ make_typename_type (tree context, tree name, enum tag_types tag_type, > member of the current instantiation or a non-dependent base; > lookup will stop when we hit a dependent base. */ > if (!dependent_scope_p (context)) > - /* We should only set WANT_TYPE when we're a nested typename type. > - Then we can give better diagnostics if we find a non-type. */ > - t = lookup_field (context, name, 2, /*want_type=*/true); > + t = lookup_member (context, name, /*protect=*/2, > + /*want_type=*/type_only, complain); > else > t = NULL_TREE; > > @@ -4362,7 +4362,7 @@ make_typename_type (tree context, tree name, enum tag_types tag_type, > else > { > if (complain & tf_error) > - error ("%<typename %T::%D%> names %q#T, which is not a type", > + error ("%<typename %T::%D%> names %q#D, which is not a type", > context, name, t); > return error_mark_node; > } > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index bc47bf15d38..27fc0b93484 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -13949,12 +13949,12 @@ 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. */ > +/* Substitute ARGS into the TYPENAME_TYPE T. The flags TEMPLATE_OK and > + TYPE_ONLY are 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) > + bool template_ok = false, bool type_only = false) > { > tree ctx = TYPE_CONTEXT (t); > if (TREE_CODE (ctx) == TYPE_PACK_EXPANSION) > @@ -13972,6 +13972,10 @@ tsubst_typename_type (tree t, tree args, tsubst_flags_t complain, tree in_decl, > } > ctx = TREE_VEC_ELT (ctx, 0); > } > + else if (TREE_CODE (ctx) == TYPENAME_TYPE > + && !typedef_variant_p (ctx)) > + ctx = tsubst_typename_type (ctx, args, complain, in_decl, > + /*template_ok=*/false, /*type_only=*/true); > else > ctx = tsubst_aggr_type (ctx, args, complain, in_decl, > /*entering_scope=*/1); > @@ -14004,7 +14008,7 @@ tsubst_typename_type (tree t, tree args, tsubst_flags_t complain, tree in_decl, > } > > f = make_typename_type (ctx, f, typename_type, complain, > - /*keep_type_decl=*/true, template_ok); > + /*keep_type_decl=*/true, template_ok, type_only); > if (f == error_mark_node) > return f; > if (TREE_CODE (f) == TYPE_DECL) > @@ -15094,6 +15098,11 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain) > scope = tsubst_pack_expansion (scope, args, complain, in_decl); > variadic_p = true; > } > + else if (TREE_CODE (scope) == TYPENAME_TYPE > + && !typedef_variant_p (scope)) > + scope = tsubst_typename_type (scope, args, complain, in_decl, > + /*template_ok=*/false, > + /*type_only=*/true); > else > scope = tsubst_copy (scope, args, complain, in_decl); > > @@ -16885,7 +16894,12 @@ tsubst_qualified_id (tree qualified_id, tree args, > scope = TREE_OPERAND (qualified_id, 0); > if (args) > { > - scope = tsubst (scope, args, complain, in_decl); > + if (TREE_CODE (scope) == TYPENAME_TYPE > + && !typedef_variant_p (scope)) > + scope = tsubst_typename_type (scope, args, complain, in_decl, > + /*template_ok=*/false, /*type_only=*/true); > + else > + scope = tsubst (scope, args, complain, in_decl); Maybe we want a tsubst_scope function? > expr = tsubst_copy (name, args, complain, in_decl); > } > else > diff --git a/gcc/cp/search.cc b/gcc/cp/search.cc > index f3f19cafec6..e472a97679d 100644 > --- a/gcc/cp/search.cc > +++ b/gcc/cp/search.cc > @@ -1109,7 +1109,7 @@ build_baselink (tree binfo, tree access_binfo, tree functions, tree optype) > > tree > lookup_member (tree xbasetype, tree name, int protect, bool want_type, > - tsubst_flags_t complain, access_failure_info *afi) > + tsubst_flags_t complain, access_failure_info *afi /* = NULL */) > { > tree rval, rval_binfo = NULL_TREE; > tree type = NULL_TREE, basetype_path = NULL_TREE; > diff --git a/gcc/testsuite/g++.dg/template/typename24.C b/gcc/testsuite/g++.dg/template/typename24.C > new file mode 100644 > index 00000000000..8b2b3718442 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/typename24.C > @@ -0,0 +1,18 @@ > +// PR c++/107773 > +// Verify lookup for a non-neste TYPENAME_TYPE correctly considers > +// non-types. > + > +struct a { > + typedef void get; > +}; > + > +struct b : a { > + int get(int i) const; > +}; > + > +template<class T> > +void f() { > + typedef typename T::get type; // { dg-error "'int b::get\\(int\\) const', which is not a type" } > +} > + > +template void f<b>(); > diff --git a/gcc/testsuite/g++.dg/template/typename25.C b/gcc/testsuite/g++.dg/template/typename25.C > new file mode 100644 > index 00000000000..924330ee8d4 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/typename25.C > @@ -0,0 +1,34 @@ > +// PR c++/107773 > +// Verify lookup for TYPENAME_TYPE appearing to the left of the scope (::) > +// operator in various contexts correctly ignores non-types. > + > +struct a { > + typedef void type; > +}; > + > +struct c { > + struct b : a { > + typedef b self; > + static int m; > + }; > + int b; > +}; > + > +template<class T> > +void f() { > + // T::b::type is a TYPENAME_TYPE whose TYPE_CONTEXT is a nested > + // TYPENAME_TYPE. > + typedef typename T::b::type type; > + // T::b::m is a SCOPE_REF whose first operand is a TYPENAME_TYPE. > + int m = T::b::m; > +} > + > +template void f<c>(); > + > +template<class T> > +struct d : T::b::self { > + // The using is a USING_DECL whose USING_DECL_SCOPE is a TYPENAME_TYPE. > + using typename T::b::type; > +}; > + > +template struct d<c>; > diff --git a/gcc/testsuite/g++.dg/template/typename26.C b/gcc/testsuite/g++.dg/template/typename26.C > new file mode 100644 > index 00000000000..4e6b764a97b > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/typename26.C > @@ -0,0 +1,20 @@ > +// Example 4 from [temp.res.general]/3. > + > +struct A { > + struct X { }; > + int X; > +}; > +struct B { > + struct X { }; > +}; > +template<class T> void f(T t) { > + typename T::X x; // { dg-error "'int A::X', which is not a type" } > +} > +void foo() { > + A a; > + B b; > + f(b); // OK, T::X refers to B::X > + // { dg-bogus "" "" { target *-*-* } .-1 } > + f(a); // error: T::X refers to the data member A::X not the struct A::X > + // { dg-message "required from here" "" { target *-*-* } .-1 } > +} ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution 2023-02-13 17:23 [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution Patrick Palka 2023-02-13 17:23 ` [PATCH 2/2] c++: TYPENAME_TYPE lookup ignoring non-types [PR107773] Patrick Palka @ 2023-02-14 22:15 ` Jason Merrill 2023-02-14 22:49 ` Jason Merrill 2023-02-15 17:21 ` Patrick Palka 1 sibling, 2 replies; 11+ messages in thread From: Jason Merrill @ 2023-02-14 22:15 UTC (permalink / raw) To: Patrick Palka, gcc-patches 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. > 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); > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution 2023-02-14 22:15 ` [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution Jason Merrill @ 2023-02-14 22:49 ` Jason Merrill 2023-02-15 17:21 ` Patrick Palka 1 sibling, 0 replies; 11+ messages in thread From: Jason Merrill @ 2023-02-14 22:49 UTC (permalink / raw) To: Patrick Palka, gcc-patches 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); > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution 2023-02-14 22:15 ` [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution Jason Merrill 2023-02-14 22:49 ` Jason Merrill @ 2023-02-15 17:21 ` Patrick Palka 2023-02-15 19:26 ` Jason Merrill 1 sibling, 1 reply; 11+ messages in thread From: Patrick Palka @ 2023-02-15 17:21 UTC (permalink / raw) To: Jason Merrill; +Cc: Patrick Palka, gcc-patches 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. -- >8 -- Subject: [PATCH] c++: TYPENAME_TYPE lookup ignoring non-types [PR107773] Currently when resolving a TYPENAME_TYPE for 'typename T::m' via make_typename_type, we consider only type bindings of 'm' and ignore non-type ones. But [temp.res.general]/3 says, in a note, "the usual qualified name lookup ([basic.lookup.qual]) applies even in the presence of 'typename'", and qualified name lookup doesn't discriminate between type and non-type bindings. So when resolving such a TYPENAME_TYPE we want the lookup to consider all bindings. An exception is when we have a TYPENAME_TYPE corresponding to the qualifying scope of the :: scope resolution operator, such as 'T::type' in 'typename T::type::m'. In that case, [basic.lookup.qual]/1 applies, and lookup for such a TYPENAME_TYPE must ignore non-type bindings. So in order to correctly handle all cases, make_typename_type needs an additional flag controlling whether lookup should ignore non-types or not. To that end this patch adds a new tsubst flag tf_qualifying_scope to communicate to make_typename_type whether we want to ignore non-type bindings during the lookup (by default we don't want to ignore them). In contexts where we do want to ignore non-types (when substituting into the scope of TYPENAME_TYPE, SCOPE_REF or USING_DECL) we simply pass tf_qualifying_scope to the relevant tsubst / tsubst_copy call. This flag is intended to apply only to top-level TYPENAME_TYPEs so we must be careful to clear the flag to avoid propagating it during substitution of sub-trees. PR c++/107773 gcc/cp/ChangeLog: * cp-tree.h (enum tsubst_flags): New flag tf_qualifying_scope. * decl.cc (make_typename_type): Use lookup_member instead of lookup_field. If tf_qualifying_scope is set, pass want_type=true instead of =false to lookup_member. Generalize format specifier in diagnostic to handle both type and non-type bindings. * pt.cc (tsubst_aggr_type_1): Clear tf_qualifying_scope. Tidy the function. (tsubst_decl) <case USING_DECL>: Set tf_qualifying_scope when substituting USING_DECL_SCOPE. (tsubst): Clear tf_qualifying_scope right away and remember if it was set. Do the same for tf_tst_ok sooner. <case TYPENAME_TYPE>: Set tf_qualifying_scope when substituting TYPE_CONTEXT. Pass tf_qualifying_scope to make_typename_type if it was set. (tsubst_qualified_id): Set tf_qualifying_scope when substituting the scope. (tsubst_copy): Clear tf_qualifying_scope and remember if it was set. <case SCOPE_REF>: Set tf_qualifying_scope when substituting the scope. <case *_TYPE>: Pass tf_qualifying_scope to tsubst if it was set. * search.cc (lookup_member): Document default argument. gcc/testsuite/ChangeLog: * g++.dg/template/typename24.C: New test. * g++.dg/template/typename25.C: New test. * g++.dg/template/typename25a.C: New test. * g++.dg/template/typename26.C: New test. --- gcc/cp/cp-tree.h | 3 ++ gcc/cp/decl.cc | 9 ++-- gcc/cp/pt.cc | 58 ++++++++++++--------- gcc/cp/search.cc | 2 +- gcc/testsuite/g++.dg/template/typename24.C | 18 +++++++ gcc/testsuite/g++.dg/template/typename25.C | 33 ++++++++++++ gcc/testsuite/g++.dg/template/typename25a.C | 37 +++++++++++++ gcc/testsuite/g++.dg/template/typename26.C | 20 +++++++ 8 files changed, 150 insertions(+), 30 deletions(-) create mode 100644 gcc/testsuite/g++.dg/template/typename24.C create mode 100644 gcc/testsuite/g++.dg/template/typename25.C create mode 100644 gcc/testsuite/g++.dg/template/typename25a.C create mode 100644 gcc/testsuite/g++.dg/template/typename26.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index be8775ed0f8..891fcf521a8 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -5597,6 +5597,9 @@ enum tsubst_flags { tf_tst_ok = 1 << 12, /* Allow a typename-specifier to name a template (C++17 or later). */ tf_dguide = 1 << 13, /* Building a deduction guide from a ctor. */ + tf_qualifying_scope = 1 << 14, /* Substituting the LHS of the :: operator. + Controls TYPENAME_TYPE resolution from + make_typename_type. */ /* Convenient substitution flags combinations. */ tf_warning_or_error = tf_warning | tf_error }; diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index d606b31d7a7..2f6412d04e6 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -4305,9 +4305,10 @@ make_typename_type (tree context, tree name, enum tag_types tag_type, member of the current instantiation or a non-dependent base; lookup will stop when we hit a dependent base. */ if (!dependent_scope_p (context)) - /* We should only set WANT_TYPE when we're a nested typename type. - Then we can give better diagnostics if we find a non-type. */ - t = lookup_field (context, name, 2, /*want_type=*/true); + { + bool want_type = (complain & tf_qualifying_scope); + t = lookup_member (context, name, /*protect=*/2, want_type, complain); + } else t = NULL_TREE; @@ -4359,7 +4360,7 @@ make_typename_type (tree context, tree name, enum tag_types tag_type, else { if (complain & tf_error) - error ("%<typename %T::%D%> names %q#T, which is not a type", + error ("%<typename %T::%D%> names %q#D, which is not a type", context, name, t); return error_mark_node; } diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index e89dbf47097..d11d540ab44 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -13919,8 +13919,7 @@ tsubst_aggr_type_1 (tree t, { if (TYPE_TEMPLATE_INFO (t) && uses_template_parms (t)) { - tree argvec; - tree r; + complain &= ~tf_qualifying_scope; /* Figure out what arguments are appropriate for the type we are trying to find. For example, given: @@ -13931,18 +13930,14 @@ tsubst_aggr_type_1 (tree t, and supposing that we are instantiating f<int, double>, then our ARGS will be {int, double}, but, when looking up S we only want {double}. */ - argvec = tsubst_template_args (TYPE_TI_ARGS (t), args, - complain, in_decl); + tree argvec = tsubst_template_args (TYPE_TI_ARGS (t), args, + complain, in_decl); if (argvec == error_mark_node) - r = error_mark_node; - else - { - r = lookup_template_class (t, argvec, in_decl, NULL_TREE, - entering_scope, complain); - r = cp_build_qualified_type (r, cp_type_quals (t), complain); - } + return error_mark_node; - return r; + tree r = lookup_template_class (t, argvec, in_decl, NULL_TREE, + entering_scope, complain); + return cp_build_qualified_type (r, cp_type_quals (t), complain); } else /* This is not a template type, so there's nothing to do. */ @@ -15003,11 +14998,15 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain) tree scope = USING_DECL_SCOPE (t); if (PACK_EXPANSION_P (scope)) { - scope = tsubst_pack_expansion (scope, args, complain, in_decl); + scope = tsubst_pack_expansion (scope, args, + complain | tf_qualifying_scope, + in_decl); variadic_p = true; } else - scope = tsubst_copy (scope, args, complain, in_decl); + scope = tsubst_copy (scope, args, + complain | tf_qualifying_scope, + in_decl); tree name = DECL_NAME (t); if (IDENTIFIER_CONV_OP_P (name) @@ -15821,6 +15820,12 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) || TREE_CODE (t) == TRANSLATION_UNIT_DECL) return t; + tsubst_flags_t tst_ok_flag = (complain & tf_tst_ok); + complain &= ~tf_tst_ok; + + tsubst_flags_t qualifying_scope_flag = (complain & tf_qualifying_scope); + complain &= ~tf_qualifying_scope; + if (DECL_P (t)) return tsubst_decl (t, args, complain); @@ -15889,9 +15894,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 @@ -16428,7 +16430,9 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) tree ctx = TYPE_CONTEXT (t); if (TREE_CODE (ctx) == TYPE_PACK_EXPANSION) { - ctx = tsubst_pack_expansion (ctx, args, complain, in_decl); + ctx = tsubst_pack_expansion (ctx, args, + complain | tf_qualifying_scope, + in_decl); if (ctx == error_mark_node || TREE_VEC_LENGTH (ctx) > 1) return error_mark_node; @@ -16442,8 +16446,9 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) ctx = TREE_VEC_ELT (ctx, 0); } else - ctx = tsubst_aggr_type (ctx, args, complain, in_decl, - /*entering_scope=*/1); + ctx = tsubst_aggr_type (ctx, args, + complain | tf_qualifying_scope, + in_decl, /*entering_scope=*/1); if (ctx == error_mark_node) return error_mark_node; @@ -16473,8 +16478,7 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) } tsubst_flags_t tcomplain = complain | tf_keep_type_decl; - if (tst_ok) - tcomplain |= tf_tst_ok; + tcomplain |= tst_ok_flag | qualifying_scope_flag; f = make_typename_type (ctx, f, typename_type, tcomplain); if (f == error_mark_node) return f; @@ -16879,7 +16883,7 @@ tsubst_qualified_id (tree qualified_id, tree args, scope = TREE_OPERAND (qualified_id, 0); if (args) { - scope = tsubst (scope, args, complain, in_decl); + scope = tsubst (scope, args, complain | tf_qualifying_scope, in_decl); expr = tsubst_copy (name, args, complain, in_decl); } else @@ -17125,6 +17129,9 @@ 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 (tree d = maybe_dependent_member_ref (t, args, complain, in_decl)) return d; @@ -17598,7 +17605,8 @@ 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, in_decl); + tree op0 = tsubst_copy (TREE_OPERAND (t, 0), args, + complain | tf_qualifying_scope, 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)); @@ -17713,7 +17721,7 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl) case TYPEOF_TYPE: case DECLTYPE_TYPE: case TYPE_DECL: - return tsubst (t, args, complain, in_decl); + return tsubst (t, args, complain | qualifying_scope_flag, in_decl); case USING_DECL: t = DECL_NAME (t); diff --git a/gcc/cp/search.cc b/gcc/cp/search.cc index f3f19cafec6..e472a97679d 100644 --- a/gcc/cp/search.cc +++ b/gcc/cp/search.cc @@ -1109,7 +1109,7 @@ build_baselink (tree binfo, tree access_binfo, tree functions, tree optype) tree lookup_member (tree xbasetype, tree name, int protect, bool want_type, - tsubst_flags_t complain, access_failure_info *afi) + tsubst_flags_t complain, access_failure_info *afi /* = NULL */) { tree rval, rval_binfo = NULL_TREE; tree type = NULL_TREE, basetype_path = NULL_TREE; diff --git a/gcc/testsuite/g++.dg/template/typename24.C b/gcc/testsuite/g++.dg/template/typename24.C new file mode 100644 index 00000000000..8b2b3718442 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/typename24.C @@ -0,0 +1,18 @@ +// PR c++/107773 +// Verify lookup for a non-neste TYPENAME_TYPE correctly considers +// non-types. + +struct a { + typedef void get; +}; + +struct b : a { + int get(int i) const; +}; + +template<class T> +void f() { + typedef typename T::get type; // { dg-error "'int b::get\\(int\\) const', which is not a type" } +} + +template void f<b>(); diff --git a/gcc/testsuite/g++.dg/template/typename25.C b/gcc/testsuite/g++.dg/template/typename25.C new file mode 100644 index 00000000000..04e48e11724 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/typename25.C @@ -0,0 +1,33 @@ +// PR c++/107773 +// Verify lookup for TYPENAME_TYPE appearing to the left of the :: +// scope resolution operator correctly ignores non-types. + +struct a { + typedef void type; +}; + +struct c { + struct b : a { + typedef b self; + static int m; + }; + int b; +}; + +template<class T> +void f() { + // A TYPENAME_TYPE whose TYPE_CONTEXT is a nested TYPENAME_TYPE. + typedef typename T::b::type type; + // A SCOPE_REF whose first operand is a TYPENAME_TYPE. + int m = T::b::m; +} + +template void f<c>(); + +template<class T> +struct d : T::b::self { + // A USING_DECL whose USING_DECL_SCOPE is a TYPENAME_TYPE. + using typename T::b::type; +}; + +template struct d<c>; diff --git a/gcc/testsuite/g++.dg/template/typename25a.C b/gcc/testsuite/g++.dg/template/typename25a.C new file mode 100644 index 00000000000..ecb34aada34 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/typename25a.C @@ -0,0 +1,37 @@ +// PR c++/107773 +// A variadic version of typename25.C +// { dg-do compile { target c++11 } } + +struct a { + typedef void type; +}; + +struct c { + struct b : a { + typedef b self; + static int m; + }; + int b; +}; + +template<class...> void sink(...); + +template<class... Ts> +void f() { + // A TYPENAME_TYPE whose TYPE_CONTEXT is a nested TYPENAME_TYPE. + sink<typename Ts::b::type...>(); + // A SCOPE_REF whose first operand is a TYPENAME_TYPE. + sink(Ts::b::m...); +} + +template void f<c>(); + +template<class... Ts> +struct d : Ts::b::self... { +#if __cpp_variadic_using + // A USING_DECL whose USING_DECL_SCOPE is a TYPENAME_TYPE. + using typename Ts::b::type...; +#endif +}; + +template struct d<c>; diff --git a/gcc/testsuite/g++.dg/template/typename26.C b/gcc/testsuite/g++.dg/template/typename26.C new file mode 100644 index 00000000000..4e6b764a97b --- /dev/null +++ b/gcc/testsuite/g++.dg/template/typename26.C @@ -0,0 +1,20 @@ +// Example 4 from [temp.res.general]/3. + +struct A { + struct X { }; + int X; +}; +struct B { + struct X { }; +}; +template<class T> void f(T t) { + typename T::X x; // { dg-error "'int A::X', which is not a type" } +} +void foo() { + A a; + B b; + f(b); // OK, T::X refers to B::X + // { dg-bogus "" "" { target *-*-* } .-1 } + f(a); // error: T::X refers to the data member A::X not the struct A::X + // { dg-message "required from here" "" { target *-*-* } .-1 } +} -- 2.39.2.422.gc867e4fa18 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution 2023-02-15 17:21 ` Patrick Palka @ 2023-02-15 19:26 ` Jason Merrill 2023-02-15 20:11 ` Patrick Palka 0 siblings, 1 reply; 11+ messages in thread From: Jason Merrill @ 2023-02-15 19:26 UTC (permalink / raw) To: Patrick Palka; +Cc: gcc-patches 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. > -- >8 -- > > Subject: [PATCH] c++: TYPENAME_TYPE lookup ignoring non-types [PR107773] > > Currently when resolving a TYPENAME_TYPE for 'typename T::m' via > make_typename_type, we consider only type bindings of 'm' and ignore > non-type ones. But [temp.res.general]/3 says, in a note, "the usual > qualified name lookup ([basic.lookup.qual]) applies even in the presence > of 'typename'", and qualified name lookup doesn't discriminate between > type and non-type bindings. So when resolving such a TYPENAME_TYPE > we want the lookup to consider all bindings. > > An exception is when we have a TYPENAME_TYPE corresponding to the > qualifying scope of the :: scope resolution operator, such as > 'T::type' in 'typename T::type::m'. In that case, [basic.lookup.qual]/1 > applies, and lookup for such a TYPENAME_TYPE must ignore non-type bindings. > So in order to correctly handle all cases, make_typename_type needs an > additional flag controlling whether lookup should ignore non-types or not. > > To that end this patch adds a new tsubst flag tf_qualifying_scope to > communicate to make_typename_type whether we want to ignore non-type > bindings during the lookup (by default we don't want to ignore them). > In contexts where we do want to ignore non-types (when substituting > into the scope of TYPENAME_TYPE, SCOPE_REF or USING_DECL) we simply > pass tf_qualifying_scope to the relevant tsubst / tsubst_copy call. > This flag is intended to apply only to top-level TYPENAME_TYPEs so > we must be careful to clear the flag to avoid propagating it during > substitution of sub-trees. > > PR c++/107773 > > gcc/cp/ChangeLog: > > * cp-tree.h (enum tsubst_flags): New flag tf_qualifying_scope. > * decl.cc (make_typename_type): Use lookup_member instead of > lookup_field. If tf_qualifying_scope is set, pass want_type=true > instead of =false to lookup_member. Generalize format specifier > in diagnostic to handle both type and non-type bindings. > * pt.cc (tsubst_aggr_type_1): Clear tf_qualifying_scope. Tidy > the function. > (tsubst_decl) <case USING_DECL>: Set tf_qualifying_scope when > substituting USING_DECL_SCOPE. > (tsubst): Clear tf_qualifying_scope right away and remember if > it was set. Do the same for tf_tst_ok sooner. > <case TYPENAME_TYPE>: Set tf_qualifying_scope when substituting > TYPE_CONTEXT. Pass tf_qualifying_scope to make_typename_type > if it was set. > (tsubst_qualified_id): Set tf_qualifying_scope when substituting > the scope. > (tsubst_copy): Clear tf_qualifying_scope and remember if it was > set. > <case SCOPE_REF>: Set tf_qualifying_scope when substituting the > scope. > <case *_TYPE>: Pass tf_qualifying_scope to tsubst if it was set. > * search.cc (lookup_member): Document default argument. > > gcc/testsuite/ChangeLog: > > * g++.dg/template/typename24.C: New test. > * g++.dg/template/typename25.C: New test. > * g++.dg/template/typename25a.C: New test. > * g++.dg/template/typename26.C: New test. > --- > gcc/cp/cp-tree.h | 3 ++ > gcc/cp/decl.cc | 9 ++-- > gcc/cp/pt.cc | 58 ++++++++++++--------- > gcc/cp/search.cc | 2 +- > gcc/testsuite/g++.dg/template/typename24.C | 18 +++++++ > gcc/testsuite/g++.dg/template/typename25.C | 33 ++++++++++++ > gcc/testsuite/g++.dg/template/typename25a.C | 37 +++++++++++++ > gcc/testsuite/g++.dg/template/typename26.C | 20 +++++++ > 8 files changed, 150 insertions(+), 30 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/template/typename24.C > create mode 100644 gcc/testsuite/g++.dg/template/typename25.C > create mode 100644 gcc/testsuite/g++.dg/template/typename25a.C > create mode 100644 gcc/testsuite/g++.dg/template/typename26.C > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index be8775ed0f8..891fcf521a8 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -5597,6 +5597,9 @@ enum tsubst_flags { > tf_tst_ok = 1 << 12, /* Allow a typename-specifier to name > a template (C++17 or later). */ > tf_dguide = 1 << 13, /* Building a deduction guide from a ctor. */ > + tf_qualifying_scope = 1 << 14, /* Substituting the LHS of the :: operator. > + Controls TYPENAME_TYPE resolution from > + make_typename_type. */ > /* Convenient substitution flags combinations. */ > tf_warning_or_error = tf_warning | tf_error > }; > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > index d606b31d7a7..2f6412d04e6 100644 > --- a/gcc/cp/decl.cc > +++ b/gcc/cp/decl.cc > @@ -4305,9 +4305,10 @@ make_typename_type (tree context, tree name, enum tag_types tag_type, > member of the current instantiation or a non-dependent base; > lookup will stop when we hit a dependent base. */ > if (!dependent_scope_p (context)) > - /* We should only set WANT_TYPE when we're a nested typename type. > - Then we can give better diagnostics if we find a non-type. */ > - t = lookup_field (context, name, 2, /*want_type=*/true); > + { > + bool want_type = (complain & tf_qualifying_scope); > + t = lookup_member (context, name, /*protect=*/2, want_type, complain); > + } > else > t = NULL_TREE; > > @@ -4359,7 +4360,7 @@ make_typename_type (tree context, tree name, enum tag_types tag_type, > else > { > if (complain & tf_error) > - error ("%<typename %T::%D%> names %q#T, which is not a type", > + error ("%<typename %T::%D%> names %q#D, which is not a type", > context, name, t); > return error_mark_node; > } > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index e89dbf47097..d11d540ab44 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -13919,8 +13919,7 @@ tsubst_aggr_type_1 (tree t, > { > if (TYPE_TEMPLATE_INFO (t) && uses_template_parms (t)) > { > - tree argvec; > - tree r; > + complain &= ~tf_qualifying_scope; > > /* Figure out what arguments are appropriate for the > type we are trying to find. For example, given: > @@ -13931,18 +13930,14 @@ tsubst_aggr_type_1 (tree t, > and supposing that we are instantiating f<int, double>, > then our ARGS will be {int, double}, but, when looking up > S we only want {double}. */ > - argvec = tsubst_template_args (TYPE_TI_ARGS (t), args, > - complain, in_decl); > + tree argvec = tsubst_template_args (TYPE_TI_ARGS (t), args, > + complain, in_decl); > if (argvec == error_mark_node) > - r = error_mark_node; > - else > - { > - r = lookup_template_class (t, argvec, in_decl, NULL_TREE, > - entering_scope, complain); > - r = cp_build_qualified_type (r, cp_type_quals (t), complain); > - } > + return error_mark_node; > > - return r; > + tree r = lookup_template_class (t, argvec, in_decl, NULL_TREE, > + entering_scope, complain); > + return cp_build_qualified_type (r, cp_type_quals (t), complain); > } > else > /* This is not a template type, so there's nothing to do. */ > @@ -15003,11 +14998,15 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain) > tree scope = USING_DECL_SCOPE (t); > if (PACK_EXPANSION_P (scope)) > { > - scope = tsubst_pack_expansion (scope, args, complain, in_decl); > + scope = tsubst_pack_expansion (scope, args, > + complain | tf_qualifying_scope, > + in_decl); > variadic_p = true; > } > else > - scope = tsubst_copy (scope, args, complain, in_decl); > + scope = tsubst_copy (scope, args, > + complain | tf_qualifying_scope, > + in_decl); > > tree name = DECL_NAME (t); > if (IDENTIFIER_CONV_OP_P (name) > @@ -15821,6 +15820,12 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) > || TREE_CODE (t) == TRANSLATION_UNIT_DECL) > return t; > > + tsubst_flags_t tst_ok_flag = (complain & tf_tst_ok); > + complain &= ~tf_tst_ok; > + > + tsubst_flags_t qualifying_scope_flag = (complain & tf_qualifying_scope); > + complain &= ~tf_qualifying_scope; > + > if (DECL_P (t)) > return tsubst_decl (t, args, complain); > > @@ -15889,9 +15894,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 > @@ -16428,7 +16430,9 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) > tree ctx = TYPE_CONTEXT (t); > if (TREE_CODE (ctx) == TYPE_PACK_EXPANSION) > { > - ctx = tsubst_pack_expansion (ctx, args, complain, in_decl); > + ctx = tsubst_pack_expansion (ctx, args, > + complain | tf_qualifying_scope, > + in_decl); > if (ctx == error_mark_node > || TREE_VEC_LENGTH (ctx) > 1) > return error_mark_node; > @@ -16442,8 +16446,9 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) > ctx = TREE_VEC_ELT (ctx, 0); > } > else > - ctx = tsubst_aggr_type (ctx, args, complain, in_decl, > - /*entering_scope=*/1); > + ctx = tsubst_aggr_type (ctx, args, > + complain | tf_qualifying_scope, > + in_decl, /*entering_scope=*/1); > if (ctx == error_mark_node) > return error_mark_node; > > @@ -16473,8 +16478,7 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) > } > > tsubst_flags_t tcomplain = complain | tf_keep_type_decl; > - if (tst_ok) > - tcomplain |= tf_tst_ok; > + tcomplain |= tst_ok_flag | qualifying_scope_flag; > f = make_typename_type (ctx, f, typename_type, tcomplain); > if (f == error_mark_node) > return f; > @@ -16879,7 +16883,7 @@ tsubst_qualified_id (tree qualified_id, tree args, > scope = TREE_OPERAND (qualified_id, 0); > if (args) > { > - scope = tsubst (scope, args, complain, in_decl); > + scope = tsubst (scope, args, complain | tf_qualifying_scope, in_decl); > expr = tsubst_copy (name, args, complain, in_decl); > } > else > @@ -17125,6 +17129,9 @@ 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 (tree d = maybe_dependent_member_ref (t, args, complain, in_decl)) > return d; > > @@ -17598,7 +17605,8 @@ 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, in_decl); > + tree op0 = tsubst_copy (TREE_OPERAND (t, 0), args, > + complain | tf_qualifying_scope, 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)); > @@ -17713,7 +17721,7 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl) > case TYPEOF_TYPE: > case DECLTYPE_TYPE: > case TYPE_DECL: > - return tsubst (t, args, complain, in_decl); > + return tsubst (t, args, complain | qualifying_scope_flag, in_decl); > > case USING_DECL: > t = DECL_NAME (t); > diff --git a/gcc/cp/search.cc b/gcc/cp/search.cc > index f3f19cafec6..e472a97679d 100644 > --- a/gcc/cp/search.cc > +++ b/gcc/cp/search.cc > @@ -1109,7 +1109,7 @@ build_baselink (tree binfo, tree access_binfo, tree functions, tree optype) > > tree > lookup_member (tree xbasetype, tree name, int protect, bool want_type, > - tsubst_flags_t complain, access_failure_info *afi) > + tsubst_flags_t complain, access_failure_info *afi /* = NULL */) > { > tree rval, rval_binfo = NULL_TREE; > tree type = NULL_TREE, basetype_path = NULL_TREE; > diff --git a/gcc/testsuite/g++.dg/template/typename24.C b/gcc/testsuite/g++.dg/template/typename24.C > new file mode 100644 > index 00000000000..8b2b3718442 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/typename24.C > @@ -0,0 +1,18 @@ > +// PR c++/107773 > +// Verify lookup for a non-neste TYPENAME_TYPE correctly considers > +// non-types. > + > +struct a { > + typedef void get; > +}; > + > +struct b : a { > + int get(int i) const; > +}; > + > +template<class T> > +void f() { > + typedef typename T::get type; // { dg-error "'int b::get\\(int\\) const', which is not a type" } > +} > + > +template void f<b>(); > diff --git a/gcc/testsuite/g++.dg/template/typename25.C b/gcc/testsuite/g++.dg/template/typename25.C > new file mode 100644 > index 00000000000..04e48e11724 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/typename25.C > @@ -0,0 +1,33 @@ > +// PR c++/107773 > +// Verify lookup for TYPENAME_TYPE appearing to the left of the :: > +// scope resolution operator correctly ignores non-types. > + > +struct a { > + typedef void type; > +}; > + > +struct c { > + struct b : a { > + typedef b self; > + static int m; > + }; > + int b; > +}; > + > +template<class T> > +void f() { > + // A TYPENAME_TYPE whose TYPE_CONTEXT is a nested TYPENAME_TYPE. > + typedef typename T::b::type type; > + // A SCOPE_REF whose first operand is a TYPENAME_TYPE. > + int m = T::b::m; > +} > + > +template void f<c>(); > + > +template<class T> > +struct d : T::b::self { > + // A USING_DECL whose USING_DECL_SCOPE is a TYPENAME_TYPE. > + using typename T::b::type; > +}; > + > +template struct d<c>; > diff --git a/gcc/testsuite/g++.dg/template/typename25a.C b/gcc/testsuite/g++.dg/template/typename25a.C > new file mode 100644 > index 00000000000..ecb34aada34 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/typename25a.C > @@ -0,0 +1,37 @@ > +// PR c++/107773 > +// A variadic version of typename25.C > +// { dg-do compile { target c++11 } } > + > +struct a { > + typedef void type; > +}; > + > +struct c { > + struct b : a { > + typedef b self; > + static int m; > + }; > + int b; > +}; > + > +template<class...> void sink(...); > + > +template<class... Ts> > +void f() { > + // A TYPENAME_TYPE whose TYPE_CONTEXT is a nested TYPENAME_TYPE. > + sink<typename Ts::b::type...>(); > + // A SCOPE_REF whose first operand is a TYPENAME_TYPE. > + sink(Ts::b::m...); > +} > + > +template void f<c>(); > + > +template<class... Ts> > +struct d : Ts::b::self... { > +#if __cpp_variadic_using > + // A USING_DECL whose USING_DECL_SCOPE is a TYPENAME_TYPE. > + using typename Ts::b::type...; > +#endif > +}; > + > +template struct d<c>; > diff --git a/gcc/testsuite/g++.dg/template/typename26.C b/gcc/testsuite/g++.dg/template/typename26.C > new file mode 100644 > index 00000000000..4e6b764a97b > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/typename26.C > @@ -0,0 +1,20 @@ > +// Example 4 from [temp.res.general]/3. > + > +struct A { > + struct X { }; > + int X; > +}; > +struct B { > + struct X { }; > +}; > +template<class T> void f(T t) { > + typename T::X x; // { dg-error "'int A::X', which is not a type" } > +} > +void foo() { > + A a; > + B b; > + f(b); // OK, T::X refers to B::X > + // { dg-bogus "" "" { target *-*-* } .-1 } > + f(a); // error: T::X refers to the data member A::X not the struct A::X > + // { dg-message "required from here" "" { target *-*-* } .-1 } > +} ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution 2023-02-15 19:26 ` Jason Merrill @ 2023-02-15 20:11 ` Patrick Palka 2023-02-20 3:49 ` Jason Merrill 0 siblings, 1 reply; 11+ messages in thread From: Patrick Palka @ 2023-02-15 20:11 UTC (permalink / raw) To: Jason Merrill; +Cc: Patrick Palka, gcc-patches 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? > > > -- >8 -- > > > > Subject: [PATCH] c++: TYPENAME_TYPE lookup ignoring non-types [PR107773] > > > > Currently when resolving a TYPENAME_TYPE for 'typename T::m' via > > make_typename_type, we consider only type bindings of 'm' and ignore > > non-type ones. But [temp.res.general]/3 says, in a note, "the usual > > qualified name lookup ([basic.lookup.qual]) applies even in the presence > > of 'typename'", and qualified name lookup doesn't discriminate between > > type and non-type bindings. So when resolving such a TYPENAME_TYPE > > we want the lookup to consider all bindings. > > > > An exception is when we have a TYPENAME_TYPE corresponding to the > > qualifying scope of the :: scope resolution operator, such as > > 'T::type' in 'typename T::type::m'. In that case, [basic.lookup.qual]/1 > > applies, and lookup for such a TYPENAME_TYPE must ignore non-type bindings. > > So in order to correctly handle all cases, make_typename_type needs an > > additional flag controlling whether lookup should ignore non-types or not. > > > > To that end this patch adds a new tsubst flag tf_qualifying_scope to > > communicate to make_typename_type whether we want to ignore non-type > > bindings during the lookup (by default we don't want to ignore them). > > In contexts where we do want to ignore non-types (when substituting > > into the scope of TYPENAME_TYPE, SCOPE_REF or USING_DECL) we simply > > pass tf_qualifying_scope to the relevant tsubst / tsubst_copy call. > > This flag is intended to apply only to top-level TYPENAME_TYPEs so > > we must be careful to clear the flag to avoid propagating it during > > substitution of sub-trees. > > > > PR c++/107773 > > > > gcc/cp/ChangeLog: > > > > * cp-tree.h (enum tsubst_flags): New flag tf_qualifying_scope. > > * decl.cc (make_typename_type): Use lookup_member instead of > > lookup_field. If tf_qualifying_scope is set, pass want_type=true > > instead of =false to lookup_member. Generalize format specifier > > in diagnostic to handle both type and non-type bindings. > > * pt.cc (tsubst_aggr_type_1): Clear tf_qualifying_scope. Tidy > > the function. > > (tsubst_decl) <case USING_DECL>: Set tf_qualifying_scope when > > substituting USING_DECL_SCOPE. > > (tsubst): Clear tf_qualifying_scope right away and remember if > > it was set. Do the same for tf_tst_ok sooner. > > <case TYPENAME_TYPE>: Set tf_qualifying_scope when substituting > > TYPE_CONTEXT. Pass tf_qualifying_scope to make_typename_type > > if it was set. > > (tsubst_qualified_id): Set tf_qualifying_scope when substituting > > the scope. > > (tsubst_copy): Clear tf_qualifying_scope and remember if it was > > set. > > <case SCOPE_REF>: Set tf_qualifying_scope when substituting the > > scope. > > <case *_TYPE>: Pass tf_qualifying_scope to tsubst if it was set. > > * search.cc (lookup_member): Document default argument. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/template/typename24.C: New test. > > * g++.dg/template/typename25.C: New test. > > * g++.dg/template/typename25a.C: New test. > > * g++.dg/template/typename26.C: New test. > > --- > > gcc/cp/cp-tree.h | 3 ++ > > gcc/cp/decl.cc | 9 ++-- > > gcc/cp/pt.cc | 58 ++++++++++++--------- > > gcc/cp/search.cc | 2 +- > > gcc/testsuite/g++.dg/template/typename24.C | 18 +++++++ > > gcc/testsuite/g++.dg/template/typename25.C | 33 ++++++++++++ > > gcc/testsuite/g++.dg/template/typename25a.C | 37 +++++++++++++ > > gcc/testsuite/g++.dg/template/typename26.C | 20 +++++++ > > 8 files changed, 150 insertions(+), 30 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/template/typename24.C > > create mode 100644 gcc/testsuite/g++.dg/template/typename25.C > > create mode 100644 gcc/testsuite/g++.dg/template/typename25a.C > > create mode 100644 gcc/testsuite/g++.dg/template/typename26.C > > > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > > index be8775ed0f8..891fcf521a8 100644 > > --- a/gcc/cp/cp-tree.h > > +++ b/gcc/cp/cp-tree.h > > @@ -5597,6 +5597,9 @@ enum tsubst_flags { > > tf_tst_ok = 1 << 12, /* Allow a typename-specifier to name > > a template (C++17 or later). */ > > tf_dguide = 1 << 13, /* Building a deduction guide from a > > ctor. */ > > + tf_qualifying_scope = 1 << 14, /* Substituting the LHS of the :: > > operator. > > + Controls TYPENAME_TYPE resolution from > > + make_typename_type. */ > > /* Convenient substitution flags combinations. */ > > tf_warning_or_error = tf_warning | tf_error > > }; > > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > > index d606b31d7a7..2f6412d04e6 100644 > > --- a/gcc/cp/decl.cc > > +++ b/gcc/cp/decl.cc > > @@ -4305,9 +4305,10 @@ make_typename_type (tree context, tree name, enum > > tag_types tag_type, > > member of the current instantiation or a non-dependent base; > > lookup will stop when we hit a dependent base. */ > > if (!dependent_scope_p (context)) > > - /* We should only set WANT_TYPE when we're a nested typename type. > > - Then we can give better diagnostics if we find a non-type. */ > > - t = lookup_field (context, name, 2, /*want_type=*/true); > > + { > > + bool want_type = (complain & tf_qualifying_scope); > > + t = lookup_member (context, name, /*protect=*/2, want_type, > > complain); > > + } > > else > > t = NULL_TREE; > > @@ -4359,7 +4360,7 @@ make_typename_type (tree context, tree name, enum > > tag_types tag_type, > > else > > { > > if (complain & tf_error) > > - error ("%<typename %T::%D%> names %q#T, which is not a type", > > + error ("%<typename %T::%D%> names %q#D, which is not a type", > > context, name, t); > > return error_mark_node; > > } > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > > index e89dbf47097..d11d540ab44 100644 > > --- a/gcc/cp/pt.cc > > +++ b/gcc/cp/pt.cc > > @@ -13919,8 +13919,7 @@ tsubst_aggr_type_1 (tree t, > > { > > if (TYPE_TEMPLATE_INFO (t) && uses_template_parms (t)) > > { > > - tree argvec; > > - tree r; > > + complain &= ~tf_qualifying_scope; > > /* Figure out what arguments are appropriate for the > > type we are trying to find. For example, given: > > @@ -13931,18 +13930,14 @@ tsubst_aggr_type_1 (tree t, > > and supposing that we are instantiating f<int, double>, > > then our ARGS will be {int, double}, but, when looking up > > S we only want {double}. */ > > - argvec = tsubst_template_args (TYPE_TI_ARGS (t), args, > > - complain, in_decl); > > + tree argvec = tsubst_template_args (TYPE_TI_ARGS (t), args, > > + complain, in_decl); > > if (argvec == error_mark_node) > > - r = error_mark_node; > > - else > > - { > > - r = lookup_template_class (t, argvec, in_decl, NULL_TREE, > > - entering_scope, complain); > > - r = cp_build_qualified_type (r, cp_type_quals (t), complain); > > - } > > + return error_mark_node; > > - return r; > > + tree r = lookup_template_class (t, argvec, in_decl, NULL_TREE, > > + entering_scope, complain); > > + return cp_build_qualified_type (r, cp_type_quals (t), complain); > > } > > else > > /* This is not a template type, so there's nothing to do. */ > > @@ -15003,11 +14998,15 @@ tsubst_decl (tree t, tree args, tsubst_flags_t > > complain) > > tree scope = USING_DECL_SCOPE (t); > > if (PACK_EXPANSION_P (scope)) > > { > > - scope = tsubst_pack_expansion (scope, args, complain, in_decl); > > + scope = tsubst_pack_expansion (scope, args, > > + complain | tf_qualifying_scope, > > + in_decl); > > variadic_p = true; > > } > > else > > - scope = tsubst_copy (scope, args, complain, in_decl); > > + scope = tsubst_copy (scope, args, > > + complain | tf_qualifying_scope, > > + in_decl); > > tree name = DECL_NAME (t); > > if (IDENTIFIER_CONV_OP_P (name) > > @@ -15821,6 +15820,12 @@ tsubst (tree t, tree args, tsubst_flags_t complain, > > tree in_decl) > > || TREE_CODE (t) == TRANSLATION_UNIT_DECL) > > return t; > > + tsubst_flags_t tst_ok_flag = (complain & tf_tst_ok); > > + complain &= ~tf_tst_ok; > > + > > + tsubst_flags_t qualifying_scope_flag = (complain & tf_qualifying_scope); > > + complain &= ~tf_qualifying_scope; > > + > > if (DECL_P (t)) > > return tsubst_decl (t, args, complain); > > @@ -15889,9 +15894,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 > > @@ -16428,7 +16430,9 @@ tsubst (tree t, tree args, tsubst_flags_t complain, > > tree in_decl) > > tree ctx = TYPE_CONTEXT (t); > > if (TREE_CODE (ctx) == TYPE_PACK_EXPANSION) > > { > > - ctx = tsubst_pack_expansion (ctx, args, complain, in_decl); > > + ctx = tsubst_pack_expansion (ctx, args, > > + complain | tf_qualifying_scope, > > + in_decl); > > if (ctx == error_mark_node > > || TREE_VEC_LENGTH (ctx) > 1) > > return error_mark_node; > > @@ -16442,8 +16446,9 @@ tsubst (tree t, tree args, tsubst_flags_t complain, > > tree in_decl) > > ctx = TREE_VEC_ELT (ctx, 0); > > } > > else > > - ctx = tsubst_aggr_type (ctx, args, complain, in_decl, > > - /*entering_scope=*/1); > > + ctx = tsubst_aggr_type (ctx, args, > > + complain | tf_qualifying_scope, > > + in_decl, /*entering_scope=*/1); > > if (ctx == error_mark_node) > > return error_mark_node; > > @@ -16473,8 +16478,7 @@ tsubst (tree t, tree args, tsubst_flags_t > > complain, tree in_decl) > > } > > tsubst_flags_t tcomplain = complain | tf_keep_type_decl; > > - if (tst_ok) > > - tcomplain |= tf_tst_ok; > > + tcomplain |= tst_ok_flag | qualifying_scope_flag; > > f = make_typename_type (ctx, f, typename_type, tcomplain); > > if (f == error_mark_node) > > return f; > > @@ -16879,7 +16883,7 @@ tsubst_qualified_id (tree qualified_id, tree args, > > scope = TREE_OPERAND (qualified_id, 0); > > if (args) > > { > > - scope = tsubst (scope, args, complain, in_decl); > > + scope = tsubst (scope, args, complain | tf_qualifying_scope, > > in_decl); > > expr = tsubst_copy (name, args, complain, in_decl); > > } > > else > > @@ -17125,6 +17129,9 @@ 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 (tree d = maybe_dependent_member_ref (t, args, complain, in_decl)) > > return d; > > @@ -17598,7 +17605,8 @@ 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, in_decl); > > + tree op0 = tsubst_copy (TREE_OPERAND (t, 0), args, > > + complain | tf_qualifying_scope, 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)); > > @@ -17713,7 +17721,7 @@ tsubst_copy (tree t, tree args, tsubst_flags_t > > complain, tree in_decl) > > case TYPEOF_TYPE: > > case DECLTYPE_TYPE: > > case TYPE_DECL: > > - return tsubst (t, args, complain, in_decl); > > + return tsubst (t, args, complain | qualifying_scope_flag, in_decl); > > case USING_DECL: > > t = DECL_NAME (t); > > diff --git a/gcc/cp/search.cc b/gcc/cp/search.cc > > index f3f19cafec6..e472a97679d 100644 > > --- a/gcc/cp/search.cc > > +++ b/gcc/cp/search.cc > > @@ -1109,7 +1109,7 @@ build_baselink (tree binfo, tree access_binfo, tree > > functions, tree optype) > > tree > > lookup_member (tree xbasetype, tree name, int protect, bool want_type, > > - tsubst_flags_t complain, access_failure_info *afi) > > + tsubst_flags_t complain, access_failure_info *afi /* = NULL */) > > { > > tree rval, rval_binfo = NULL_TREE; > > tree type = NULL_TREE, basetype_path = NULL_TREE; > > diff --git a/gcc/testsuite/g++.dg/template/typename24.C > > b/gcc/testsuite/g++.dg/template/typename24.C > > new file mode 100644 > > index 00000000000..8b2b3718442 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/template/typename24.C > > @@ -0,0 +1,18 @@ > > +// PR c++/107773 > > +// Verify lookup for a non-neste TYPENAME_TYPE correctly considers > > +// non-types. > > + > > +struct a { > > + typedef void get; > > +}; > > + > > +struct b : a { > > + int get(int i) const; > > +}; > > + > > +template<class T> > > +void f() { > > + typedef typename T::get type; // { dg-error "'int b::get\\(int\\) const', > > which is not a type" } > > +} > > + > > +template void f<b>(); > > diff --git a/gcc/testsuite/g++.dg/template/typename25.C > > b/gcc/testsuite/g++.dg/template/typename25.C > > new file mode 100644 > > index 00000000000..04e48e11724 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/template/typename25.C > > @@ -0,0 +1,33 @@ > > +// PR c++/107773 > > +// Verify lookup for TYPENAME_TYPE appearing to the left of the :: > > +// scope resolution operator correctly ignores non-types. > > + > > +struct a { > > + typedef void type; > > +}; > > + > > +struct c { > > + struct b : a { > > + typedef b self; > > + static int m; > > + }; > > + int b; > > +}; > > + > > +template<class T> > > +void f() { > > + // A TYPENAME_TYPE whose TYPE_CONTEXT is a nested TYPENAME_TYPE. > > + typedef typename T::b::type type; > > + // A SCOPE_REF whose first operand is a TYPENAME_TYPE. > > + int m = T::b::m; > > +} > > + > > +template void f<c>(); > > + > > +template<class T> > > +struct d : T::b::self { > > + // A USING_DECL whose USING_DECL_SCOPE is a TYPENAME_TYPE. > > + using typename T::b::type; > > +}; > > + > > +template struct d<c>; > > diff --git a/gcc/testsuite/g++.dg/template/typename25a.C > > b/gcc/testsuite/g++.dg/template/typename25a.C > > new file mode 100644 > > index 00000000000..ecb34aada34 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/template/typename25a.C > > @@ -0,0 +1,37 @@ > > +// PR c++/107773 > > +// A variadic version of typename25.C > > +// { dg-do compile { target c++11 } } > > + > > +struct a { > > + typedef void type; > > +}; > > + > > +struct c { > > + struct b : a { > > + typedef b self; > > + static int m; > > + }; > > + int b; > > +}; > > + > > +template<class...> void sink(...); > > + > > +template<class... Ts> > > +void f() { > > + // A TYPENAME_TYPE whose TYPE_CONTEXT is a nested TYPENAME_TYPE. > > + sink<typename Ts::b::type...>(); > > + // A SCOPE_REF whose first operand is a TYPENAME_TYPE. > > + sink(Ts::b::m...); > > +} > > + > > +template void f<c>(); > > + > > +template<class... Ts> > > +struct d : Ts::b::self... { > > +#if __cpp_variadic_using > > + // A USING_DECL whose USING_DECL_SCOPE is a TYPENAME_TYPE. > > + using typename Ts::b::type...; > > +#endif > > +}; > > + > > +template struct d<c>; > > diff --git a/gcc/testsuite/g++.dg/template/typename26.C > > b/gcc/testsuite/g++.dg/template/typename26.C > > new file mode 100644 > > index 00000000000..4e6b764a97b > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/template/typename26.C > > @@ -0,0 +1,20 @@ > > +// Example 4 from [temp.res.general]/3. > > + > > +struct A { > > + struct X { }; > > + int X; > > +}; > > +struct B { > > + struct X { }; > > +}; > > +template<class T> void f(T t) { > > + typename T::X x; // { dg-error "'int A::X', which is not a type" } > > +} > > +void foo() { > > + A a; > > + B b; > > + f(b); // OK, T::X refers to B::X > > + // { dg-bogus "" "" { target *-*-* } .-1 } > > + f(a); // error: T::X refers to the data member A::X not the struct A::X > > + // { dg-message "required from here" "" { target *-*-* } .-1 } > > +} > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution 2023-02-15 20:11 ` Patrick Palka @ 2023-02-20 3:49 ` Jason Merrill 2023-02-22 0:05 ` Patrick Palka 0 siblings, 1 reply; 11+ messages in thread From: Jason Merrill @ 2023-02-20 3:49 UTC (permalink / raw) To: Patrick Palka; +Cc: gcc-patches 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. 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. I'd call tsubst, and leave the tsubst_aggr_type call alone for GCC 13. >>> -- >8 -- >>> >>> Subject: [PATCH] c++: TYPENAME_TYPE lookup ignoring non-types [PR107773] >>> >>> Currently when resolving a TYPENAME_TYPE for 'typename T::m' via >>> make_typename_type, we consider only type bindings of 'm' and ignore >>> non-type ones. But [temp.res.general]/3 says, in a note, "the usual >>> qualified name lookup ([basic.lookup.qual]) applies even in the presence >>> of 'typename'", and qualified name lookup doesn't discriminate between >>> type and non-type bindings. So when resolving such a TYPENAME_TYPE >>> we want the lookup to consider all bindings. >>> >>> An exception is when we have a TYPENAME_TYPE corresponding to the >>> qualifying scope of the :: scope resolution operator, such as >>> 'T::type' in 'typename T::type::m'. In that case, [basic.lookup.qual]/1 >>> applies, and lookup for such a TYPENAME_TYPE must ignore non-type bindings. >>> So in order to correctly handle all cases, make_typename_type needs an >>> additional flag controlling whether lookup should ignore non-types or not. >>> >>> To that end this patch adds a new tsubst flag tf_qualifying_scope to >>> communicate to make_typename_type whether we want to ignore non-type >>> bindings during the lookup (by default we don't want to ignore them). >>> In contexts where we do want to ignore non-types (when substituting >>> into the scope of TYPENAME_TYPE, SCOPE_REF or USING_DECL) we simply >>> pass tf_qualifying_scope to the relevant tsubst / tsubst_copy call. >>> This flag is intended to apply only to top-level TYPENAME_TYPEs so >>> we must be careful to clear the flag to avoid propagating it during >>> substitution of sub-trees. >>> >>> PR c++/107773 >>> >>> gcc/cp/ChangeLog: >>> >>> * cp-tree.h (enum tsubst_flags): New flag tf_qualifying_scope. >>> * decl.cc (make_typename_type): Use lookup_member instead of >>> lookup_field. If tf_qualifying_scope is set, pass want_type=true >>> instead of =false to lookup_member. Generalize format specifier >>> in diagnostic to handle both type and non-type bindings. >>> * pt.cc (tsubst_aggr_type_1): Clear tf_qualifying_scope. Tidy >>> the function. >>> (tsubst_decl) <case USING_DECL>: Set tf_qualifying_scope when >>> substituting USING_DECL_SCOPE. >>> (tsubst): Clear tf_qualifying_scope right away and remember if >>> it was set. Do the same for tf_tst_ok sooner. >>> <case TYPENAME_TYPE>: Set tf_qualifying_scope when substituting >>> TYPE_CONTEXT. Pass tf_qualifying_scope to make_typename_type >>> if it was set. >>> (tsubst_qualified_id): Set tf_qualifying_scope when substituting >>> the scope. >>> (tsubst_copy): Clear tf_qualifying_scope and remember if it was >>> set. >>> <case SCOPE_REF>: Set tf_qualifying_scope when substituting the >>> scope. >>> <case *_TYPE>: Pass tf_qualifying_scope to tsubst if it was set. >>> * search.cc (lookup_member): Document default argument. >>> >>> gcc/testsuite/ChangeLog: >>> >>> * g++.dg/template/typename24.C: New test. >>> * g++.dg/template/typename25.C: New test. >>> * g++.dg/template/typename25a.C: New test. >>> * g++.dg/template/typename26.C: New test. >>> --- >>> gcc/cp/cp-tree.h | 3 ++ >>> gcc/cp/decl.cc | 9 ++-- >>> gcc/cp/pt.cc | 58 ++++++++++++--------- >>> gcc/cp/search.cc | 2 +- >>> gcc/testsuite/g++.dg/template/typename24.C | 18 +++++++ >>> gcc/testsuite/g++.dg/template/typename25.C | 33 ++++++++++++ >>> gcc/testsuite/g++.dg/template/typename25a.C | 37 +++++++++++++ >>> gcc/testsuite/g++.dg/template/typename26.C | 20 +++++++ >>> 8 files changed, 150 insertions(+), 30 deletions(-) >>> create mode 100644 gcc/testsuite/g++.dg/template/typename24.C >>> create mode 100644 gcc/testsuite/g++.dg/template/typename25.C >>> create mode 100644 gcc/testsuite/g++.dg/template/typename25a.C >>> create mode 100644 gcc/testsuite/g++.dg/template/typename26.C >>> >>> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h >>> index be8775ed0f8..891fcf521a8 100644 >>> --- a/gcc/cp/cp-tree.h >>> +++ b/gcc/cp/cp-tree.h >>> @@ -5597,6 +5597,9 @@ enum tsubst_flags { >>> tf_tst_ok = 1 << 12, /* Allow a typename-specifier to name >>> a template (C++17 or later). */ >>> tf_dguide = 1 << 13, /* Building a deduction guide from a >>> ctor. */ >>> + tf_qualifying_scope = 1 << 14, /* Substituting the LHS of the :: >>> operator. >>> + Controls TYPENAME_TYPE resolution from >>> + make_typename_type. */ >>> /* Convenient substitution flags combinations. */ >>> tf_warning_or_error = tf_warning | tf_error >>> }; >>> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc >>> index d606b31d7a7..2f6412d04e6 100644 >>> --- a/gcc/cp/decl.cc >>> +++ b/gcc/cp/decl.cc >>> @@ -4305,9 +4305,10 @@ make_typename_type (tree context, tree name, enum >>> tag_types tag_type, >>> member of the current instantiation or a non-dependent base; >>> lookup will stop when we hit a dependent base. */ >>> if (!dependent_scope_p (context)) >>> - /* We should only set WANT_TYPE when we're a nested typename type. >>> - Then we can give better diagnostics if we find a non-type. */ >>> - t = lookup_field (context, name, 2, /*want_type=*/true); >>> + { >>> + bool want_type = (complain & tf_qualifying_scope); >>> + t = lookup_member (context, name, /*protect=*/2, want_type, >>> complain); >>> + } >>> else >>> t = NULL_TREE; >>> @@ -4359,7 +4360,7 @@ make_typename_type (tree context, tree name, enum >>> tag_types tag_type, >>> else >>> { >>> if (complain & tf_error) >>> - error ("%<typename %T::%D%> names %q#T, which is not a type", >>> + error ("%<typename %T::%D%> names %q#D, which is not a type", >>> context, name, t); >>> return error_mark_node; >>> } >>> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc >>> index e89dbf47097..d11d540ab44 100644 >>> --- a/gcc/cp/pt.cc >>> +++ b/gcc/cp/pt.cc >>> @@ -13919,8 +13919,7 @@ tsubst_aggr_type_1 (tree t, >>> { >>> if (TYPE_TEMPLATE_INFO (t) && uses_template_parms (t)) >>> { >>> - tree argvec; >>> - tree r; >>> + complain &= ~tf_qualifying_scope; >>> /* Figure out what arguments are appropriate for the >>> type we are trying to find. For example, given: >>> @@ -13931,18 +13930,14 @@ tsubst_aggr_type_1 (tree t, >>> and supposing that we are instantiating f<int, double>, >>> then our ARGS will be {int, double}, but, when looking up >>> S we only want {double}. */ >>> - argvec = tsubst_template_args (TYPE_TI_ARGS (t), args, >>> - complain, in_decl); >>> + tree argvec = tsubst_template_args (TYPE_TI_ARGS (t), args, >>> + complain, in_decl); >>> if (argvec == error_mark_node) >>> - r = error_mark_node; >>> - else >>> - { >>> - r = lookup_template_class (t, argvec, in_decl, NULL_TREE, >>> - entering_scope, complain); >>> - r = cp_build_qualified_type (r, cp_type_quals (t), complain); >>> - } >>> + return error_mark_node; >>> - return r; >>> + tree r = lookup_template_class (t, argvec, in_decl, NULL_TREE, >>> + entering_scope, complain); >>> + return cp_build_qualified_type (r, cp_type_quals (t), complain); >>> } >>> else >>> /* This is not a template type, so there's nothing to do. */ >>> @@ -15003,11 +14998,15 @@ tsubst_decl (tree t, tree args, tsubst_flags_t >>> complain) >>> tree scope = USING_DECL_SCOPE (t); >>> if (PACK_EXPANSION_P (scope)) >>> { >>> - scope = tsubst_pack_expansion (scope, args, complain, in_decl); >>> + scope = tsubst_pack_expansion (scope, args, >>> + complain | tf_qualifying_scope, >>> + in_decl); >>> variadic_p = true; >>> } >>> else >>> - scope = tsubst_copy (scope, args, complain, in_decl); >>> + scope = tsubst_copy (scope, args, >>> + complain | tf_qualifying_scope, >>> + in_decl); >>> tree name = DECL_NAME (t); >>> if (IDENTIFIER_CONV_OP_P (name) >>> @@ -15821,6 +15820,12 @@ tsubst (tree t, tree args, tsubst_flags_t complain, >>> tree in_decl) >>> || TREE_CODE (t) == TRANSLATION_UNIT_DECL) >>> return t; >>> + tsubst_flags_t tst_ok_flag = (complain & tf_tst_ok); >>> + complain &= ~tf_tst_ok; >>> + >>> + tsubst_flags_t qualifying_scope_flag = (complain & tf_qualifying_scope); >>> + complain &= ~tf_qualifying_scope; >>> + >>> if (DECL_P (t)) >>> return tsubst_decl (t, args, complain); >>> @@ -15889,9 +15894,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 >>> @@ -16428,7 +16430,9 @@ tsubst (tree t, tree args, tsubst_flags_t complain, >>> tree in_decl) >>> tree ctx = TYPE_CONTEXT (t); >>> if (TREE_CODE (ctx) == TYPE_PACK_EXPANSION) >>> { >>> - ctx = tsubst_pack_expansion (ctx, args, complain, in_decl); >>> + ctx = tsubst_pack_expansion (ctx, args, >>> + complain | tf_qualifying_scope, >>> + in_decl); >>> if (ctx == error_mark_node >>> || TREE_VEC_LENGTH (ctx) > 1) >>> return error_mark_node; >>> @@ -16442,8 +16446,9 @@ tsubst (tree t, tree args, tsubst_flags_t complain, >>> tree in_decl) >>> ctx = TREE_VEC_ELT (ctx, 0); >>> } >>> else >>> - ctx = tsubst_aggr_type (ctx, args, complain, in_decl, >>> - /*entering_scope=*/1); >>> + ctx = tsubst_aggr_type (ctx, args, >>> + complain | tf_qualifying_scope, >>> + in_decl, /*entering_scope=*/1); >>> if (ctx == error_mark_node) >>> return error_mark_node; >>> @@ -16473,8 +16478,7 @@ tsubst (tree t, tree args, tsubst_flags_t >>> complain, tree in_decl) >>> } >>> tsubst_flags_t tcomplain = complain | tf_keep_type_decl; >>> - if (tst_ok) >>> - tcomplain |= tf_tst_ok; >>> + tcomplain |= tst_ok_flag | qualifying_scope_flag; >>> f = make_typename_type (ctx, f, typename_type, tcomplain); >>> if (f == error_mark_node) >>> return f; >>> @@ -16879,7 +16883,7 @@ tsubst_qualified_id (tree qualified_id, tree args, >>> scope = TREE_OPERAND (qualified_id, 0); >>> if (args) >>> { >>> - scope = tsubst (scope, args, complain, in_decl); >>> + scope = tsubst (scope, args, complain | tf_qualifying_scope, >>> in_decl); >>> expr = tsubst_copy (name, args, complain, in_decl); >>> } >>> else >>> @@ -17125,6 +17129,9 @@ 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 (tree d = maybe_dependent_member_ref (t, args, complain, in_decl)) >>> return d; >>> @@ -17598,7 +17605,8 @@ 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, in_decl); >>> + tree op0 = tsubst_copy (TREE_OPERAND (t, 0), args, >>> + complain | tf_qualifying_scope, 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)); >>> @@ -17713,7 +17721,7 @@ tsubst_copy (tree t, tree args, tsubst_flags_t >>> complain, tree in_decl) >>> case TYPEOF_TYPE: >>> case DECLTYPE_TYPE: >>> case TYPE_DECL: >>> - return tsubst (t, args, complain, in_decl); >>> + return tsubst (t, args, complain | qualifying_scope_flag, in_decl); >>> case USING_DECL: >>> t = DECL_NAME (t); >>> diff --git a/gcc/cp/search.cc b/gcc/cp/search.cc >>> index f3f19cafec6..e472a97679d 100644 >>> --- a/gcc/cp/search.cc >>> +++ b/gcc/cp/search.cc >>> @@ -1109,7 +1109,7 @@ build_baselink (tree binfo, tree access_binfo, tree >>> functions, tree optype) >>> tree >>> lookup_member (tree xbasetype, tree name, int protect, bool want_type, >>> - tsubst_flags_t complain, access_failure_info *afi) >>> + tsubst_flags_t complain, access_failure_info *afi /* = NULL */) >>> { >>> tree rval, rval_binfo = NULL_TREE; >>> tree type = NULL_TREE, basetype_path = NULL_TREE; >>> diff --git a/gcc/testsuite/g++.dg/template/typename24.C >>> b/gcc/testsuite/g++.dg/template/typename24.C >>> new file mode 100644 >>> index 00000000000..8b2b3718442 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/template/typename24.C >>> @@ -0,0 +1,18 @@ >>> +// PR c++/107773 >>> +// Verify lookup for a non-neste TYPENAME_TYPE correctly considers >>> +// non-types. >>> + >>> +struct a { >>> + typedef void get; >>> +}; >>> + >>> +struct b : a { >>> + int get(int i) const; >>> +}; >>> + >>> +template<class T> >>> +void f() { >>> + typedef typename T::get type; // { dg-error "'int b::get\\(int\\) const', >>> which is not a type" } >>> +} >>> + >>> +template void f<b>(); >>> diff --git a/gcc/testsuite/g++.dg/template/typename25.C >>> b/gcc/testsuite/g++.dg/template/typename25.C >>> new file mode 100644 >>> index 00000000000..04e48e11724 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/template/typename25.C >>> @@ -0,0 +1,33 @@ >>> +// PR c++/107773 >>> +// Verify lookup for TYPENAME_TYPE appearing to the left of the :: >>> +// scope resolution operator correctly ignores non-types. >>> + >>> +struct a { >>> + typedef void type; >>> +}; >>> + >>> +struct c { >>> + struct b : a { >>> + typedef b self; >>> + static int m; >>> + }; >>> + int b; >>> +}; >>> + >>> +template<class T> >>> +void f() { >>> + // A TYPENAME_TYPE whose TYPE_CONTEXT is a nested TYPENAME_TYPE. >>> + typedef typename T::b::type type; >>> + // A SCOPE_REF whose first operand is a TYPENAME_TYPE. >>> + int m = T::b::m; >>> +} >>> + >>> +template void f<c>(); >>> + >>> +template<class T> >>> +struct d : T::b::self { >>> + // A USING_DECL whose USING_DECL_SCOPE is a TYPENAME_TYPE. >>> + using typename T::b::type; >>> +}; >>> + >>> +template struct d<c>; >>> diff --git a/gcc/testsuite/g++.dg/template/typename25a.C >>> b/gcc/testsuite/g++.dg/template/typename25a.C >>> new file mode 100644 >>> index 00000000000..ecb34aada34 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/template/typename25a.C >>> @@ -0,0 +1,37 @@ >>> +// PR c++/107773 >>> +// A variadic version of typename25.C >>> +// { dg-do compile { target c++11 } } >>> + >>> +struct a { >>> + typedef void type; >>> +}; >>> + >>> +struct c { >>> + struct b : a { >>> + typedef b self; >>> + static int m; >>> + }; >>> + int b; >>> +}; >>> + >>> +template<class...> void sink(...); >>> + >>> +template<class... Ts> >>> +void f() { >>> + // A TYPENAME_TYPE whose TYPE_CONTEXT is a nested TYPENAME_TYPE. >>> + sink<typename Ts::b::type...>(); >>> + // A SCOPE_REF whose first operand is a TYPENAME_TYPE. >>> + sink(Ts::b::m...); >>> +} >>> + >>> +template void f<c>(); >>> + >>> +template<class... Ts> >>> +struct d : Ts::b::self... { >>> +#if __cpp_variadic_using >>> + // A USING_DECL whose USING_DECL_SCOPE is a TYPENAME_TYPE. >>> + using typename Ts::b::type...; >>> +#endif >>> +}; >>> + >>> +template struct d<c>; >>> diff --git a/gcc/testsuite/g++.dg/template/typename26.C >>> b/gcc/testsuite/g++.dg/template/typename26.C >>> new file mode 100644 >>> index 00000000000..4e6b764a97b >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/template/typename26.C >>> @@ -0,0 +1,20 @@ >>> +// Example 4 from [temp.res.general]/3. >>> + >>> +struct A { >>> + struct X { }; >>> + int X; >>> +}; >>> +struct B { >>> + struct X { }; >>> +}; >>> +template<class T> void f(T t) { >>> + typename T::X x; // { dg-error "'int A::X', which is not a type" } >>> +} >>> +void foo() { >>> + A a; >>> + B b; >>> + f(b); // OK, T::X refers to B::X >>> + // { dg-bogus "" "" { target *-*-* } .-1 } >>> + f(a); // error: T::X refers to the data member A::X not the struct A::X >>> + // { dg-message "required from here" "" { target *-*-* } .-1 } >>> +} >> >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution 2023-02-20 3:49 ` Jason Merrill @ 2023-02-22 0:05 ` Patrick Palka 2023-03-01 21:33 ` Jason Merrill 0 siblings, 1 reply; 11+ messages in thread From: Patrick Palka @ 2023-02-22 0:05 UTC (permalink / raw) To: Jason Merrill; +Cc: Patrick Palka, gcc-patches 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<T> instead of the primary template type A<T>, 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. -- >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) <case USING_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. <case SCOPE_REF>: 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); -- 2.39.2.501.gd9d677b2d8 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution 2023-02-22 0:05 ` Patrick Palka @ 2023-03-01 21:33 ` Jason Merrill 0 siblings, 0 replies; 11+ messages in thread From: Jason Merrill @ 2023-03-01 21:33 UTC (permalink / raw) To: Patrick Palka; +Cc: gcc-patches 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<T> instead of the primary template > type A<T>, 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) <case USING_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. > <case SCOPE_REF>: 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); ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-03-01 21:33 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-02-13 17:23 [PATCH 1/2] c++: factor out TYPENAME_TYPE substitution 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 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
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).