On 10/4/23 12:08, Patrick Palka wrote:
On Tue, 3 Oct 2023, Jason Merrill wrote:
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.
Ah, that makes sense I think.
and finally it dispatches to tsubst for type trees.
And it looks like you fix the callers to avoid that?
Yes, I'll make a note of that in the commit message.
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?
Good idea, that'll complement tsubst_scope nicely.
Can we also merge in tsubst_expr and use that name instead of the unwieldy
tsubst_copy_and_build?
That'd be nice. Another idea would be to rename tsubst_expr to
tsubst_stmt and make it disjoint from tsubst_copy_and_build, and then
rename tsubst_copy_and_build to tsubst_expr, to draw a distinction
between statement-like trees (the substitution of which typically has
side effects like calling add_stmt) and expression-like trees (which
don't usually have such side effects). I can work on that as a
follow-up patch.
Here's v2 which guards the call to maybe_dependent_member_ref and adds
tsubst_name, bootstrapped and regtested on x86_64-pc-linux-gnu:
OK.
