On Fri, 28 Nov 2025, Jason Merrill wrote: > On 11/27/25 12:53 AM, Marek Polacek wrote: > > On Wed, Nov 26, 2025 at 01:29:35PM -0500, Patrick Palka wrote: > > > On Wed, 26 Nov 2025, Patrick Palka wrote: > > > > > > > On Wed, 26 Nov 2025, Marek Polacek wrote: > > > > > > > > > On Mon, Nov 24, 2025 at 08:20:10PM -0500, Patrick Palka wrote: > > > > > > On Mon, 24 Nov 2025, Marek Polacek wrote: > > > > > > > > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/15? > > > > > > > > > > > > > > -- >8 -- > > > > > > > In my r15-6792 patch I added a call to tsubst in tsubst_pack_index > > > > > > > to fully instantiate args#N in the pack. > > > > > > > > > > > > > > Here we are in an unevaluated context, but since the pack is > > > > > > > a TREE_VEC, we call tsubst_template_args which has cp_evaluated > > > > > > > at the beginning. That causes a crash because we trip on the > > > > > > > assert in tsubst_expr/PARM_DECL: > > > > > > > > > > > > > > gcc_assert (cp_unevaluated_operand); > > > > > > > > > > > > > > because retrieve_local_specialization didn't find anything (becase > > > > > > > there are no local_specializations yet). > > > > > > > > > > > > > > ISTM that we don't need a full instantiation in an unevaluated > > > > > > > context > > > > > > > so we can avoid the crash like this. > > > > > > > > > > > > Hmm, it doesn't seem right to avoid doing a substitution solely > > > > > > because > > > > > > we're in an unevaluated context.. > > > > > > > > > > > > Here the TREE_VEC is just a subexpression of the templated > > > > > > PACK_INDEX_EXPR, not a template argument list, so we should still > > > > > > substitute it normally, just without setting cp_evaluated, I think. > > > > > > > > > > I think you're right. How about I just walk the TREE_VEC and subst > > > > > each element, like this? > > > > > > > > In-place modification of a templated tree is unusual for tsubst, we > > > > probably should just return a new TREE_VEC. I think we could use a > > > > version of tsubst_template_args that doesn't do cp_evaluated > > > > (tsubst_tree_vec?), define tsubst_template_args in terms of that, and > > > > also use it here? > > > > > > Actually never mind about the idea of defining tsubst_template_args > > > in terms of tsubst_tree_vec... tsubst_template_args does a lot of > > > stuff that's specific to template arguments which wouldn't be suitable > > > for TREE_VEC expression substitution. I suppose we could just use a > > > standalone tsubst_tree_vec routine that tsubsts each element and returns > > > a new TREE_VEC. > > Fair enough. > > > > > (I believe TRAIT_EXPR is another tree that has a TREE_VEC subexpression > > > and probably has a similiar bug wrt cp_evaluated being set, so we could > > > use tsubst_tree_vec there as well.) > > > > I've not done that yet. > > > > How does this look? Thanks, > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > -- >8 -- > > In my r15-6792 patch I added a call to tsubst in tsubst_pack_index > > to fully instantiate args#N in the pack. > > > > Here we are in an unevaluated context, but since the pack is > > a TREE_VEC, we call tsubst_template_args which has cp_evaluated > > at the beginning. That causes a crash because we trip on the > > assert in tsubst_expr/PARM_DECL: > > > > gcc_assert (cp_unevaluated_operand); > > > > because retrieve_local_specialization didn't find anything (becase > > there are no local_specializations yet). > > > > We can avoid the cp_evaluated by calling the new tsubst_tree_vec, > > which creates a new TREE_VEC and substitutes each element. > > > > PR c++/121325 > > > > gcc/cp/ChangeLog: > > > > * pt.cc (tsubst_tree_vec): New. > > (tsubst_pack_index): Call it. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/cpp26/pack-indexing18.C: New test. > > > > Reviewed-by: Patrick Palka <[email protected]> > > --- > > gcc/cp/pt.cc | 21 ++++++++++++- > > gcc/testsuite/g++.dg/cpp26/pack-indexing18.C | 32 ++++++++++++++++++++ > > 2 files changed, 52 insertions(+), 1 deletion(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp26/pack-indexing18.C > > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > > index e74e34d8149..4dc8f980d0d 100644 > > --- a/gcc/cp/pt.cc > > +++ b/gcc/cp/pt.cc > > @@ -14203,6 +14203,25 @@ tsubst_pack_expansion (tree t, tree args, > > tsubst_flags_t complain, > > return result; > > } > > +/* Substitute ARGS into T, which is a TREE_VEC. This function creates a > > new > > + TREE_VEC rather than substituting the elements in-place. */ > > + > > +static tree > > +tsubst_tree_vec (tree t, tree args, tsubst_flags_t complain, tree in_decl) > > +{ > > + const int len = TREE_VEC_LENGTH (t); > > + tree r = make_tree_vec (len); > > + for (int i = 0; i < len; ++i) > > + { > > + tree arg = TREE_VEC_ELT (t, i); > > + if (TYPE_P (arg)) > > + TREE_VEC_ELT (r, i) = tsubst (arg, args, complain, in_decl); > > + else > > + TREE_VEC_ELT (r, i) = tsubst_expr (arg, args, complain, in_decl); > > + } > > + return r; > > +} > > + > > /* Substitute ARGS into T, which is a pack index (i.e., PACK_INDEX_TYPE or > > PACK_INDEX_EXPR). Returns a single type or expression, a PACK_INDEX_* > > node if only a partial substitution could be performed, or > > ERROR_MARK_NODE > > @@ -14220,7 +14239,7 @@ tsubst_pack_index (tree t, tree args, tsubst_flags_t > > complain, tree in_decl) > > a partially instantiated closure. Let tsubst find the > > fully-instantiated one. */ > > gcc_assert (TREE_CODE (pack) == TREE_VEC); > > - pack = tsubst (pack, args, complain, in_decl); > > + pack = tsubst_tree_vec (pack, args, complain, in_decl); > > In this case, why can't we tsubst just the element at the desired index?
I suppose we can if we substitute the index first and see it's not still dependent. But it seems we still need something like tsubst_tree_vec for when the pack is a TREE_VEC and the index is still dependent I think? > > Jason > >
