On 10/22/24 3:22 PM, 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>.
Great!
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
feature is akin to __type_pack_element, so they can share the element
extraction part.
A pack indexing in a decltype proved to be a bit tricky; eventually,
I've added PACK_EXPANSION_PARENTHESIZED_P -- while parsing, we can't
really tell what it's going to expand to.
As I comment below, I think we should have enough information while
parsing; what it expands to doesn't matter.
With this feature, it's valid to write something like
using U = tmpl<Ts...[Is]...>;
where we first expand the template argument into
Ts...[Is#0], Ts...[Is#1], ...
and then substitute each individual pack index.
I have no test for the module.cc code, that is just guesswork.
Looks straightforward enough.
@@ -2605,6 +2605,8 @@ write_type (tree type)
case TYPE_PACK_EXPANSION:
write_string ("Dp");
write_type (PACK_EXPANSION_PATTERN (type));
+ /* TODO: Mangle PACK_EXPANSION_INDEX
+ <https://github.com/itanium-cxx-abi/cxx-abi/issues/175> */
Could we warn about this so it doesn't get forgotten? And similarly in
write_expression?
@@ -3952,7 +3953,11 @@ find_parameter_packs_r (tree *tp, int *walk_subtrees,
void* data)
break;
case VAR_DECL:
- if (DECL_PACK_P (t))
+ /* We can have
+ T...[0] a;
+ (T...[0])(a); // #1
+ where the 'a' in #1 is not a bare parameter pack. */
+ if (DECL_PACK_P (t) && !PACK_EXPANSION_INDEX (TREE_TYPE (t)))
Seems like the INDEX check should move into DECL_PACK_P?
Why doesn't this apply to PARM_DECL above?
@@ -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);
Could we only compute the desired element rather than computing all of
them and selecting the desired one?
@@ -16897,17 +16924,23 @@ tsubst (tree t, tree args, tsubst_flags_t complain,
tree in_decl)
ctx = tsubst_pack_expansion (ctx, args,
complain | tf_qualifying_scope,
in_decl);
- if (ctx == error_mark_node
- || TREE_VEC_LENGTH (ctx) > 1)
+ if (ctx == error_mark_node)
return error_mark_node;
- if (TREE_VEC_LENGTH (ctx) == 0)
+ /* If there was a pack-index-specifier, we won't get a TREE_VEC,
+ just the single element. */
+ if (TREE_CODE (ctx) == TREE_VEC)
{
- if (complain & tf_error)
- error ("%qD is instantiated for an empty pack",
- TYPENAME_TYPE_FULLNAME (t));
- return error_mark_node;
+ if (TREE_VEC_LENGTH (ctx) > 1)
+ return error_mark_node;
This is preexisting, but it seems like we're missing a call to error()
in this case.
@@ -17041,13 +17074,20 @@ tsubst (tree t, tree args, tsubst_flags_t complain,
tree in_decl)
else
{
bool id = DECLTYPE_TYPE_ID_EXPR_OR_MEMBER_ACCESS_P (t);
- if (id && TREE_CODE (DECLTYPE_TYPE_EXPR (t)) == BIT_NOT_EXPR
- && EXPR_P (type))
+ tree op = DECLTYPE_TYPE_EXPR (t);
+ if (id && TREE_CODE (op) == BIT_NOT_EXPR && EXPR_P (type))
/* In a template ~id could be either a complement expression
or an unqualified-id naming a destructor; if instantiating
it produces an expression, it's not an id-expression or
member access. */
id = false;
+ /* With pack indexing, we don't know what it's going to expand to
+ until instantiation. The intent is that a pack indexing
+ expression behaves exactly as the underlying expression
+ would. */
+ else if (PACK_EXPANSION_P (op))
+ id = (!PACK_EXPANSION_PARENTHESIZED_P (op)
+ && unparenthesized_id_or_class_member_access_p (type));
Why isn't the value of DECLTYPE_TYPE_ID_EXPR_OR_MEMBER_ACCESS_P already
accurate in this case? It seems to me that 'id' should be based on the
syntax of the decltype, not what the substitution produces.
And so maybe we don't need PACK_EXPANSION_PARENTHESIZED_P except maybe
for decltype(auto)?
@@ -17074,6 +17114,11 @@ tsubst (tree t, tree args, tsubst_flags_t complain,
tree in_decl)
case NONTYPE_ARGUMENT_PACK:
return tsubst_argument_pack (t, args, complain, in_decl);
+ case TYPE_PACK_EXPANSION:
+ if (PACK_EXPANSION_INDEX (t))
+ return tsubst_pack_expansion (t, args, complain, in_decl);
+ gcc_fallthrough ();
Let's gcc_unreachable here rather than fallthrough to unreachable.
@@ -19596,6 +19641,8 @@ tsubst_stmt (tree t, tree args, tsubst_flags_t
complain, tree in_decl)
}
case EXPR_PACK_EXPANSION:
+ if (PACK_EXPANSION_INDEX (t))
+ RETURN (tsubst_pack_expansion (t, args, complain, in_decl));
error ("invalid use of pack expansion expression");
RETURN (error_mark_node);
@@ -21776,6 +21823,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
}
case EXPR_PACK_EXPANSION:
+ if (PACK_EXPANSION_INDEX (t))
+ RETURN (tsubst_pack_expansion (t, args, complain, in_decl));
error ("invalid use of pack expansion expression");
RETURN (error_mark_node);
Preexisting, but do we need this case in both _stmt and _expr?
@@ -27685,6 +27734,14 @@ tsubst_initializer_list (tree t, tree argvec)
NULL_TREE);
if (expanded_bases == error_mark_node)
continue;
+ /* If there was a pack-index-specifier, we won't get
+ a TREE_VEC but the rest of the code assumes so. */
Hmm, this difference in return value seems fragile, but I guess you only
actually need to compensate for it in a couple of places...
@@ -4181,7 +4183,9 @@ finish_base_specifier (tree base, tree access, bool
virtual_p)
error ("invalid base-class specification");
result = NULL_TREE;
}
- else if (! MAYBE_CLASS_TYPE_P (base))
+ else if (! MAYBE_CLASS_TYPE_P (base)
+ && ! (PACK_EXPANSION_P (base)
+ && PACK_EXPANSION_INDEX (base)))
Instead of this change, this case should be added to WILDCARD_TYPE_P.
-/* Implement the __type_pack_element keyword: Return the type
- at index IDX within TYPES. */
+/* Return the type at index IDX within TYPES. */
static tree
-finish_type_pack_element (tree idx, tree types, tsubst_flags_t complain)
+get_vec_elt_checking (tree idx, tree types, bool pack_index_p,
+ tsubst_flags_t complain)
{
idx = maybe_constant_value (idx);
if (TREE_CODE (idx) != INTEGER_CST || !INTEGRAL_TYPE_P (TREE_TYPE (idx)))
{
if (complain & tf_error)
- error ("%<__type_pack_element%> index is not an integral constant");
+ {
+ if (pack_index_p)
+ error ("pack index is not an integral constant");
+ else
+ error ("%<__type_pack_element%> index is not an integral constant");
Maybe just always say "pack index"?
@@ -5575,12 +5581,16 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p,
walk_tree_fn func,
case TYPE_PACK_EXPANSION:
WALK_SUBTREE (TREE_TYPE (t));
WALK_SUBTREE (PACK_EXPANSION_EXTRA_ARGS (t));
+ if (PACK_EXPANSION_INDEX (t))
+ WALK_SUBTREE (PACK_EXPANSION_INDEX (t));
I don't think it's necessary to check non-null before WALK_SUBTREE.
Jason
