* [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 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 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-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).