On Fri, Jul 26, 2024 at 08:43:44PM +0200, Jakub Jelinek wrote:
> Yeah, I saw ARGUMENT_PACK_SELECT being used, but didn't notice that
> important
> if (arg_pack && TREE_CODE (arg_pack) == ARGUMENT_PACK_SELECT)
> arg_pack = ARGUMENT_PACK_SELECT_FROM_PACK (arg_pack);
> part of tsubst_pack_expansion where it picks the pack from
> ARGUMENT_PACK_SELECT when needed.
>
> I also saw the iterative_hash_template_arg case and was afraid to modify it
> because of that reuse of ARGUMENT_PACK_SELECT tree.
> But I guess you're right, if there is a flag added to it only set by
> satisfy_fold which needs to create them immutable because there is the
> satisfaction cache etc., it might work. Will try that soon.
With following incremental patch I've made some progress, but still
something to debug...
--- gcc/cp/cp-tree.h 2024-07-26 09:19:10.626908903 +0200
+++ gcc/cp/cp-tree.h 2024-07-26 21:08:02.698018826 +0200
@@ -4100,6 +4100,11 @@
#define ARGUMENT_PACK_SELECT_INDEX(NODE) \
(((struct tree_argument_pack_select *)ARGUMENT_PACK_SELECT_CHECK
(NODE))->index)
+/* True in ARGUMENT_PACK_SELECT which is immutable, won't be modified
+ once it is created and so can be safely hashed. */
+#define ARGUMENT_PACK_SELECT_IMMUTABLE_P(NODE) \
+ TREE_STATIC (ARGUMENT_PACK_SELECT_CHECK (NODE))
+
#define FOLD_EXPR_CHECK(NODE) \
TREE_CHECK4 (NODE, UNARY_LEFT_FOLD_EXPR, UNARY_RIGHT_FOLD_EXPR, \
BINARY_LEFT_FOLD_EXPR, BINARY_RIGHT_FOLD_EXPR)
--- gcc/cp/constraint.cc 2024-07-26 15:03:04.438445216 +0200
+++ gcc/cp/constraint.cc 2024-07-26 21:38:36.447741106 +0200
@@ -2249,7 +2249,7 @@
int index;
template_parm_level_and_index (e, &level, &index);
tree a = TMPL_ARG (args, level, index);
- if (!ARGUMENT_PACK_P (a))
+ if (TREE_CODE (a) == ARGUMENT_PACK_SELECT)
arg = t;
}
}
@@ -2936,6 +2936,8 @@
int level, index;
template_parm_level_and_index (TREE_VALUE (p), &level, &index);
tree a = TMPL_ARG (orig_args, level, index);
+ if (TREE_CODE (a) == ARGUMENT_PACK_SELECT)
+ a = ARGUMENT_PACK_SELECT_FROM_PACK (a);
int this_len = TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (a));
if (len == -1)
len = this_len;
@@ -3005,8 +3007,13 @@
SET_TMPL_ARGS_LEVEL (args, level, copy_node (v));
}
tree a = TMPL_ARG (orig_args, level, index);
- TMPL_ARG (args, level, index)
- = TREE_VEC_ELT (ARGUMENT_PACK_ARGS (a), i);
+ if (TREE_CODE (a) == ARGUMENT_PACK_SELECT)
+ a = ARGUMENT_PACK_SELECT_FROM_PACK (a);
+ tree aps = make_node (ARGUMENT_PACK_SELECT);
+ ARGUMENT_PACK_SELECT_FROM_PACK (aps) = a;
+ ARGUMENT_PACK_SELECT_INDEX (aps) = i;
+ ARGUMENT_PACK_SELECT_IMMUTABLE_P (aps) = 1;
+ TMPL_ARG (args, level, index) = aps;
}
tree result = satisfy_constraint_r (FOLD_CONSTR_EXPR (t), args, info);
if (result == error_mark_node)
--- gcc/cp/pt.cc.jj 2024-07-26 08:34:18.114159967 +0200
+++ gcc/cp/pt.cc 2024-07-26 21:21:56.967434333 +0200
@@ -1763,8 +1763,17 @@ iterative_hash_template_arg (tree arg, h
/* Getting here with an ARGUMENT_PACK_SELECT means we're probably
preserving it in a hash table, which is bad because it will change
meaning when gen_elem_of_pack_expansion_instantiation changes the
- ARGUMENT_PACK_SELECT_INDEX. */
- gcc_unreachable ();
+ ARGUMENT_PACK_SELECT_INDEX. The exception is immutable
+ ARGUMENT_PACK_SELECTs created by constraint.cc (satisfy_fold). */
+ gcc_assert (ARGUMENT_PACK_SELECT_IMMUTABLE_P (arg));
+ val = iterative_hash_template_arg (ARGUMENT_PACK_SELECT_FROM_PACK (arg),
+ val);
+ if (sizeof (ARGUMENT_PACK_SELECT_INDEX (arg)) <= sizeof (hashval_t))
+ return iterative_hash_hashval_t (ARGUMENT_PACK_SELECT_INDEX (arg),
+ val);
+ else
+ return iterative_hash_host_wide_int (ARGUMENT_PACK_SELECT_INDEX (arg),
+ val);
case ERROR_MARK:
return val;
@@ -9464,7 +9473,14 @@ template_args_equal (tree ot, tree nt)
else if (ARGUMENT_PACK_P (ot) || ARGUMENT_PACK_P (nt))
return cp_tree_equal (ot, nt);
else if (TREE_CODE (ot) == ARGUMENT_PACK_SELECT)
- gcc_unreachable ();
+ {
+ gcc_assert (ARGUMENT_PACK_SELECT_IMMUTABLE_P (ot)
+ && ARGUMENT_PACK_SELECT_IMMUTABLE_P (nt));
+ return (ARGUMENT_PACK_SELECT_INDEX (ot)
+ == ARGUMENT_PACK_SELECT_INDEX (nt)
+ && template_args_equal (ARGUMENT_PACK_SELECT_FROM_PACK (ot),
+ ARGUMENT_PACK_SELECT_FROM_PACK (nt)));
+ }
else if (TYPE_P (nt) || TYPE_P (ot))
{
if (!(TYPE_P (nt) && TYPE_P (ot)))
Jakub
