On Wed, Oct 23, 2024 at 10:20:39AM -0400, Patrick Palka wrote:
> On Tue, 22 Oct 2024, Marek Polacek wrote:
>
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> >
> > -- >8 --
> > This patch implements C++26 Pack Indexing, as described in
> > <https://wg21.link/P2662R3>.
> >
> > The issue discussing how to mangle pack indexes has not been resolved
> > yet <https://github.com/itanium-cxx-abi/cxx-abi/issues/175> and I've
> > made no attempt to address it so far.
> >
> > Rather than introducing a new template code for a pack indexing, I'm
> > adding a new operand to EXPR_PACK_EXPANSION to store the index; for
> > TYPE_PACK_EXPANSION, I'm stashing the index into TYPE_VALUES_RAW. This
>
> What are the pros and cons of reusing TYPE/EXPR_PACK_EXPANSION instead
> of creating two new tree codes for these operators (one of whose
> operands would itself be a bare TYPE/EXPR_PACK_EXPANSION)?
>
> I feel a little iffy at first glance about reusing these tree codes
> since it muddles what "kind" of tree they are: currently they represent
> a _vector_ or types/exprs (which is reflected by their tcc_exceptional
> class), and with this approach they can now also represent a single
> type/expr (despite their tcc_exceptional class), depending on whether
> PACK_EXPANSION_INDEX is set.
>
> At the same time, the pattern of a generic *_PACK_EXPANSION can be
> anything whereas for these index operators we know it's always a single
> bare pack, so we also don't need the full expressivity of
> *_PACK_EXPANSION to represent these operators either.
>
> Maybe it's the case that reusing these tree codes significantly
> simplifies parts of the implementation?
That was pretty much the reason. But now I think it's cleaner to introduce
new codes, and that the clarity is worth the pain of adding new codes.
> > @@ -13814,6 +13833,10 @@ tsubst_pack_expansion (tree t, tree args,
> > tsubst_flags_t complain,
> > {
> > tree args = ARGUMENT_PACK_ARGS (TREE_VALUE (packs));
> >
> > + /* C++26 Pack Indexing. */
> > + if (index)
> > + return pack_index_element (index, args, complain);
>
> I'd expect every pack index operator to hit this code path since its
> pattern should always be a bare pack...
>
> > +
> > /* If the argument pack is a single pack expansion, pull it out. */
> > if (TREE_VEC_LENGTH (args) == 1
> > && pack_expansion_args_count (args))
> > @@ -13946,6 +13969,10 @@ tsubst_pack_expansion (tree t, tree args,
> > tsubst_flags_t complain,
> > && PACK_EXPANSION_P (TREE_VEC_ELT (result, 0)))
> > return TREE_VEC_ELT (result, 0);
> >
> > + /* C++26 Pack Indexing. */
> > + if (index)
> > + return pack_index_element (index, result, complain);
>
> ... so this code path should be necessary?
This is no longer in the v2 patch.
Marek
