Re: [PATCH] c++: merge tsubst_copy into tsubst_copy_and_build

[Jason Merrill <jason@redhat.com>]

On 10/3/23 08:41, Patrick Palka wrote:
> On Mon, 2 Oct 2023, Patrick Palka wrote:
> 
>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
>> OK for trunk?
>>
>> -- >8 --
>>
>> The relationship between tsubst_copy_and_build and tsubst_copy (two of
>> the main template argument substitution routines for expression trees)
>> is rather hazy.  The former is mostly a superset of the latter, with
>> some differences.
>>
>> The main difference is that they handle many tree codes differently, but
>> much of the tree code handling in tsubst_copy appears to be dead code[1].
>> This is because tsubst_copy only gets directly called in a few places
>> and mostly on id-expressions.  The interesting exceptions are PARM_DECL,
>> VAR_DECL, BIT_NOT_EXPR, SCOPE_REF, TEMPLATE_ID_EXPR and IDENTIFIER_NODE:
>>
>>   * for PARM_DECL and VAR_DECL, tsubst_copy_and_build calls tsubst_copy
>>     followed by doing some extra handling of its own
>>   * for BIT_NOT_EXPR tsubst_copy implicitly handles unresolved destructor
>>     calls (i.e. the first operand is an identifier or a type)
>>   * for SCOPE_REF, TEMPLATE_ID_EXPR and IDENTIFIER_NODE tsubst_copy
>>     refrains from doing name lookup of the terminal name
>>
>> Other more minor differences are that tsubst_copy exits early when
>> 'args' is null, and it calls maybe_dependent_member_ref

That's curious, since what that function does seems like name lookup; I 
wouldn't think we would want to call it when tf_no_name_lookup.

>> and finally it dispatches to tsubst for type trees.

And it looks like you fix the callers to avoid that?

>> Thus tsubst_copy is (at this point) similar enough to tsubst_copy_and_build
>> that it makes sense to merge the two functions, with the main difference
>> being the name lookup behavior[2].  So this patch merges tsubst_copy into
>> tsubst_copy_and_build via a new tsubst tf_no_name_lookup which controls
>> name lookup and resolution of a (top-level) id-expression.
>>
>> [1]: http://thrifty.mooo.com:8008/gcc-lcov/gcc/cp/pt.cc.gcov.html#17231
>> [2]: I don't know the history of tsubst_copy but I would guess it was
>> added before we settled on using processing_template_decl to control
>> whether our AST building routines perform semantic checking and return
>> non-templated trees, and so we needed a separate tsubst routine that
>> avoids semantic checking and always returns a templated tree for e.g.
>> partial substitution.
> 
> Oops, this is wrong -- tsubst_copy_and_build came after tsubst_copy,
> and was introduced as an optimization with the intent of getting rid
> of tsubst_copy eventually:
> https://gcc.gnu.org/pipermail/gcc-patches/2003-January/093659.html

I wonder if we want to add a small tsubst_name wrapper to call 
tsubst_copy_and_build with tf_no_name_lookup?

Can we also merge in tsubst_expr and use that name instead of the 
unwieldy tsubst_copy_and_build?

Jason