Re: [PATCH 2/2] c++: remove lookup_template_class's entering_scope flag
On 5/2/24 13:49, Patrick Palka wrote:
On Wed, 1 May 2024, Jason Merrill wrote:
On 5/1/24 13:40, Patrick Palka wrote:
On Wed, 1 May 2024, Jason Merrill wrote:
On 5/1/24 12:41, Patrick Palka wrote:
On Fri, 2 Feb 2024, Patrick Palka wrote:
Bootstrapped and regtested on x86_64-pc-linux, does this look like
an improvement? This is not a bugfix and barely related to the
previous
patch, but the previous patch's new use of entering_scope=true
motivated
me to submit this patch since it seems like a nice simplification.
Ping, now that stage 1 is open.
Thanks for the ping. The earlier message isn't showing up in Thunderbird
for
some reason, though I see it in the gmail web interface...
Ah, weird. No worries, this patch was very much stage 1 material anyway.
@@ -16771,9 +16722,10 @@ 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 | tf_qualifying_scope,
- in_decl, /*entering_scope=*/1);
+ {
+ ctx = tsubst_scope (ctx, args, complain, in_decl);
Why is this one tsubst_scope while the others are all plain tsubst?
Ah, just because the call to tsubst_aggr_type being replace passes
tf_qualifying_scope already, so we might as well use tsubst_scope
as shorthand.
Do we want a tsubst_entering_scope function?
Which is just shorthand for tsubst + adjust_type_for_entering_scope?
That's what I was thinking.
Sure, though I was wondering if we eventually might want to get rid of
the distinction between the primary template type A and the generic
instantiation A, and we could treat this as an incremental step
towards that (then we'd just eventually remove the
adjust_type_for_entering_scope calls and keep the tsubst calls).
I don't think we want that; having the distinction helps to avoid wrongly
looking up members of the primary template in contexts that shouldn't be able
to.
Makes sense. How does the following look?
The name tsubst_entering_scope sounds confusingly similar to
tsubst_scope, but I can't think of a better name for it or for
adjust_type_for_entering_scope :/
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 1c3eef60c06..261d6b9f4c8 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -185,8 +185,6 @@ static int unify_pack_expansion (tree, tree, tree,
static tree copy_template_args (tree);
static tree tsubst_template_parms (tree, tree, tsubst_flags_t);
static void tsubst_each_template_parm_constraints (tree, tree,
tsubst_flags_t);
-static tree tsubst_aggr_type (tree, tree, tsubst_flags_t, tree, int);
-static tree tsubst_aggr_type_1 (tree, tree, tsubst_flags_t, tree, int);
static tree tsubst_arg_types (tree, tree, tree, tsubst_flags_t, tree);
static tree tsubst_function_type (tree, tree, tsubst_flags_t, tree);
static bool check_specialization_scope (void);
@@ -9901,6 +9899,34 @@ maybe_get_template_decl_from_type_decl (tree decl)
? CLASSTYPE_TI_TEMPLATE (TREE_TYPE (decl)) : decl;
}
+/* If TYPE is the generic implicit instantiation A, return the primary
+ template type A (which is suitable for entering into e.g. for name
+ lookup), otherwise return TYPE. */
Maybe "e.g. for defining a member of A"
OK either way.
+
+tree
+adjust_type_for_entering_scope (tree type)
+{
+ if (CLASS_TYPE_P (type)
+ && dependent_type_p (type)
+ && TYPE_TEMPLATE_INFO (type)
+ /* We detect the generic implicit instantiation A by inspecting
+TYPE_CANONICAL, which lookup_template_class sets to the primary
+template type A. */
+ && TYPE_CANONICAL (type) == TREE_TYPE (TYPE_TI_TEMPLATE (type)))
+type = TYPE_CANONICAL (type);
+ return type;
+}
Re: [PATCH 2/2] c++: remove lookup_template_class's entering_scope flag
On Wed, 1 May 2024, Jason Merrill wrote:
> On 5/1/24 13:40, Patrick Palka wrote:
> > On Wed, 1 May 2024, Jason Merrill wrote:
> >
> > > On 5/1/24 12:41, Patrick Palka wrote:
> > > > On Fri, 2 Feb 2024, Patrick Palka wrote:
> > > >
> > > > > Bootstrapped and regtested on x86_64-pc-linux, does this look like
> > > > > an improvement? This is not a bugfix and barely related to the
> > > > > previous
> > > > > patch, but the previous patch's new use of entering_scope=true
> > > > > motivated
> > > > > me to submit this patch since it seems like a nice simplification.
> > > >
> > > > Ping, now that stage 1 is open.
> > >
> > > Thanks for the ping. The earlier message isn't showing up in Thunderbird
> > > for
> > > some reason, though I see it in the gmail web interface...
> >
> > Ah, weird. No worries, this patch was very much stage 1 material anyway.
> >
> > >
> > > > > @@ -16771,9 +16722,10 @@ 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 | tf_qualifying_scope,
> > > > > - in_decl, /*entering_scope=*/1);
> > > > > + {
> > > > > + ctx = tsubst_scope (ctx, args, complain, in_decl);
> > >
> > > Why is this one tsubst_scope while the others are all plain tsubst?
> >
> > Ah, just because the call to tsubst_aggr_type being replace passes
> > tf_qualifying_scope already, so we might as well use tsubst_scope
> > as shorthand.
> >
> > > Do we want a tsubst_entering_scope function?
> >
> > Which is just shorthand for tsubst + adjust_type_for_entering_scope?
>
> That's what I was thinking.
>
> > Sure, though I was wondering if we eventually might want to get rid of
> > the distinction between the primary template type A and the generic
> > instantiation A, and we could treat this as an incremental step
> > towards that (then we'd just eventually remove the
> > adjust_type_for_entering_scope calls and keep the tsubst calls).
>
> I don't think we want that; having the distinction helps to avoid wrongly
> looking up members of the primary template in contexts that shouldn't be able
> to.
Makes sense. How does the following look?
The name tsubst_entering_scope sounds confusingly similar to
tsubst_scope, but I can't think of a better name for it or for
adjust_type_for_entering_scope :/
-- >8 --
Subject: [PATCH] c++: remove lookup_template_class's entering_scope flag
gcc/cp/ChangeLog:
* coroutines.cc (instantiate_coro_traits): Adjust call to
lookup_template_class.
(instantiate_coro_handle_for_promise_type): Likewise.
* cp-tree.h (adjust_type_for_entering_scope): Declare.
(lookup_template_class): Adjust declaration.
* decl.cc (make_typename_type): Adjust call to
lookup_template_class. Likewise.
(get_tuple_size): Likewise.
(get_tuple_element_type): Likewise.
* pt.cc (adjust_type_for_entering_scope): Define.
(tsubst_entering_scope): Define.
(lookup_template_class): Remove entering_scope parameter.
Replace tsubst_aggr_type call with tsubst_entering_scope.
(tsubst_aggr_type): Remove.
(tsubst_aggr_type_1): Inline into tsubst.
(tsubst_function_decl): Replace tsubst_aggr_type call
with tsubst_entering_scope.
(tsubst_template_decl): Likewise.
(tsubst_decl): Likewise.
(tsubst) :
Inlined from tsubst_aggr_type_1.
: Adjust calls to
lookup_template_class.
: Replace tsubst_aggr_type call with
tsubst_scope followed by adjust_type_for_entering_scope.
: Replace tsubst_aggr_type
call with tsubst_entering_scope.
Increment processing_template_decl when substituting the
context.
(tsubst_expr) : Replace tsubst_aggr_type
call with tsubst_entering_scope.
: Likewise.
(instantiate_template): Likewise.
(resolve_typename_type): Adjust lookup_template_class call
and call adjust_type_for_entering_scope afterward.
(listify): Adjust lookup_template_class call.
(alias_ctad_tweaks): Likewise.
* semantics.cc (finish_template_type): Adjust lookup_template_class
call and maybe call adjust_type_for_entering_scope afterward.
---
gcc/cp/coroutines.cc | 4 +-
gcc/cp/cp-tree.h | 3 +-
gcc/cp/decl.cc | 4 +-
gcc/cp/pt.cc | 209 +--
gcc/cp/semantics.cc | 4 +-
5 files changed, 90 insertions(+), 134 deletions(-)
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index b05cb9eb330..97bc211ff67 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -353,7 +353,7 @@ instantiate_coro_traits (tree fndecl, location_t kw)
tree traits_class
= lookup_template_class (coro_traits_templ, targ,
Re: [PATCH 2/2] c++: remove lookup_template_class's entering_scope flag
On 5/1/24 13:40, Patrick Palka wrote:
On Wed, 1 May 2024, Jason Merrill wrote:
On 5/1/24 12:41, Patrick Palka wrote:
On Fri, 2 Feb 2024, Patrick Palka wrote:
Bootstrapped and regtested on x86_64-pc-linux, does this look like
an improvement? This is not a bugfix and barely related to the previous
patch, but the previous patch's new use of entering_scope=true motivated
me to submit this patch since it seems like a nice simplification.
Ping, now that stage 1 is open.
Thanks for the ping. The earlier message isn't showing up in Thunderbird for
some reason, though I see it in the gmail web interface...
Ah, weird. No worries, this patch was very much stage 1 material anyway.
@@ -16771,9 +16722,10 @@ 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 | tf_qualifying_scope,
- in_decl, /*entering_scope=*/1);
+ {
+ ctx = tsubst_scope (ctx, args, complain, in_decl);
Why is this one tsubst_scope while the others are all plain tsubst?
Ah, just because the call to tsubst_aggr_type being replace passes
tf_qualifying_scope already, so we might as well use tsubst_scope
as shorthand.
Do we want a tsubst_entering_scope function?
Which is just shorthand for tsubst + adjust_type_for_entering_scope?
That's what I was thinking.
Sure, though I was wondering if we eventually might want to get rid of
the distinction between the primary template type A and the generic
instantiation A, and we could treat this as an incremental step
towards that (then we'd just eventually remove the
adjust_type_for_entering_scope calls and keep the tsubst calls).
I don't think we want that; having the distinction helps to avoid
wrongly looking up members of the primary template in contexts that
shouldn't be able to.
Jason
Re: [PATCH 2/2] c++: remove lookup_template_class's entering_scope flag
On Wed, 1 May 2024, Jason Merrill wrote:
> On 5/1/24 12:41, Patrick Palka wrote:
> > On Fri, 2 Feb 2024, Patrick Palka wrote:
> >
> > > Bootstrapped and regtested on x86_64-pc-linux, does this look like
> > > an improvement? This is not a bugfix and barely related to the previous
> > > patch, but the previous patch's new use of entering_scope=true motivated
> > > me to submit this patch since it seems like a nice simplification.
> >
> > Ping, now that stage 1 is open.
>
> Thanks for the ping. The earlier message isn't showing up in Thunderbird for
> some reason, though I see it in the gmail web interface...
Ah, weird. No worries, this patch was very much stage 1 material anyway.
>
> > > @@ -16771,9 +16722,10 @@ 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 | tf_qualifying_scope,
> > > - in_decl, /*entering_scope=*/1);
> > > + {
> > > + ctx = tsubst_scope (ctx, args, complain, in_decl);
>
> Why is this one tsubst_scope while the others are all plain tsubst?
Ah, just because the call to tsubst_aggr_type being replace passes
tf_qualifying_scope already, so we might as well use tsubst_scope
as shorthand.
>
> Do we want a tsubst_entering_scope function?
Which is just shorthand for tsubst + adjust_type_for_entering_scope?
Sure, though I was wondering if we eventually might want to get rid of
the distinction between the primary template type A and the generic
instantiation A, and we could treat this as an incremental step
towards that (then we'd just eventually remove the
adjust_type_for_entering_scope calls and keep the tsubst calls).
>
> > > + ctx = adjust_type_for_entering_scope (ctx);
>
>
Re: [PATCH 2/2] c++: remove lookup_template_class's entering_scope flag
On 5/1/24 12:41, Patrick Palka wrote:
On Fri, 2 Feb 2024, Patrick Palka wrote:
Bootstrapped and regtested on x86_64-pc-linux, does this look like
an improvement? This is not a bugfix and barely related to the previous
patch, but the previous patch's new use of entering_scope=true motivated
me to submit this patch since it seems like a nice simplification.
Ping, now that stage 1 is open.
Thanks for the ping. The earlier message isn't showing up in
Thunderbird for some reason, though I see it in the gmail web interface...
@@ -16771,9 +16722,10 @@ 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 | tf_qualifying_scope,
- in_decl, /*entering_scope=*/1);
+ {
+ ctx = tsubst_scope (ctx, args, complain, in_decl);
Why is this one tsubst_scope while the others are all plain tsubst?
Do we want a tsubst_entering_scope function?
+ ctx = adjust_type_for_entering_scope (ctx);
Re: [PATCH 2/2] c++: remove lookup_template_class's entering_scope flag
On Fri, 2 Feb 2024, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux, does this look like
> an improvement? This is not a bugfix and barely related to the previous
> patch, but the previous patch's new use of entering_scope=true motivated
> me to submit this patch since it seems like a nice simplification.
Ping, now that stage 1 is open.
>
> -- >8 --
>
> lookup_template_class's entering_scope flag controls whether to prefer
> returning the primary template type A instead of the corresponding
> implicit instantiation A. When we want to set this flag as part of
> substitution, we need to use tsubst_aggr_type which also takes the flag,
> but this separate entry point to substitution turned out to be subtly
> problematic because it doesn't reuse typedefs like tsubst does, which
> r13-4729-gbe124477b38a71 fixed in a way that respects the flag after the
> fact, by adjusting the entering_scope=false result of
> lookup_template_class as if entering_scope=true was passed.
>
> But if that's possible then it means lookup_template_class's the
> entering_scope flag is not necessary after all -- we can just do the
> after-the-fact adjustment everywhere that we currently pass
> entering_scope=true to it and tsubst_aggr_type.
>
> To that end, this patch replaces this flag with an adjustment function
> adjust_type_for_entering_scope, to be used whereever we currently need
> the entering_scope=true behavior. This means we can also get rid of
> tsubst_aggr_type, since the only reason we needed this entry point
> was to be able to pass entering_scope=true to lookup_template_class.
>
> gcc/cp/ChangeLog:
>
> * coroutines.cc (instantiate_coro_traits): Adjust call to
> lookup_template_class.
> (instantiate_coro_handle_for_promise_type): Likewise.
> * cp-tree.h (adjust_type_for_entering_scope): Declare.
> (lookup_template_class): Adjust declaration.
> * decl.cc (make_typename_type): Adjust call to
> lookup_template_class. Likewise.
> (get_tuple_size): Likewise.
> (get_tuple_element_type): Likewise.
> * pt.cc (adjust_type_for_entering_scope): Define.
> (lookup_template_class): Remove entering_scope parameter.
> Replace tsubst_aggr_type call with tsubst followed by
> adjust_type_for_entering_scope.
> (tsubst_aggr_type): Remove.
> (tsubst_aggr_type_1): Inline into tsubst.
> (tsubst_function_decl): Replace tsubst_aggr_type call
> with tsubst followed by adjust_type_for_entering_scope.
> (tsubst_template_decl): Likewise.
> (tsubst_decl): Likewise.
> (tsubst) :
> Inlined from tsubst_aggr_type_1.
> : Adjust calls to
> lookup_template_class.
> : Replace tsubst_aggr_type call with
> tsubst tsubst_scope followed by adjust_type_for_entering_scope.
> : Replace tsubst_aggr_type
> call with tsubst followed by adjust_type_for_entering_scope.
> Increment processing_template_decl when substituting the
> context.
> (tsubst_baselink): Replace tsubst_aggr_type call with tsubst
> followed by adjust_type_for_entering_scope.
> (tsubst_expr) : Replace tsubst_aggr_type
> call with tsubst followed by adjust_type_for_entering_scope.
> : Likewise.
> (instantiate_template): Likewise.
> (resolve_typename_type): Adjust lookup_template_class call
> and call adjust_type_for_entering_scope afterward.
> (listify): Adjust lookup_template_class call.
> (alias_ctad_tweaks): Likewise.
> * semantics.cc (finish_template_type): Adjust lookup_template_class
> call and maybe call adjust_type_for_entering_scope afterward.
> ---
> gcc/cp/coroutines.cc | 4 +-
> gcc/cp/cp-tree.h | 3 +-
> gcc/cp/decl.cc | 4 +-
> gcc/cp/pt.cc | 212 +--
> gcc/cp/semantics.cc | 4 +-
> 5 files changed, 91 insertions(+), 136 deletions(-)
>
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index 3194c911e8c..8dab173d5cb 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -353,7 +353,7 @@ instantiate_coro_traits (tree fndecl, location_t kw)
>tree traits_class
> = lookup_template_class (coro_traits_templ, targ,
>/*in_decl=*/NULL_TREE, /*context=*/NULL_TREE,
> - /*entering scope=*/false, tf_warning_or_error);
> + tf_warning_or_error);
>
>if (traits_class == error_mark_node)
> {
> @@ -400,7 +400,7 @@ instantiate_coro_handle_for_promise_type (location_t kw,
> tree promise_type)
> = lookup_template_class (coro_handle_identifier, targ,
>/* in_decl=*/NULL_TREE,
>/* context=*/std_node,
> - /* entering scope=*/false, tf_warning_or_error);
> + tf_warning_or_error);
>
>if (handle_type == error_mark_node)
> {
> diff --git
