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
