Re: [PATCH 2/2] c++: remove lookup_template_class's entering_scope flag

[Jason Merrill <jason@redhat.com>]

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<T> and the generic
>>> instantiation A<T>, 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<T>, return the primary
> +   template type A<T> (which is suitable for entering into e.g. for name
> +   lookup), otherwise return TYPE.  */

Maybe "e.g. for defining a member of A<T>"

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<T> by inspecting
> +	 TYPE_CANONICAL, which lookup_template_class sets to the primary
> +	 template type A<T>.  */
> +      && TYPE_CANONICAL (type) == TREE_TYPE (TYPE_TI_TEMPLATE (type)))
> +    type = TYPE_CANONICAL (type);
> +  return type;
> +}