On 8/30/21 10:05 PM, Patrick Palka wrote:
Here when partially substituting into the pack expansion, substitution
into the constexpr if yields a still-dependent tree, so tsubst_expr
returns an IF_STMT with an unsubstituted IF_COND and with
IF_STMT_EXTRA_ARGS added to. Hence after partial substitution
the pack expansion pattern still refers to the parameter pack 'ts' of
level 2 (and it's thus represented in the new PACK_EXPANSION_PARAMETER_PACKS)
even though the partially instantiated generic lambda admits only one
level of template arguments.
This causes us to crash during the
subsequent instantiation with the lambda's template arguments because of
the level mismatch. (Likewise when the constexpr if is replaced by a
requires-expr, which too uses the extra args mechanism for delaying
partial instantiation.)
So essentially, a pack expansion pattern that contains a parameter pack
inside an "extra args" tree doesn't play well with partial substitution
thereof. This patch fixes this by forcing such pack expansions to use
the extra args mechanism as well.
Why is this specific to parameter packs? Won't non-pack template
parameters also suffer from the level mismatch? I'd think it would be
simpler to just note when the pattern contains a constexpr if or
requires-expression, for which we can't substitute into the pattern like
a pack expansion, and know we need to use extra args in that case.
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?
PR c++/101764
gcc/cp/ChangeLog:
* cp-tree.h (PACK_EXPANSION_FORCE_EXTRA_ARGS_P): New accessor
macro.
* pt.c (uses_extra_args_mechanism_p): New function.
(find_parameter_pack_data::found_pack_within_extra_args_tree_p):
New data member.
(find_parameter_pack_data::inside_extra_args_tree_p): Likewise.
(find_parameter_packs_r): Detect parameter packs within "extra
args" trees and set found_pack_within_extra_args_tree_p
appropriately.
(make_pack_expansion): Set PACK_EXPANSION_FORCE_EXTRA_ARGS_P if
found_pack_within_extra_args_tree_p.
(use_pack_expansion_extra_args_p): Return true if there were
unsubstituted packs and PACK_EXPANSION_FORCE_EXTRA_ARGS_P.
(tsubst_pack_expansion): Pass the pack expansion to
use_pack_expansion_extra_args_p.
gcc/testsuite/ChangeLog:
* g++.dg/cpp1z/constexpr-if35.C: New test.
---
gcc/cp/cp-tree.h | 5 ++
gcc/cp/pt.c | 69 ++++++++++++++++++++-
gcc/testsuite/g++.dg/cpp1z/constexpr-if35.C | 18 ++++++
3 files changed, 90 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-if35.C
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index ce7ca53a113..06dec495428 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -493,6 +493,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
CONSTRUCTOR_C99_COMPOUND_LITERAL (in CONSTRUCTOR)
OVL_NESTED_P (in OVERLOAD)
DECL_MODULE_EXPORT_P (in _DECL)
+ PACK_EXPANSION_FORCE_EXTRA_ARGS_P (in *_PACK_EXPANSION)
4: IDENTIFIER_MARKED (IDENTIFIER_NODEs)
TREE_HAS_CONSTRUCTOR (in INDIRECT_REF, SAVE_EXPR, CONSTRUCTOR,
CALL_EXPR, or FIELD_DECL).
@@ -3902,6 +3903,10 @@ struct GTY(()) lang_decl {
/* True iff this pack expansion is for auto... in lambda init-capture. */
#define PACK_EXPANSION_AUTO_P(NODE) TREE_LANG_FLAG_2 (NODE)
+/* True if we must use PACK_EXPANSION_EXTRA_ARGS to avoid partial
+ substitution into this pack expansion. */
+#define PACK_EXPANSION_FORCE_EXTRA_ARGS_P(NODE) TREE_LANG_FLAG_3 (NODE)
+
/* True iff the wildcard can match a template parameter pack. */
#define WILDCARD_PACK_P(NODE) TREE_LANG_FLAG_0 (NODE)
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index fcf3ac31b25..a92dff88d9d 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -3855,6 +3855,20 @@ expand_builtin_pack_call (tree call, tree args,
tsubst_flags_t complain,
return NULL_TREE;
}
+/* Return true if the tree T uses the extra args mechanism for
+ deferring partial substitution into it. */
+
+static bool
+uses_extra_args_mechanism_p (tree t)
+{
+ return (PACK_EXPANSION_P (t)
+ || TREE_CODE (t) == REQUIRES_EXPR
+ || (TREE_CODE (t) == IF_STMT
+ && IF_STMT_CONSTEXPR_P (t)));
+}
+
+static tree find_parameter_packs_r (tree *, int *, void*);
+
/* Structure used to track the progress of find_parameter_packs_r. */
struct find_parameter_pack_data
{
@@ -3867,6 +3881,16 @@ struct find_parameter_pack_data
/* True iff we're making a type pack expansion. */
bool type_pack_expansion_p;
+
+ /* True iff we found a pack inside a subtree that uses the extra
+ args mechanism. */
+ bool found_pack_within_extra_args_tree_p = false;
+
+private:
+ /* True iff find_parameter_packs_r is currently visiting a tree
+ that uses the extra args mechanism or a subtree thereof. */
+ bool inside_extra_args_tree_p = false;
+ friend tree find_parameter_packs_r (tree *, int *, void*);
};
/* Identifies all of the argument packs that occur in a template
@@ -3968,6 +3992,37 @@ find_parameter_packs_r (tree *tp, int *walk_subtrees,
void* data)
*ppd->parameter_packs = tree_cons (NULL_TREE, t, *ppd->parameter_packs);
}
+ if (!ppd->inside_extra_args_tree_p
Why this flag?
+ && uses_extra_args_mechanism_p (t)
+ && !PACK_EXPANSION_P (t))
+ {
+ /* This tree is using the extra args mechanism. Update *ppd and recurse
+ so that we can try to detect a parameter pack within. */
This comment needs to mention why pack expansions are different. And it
seems wrong to say that the tree *is using* the mechanism; it's a tree
that would use the mechanism in case of partial instantiation.
+ tree parameter_packs = NULL_TREE;
+ hash_set<tree> visited;
+
+ {
+ auto iovr = make_temp_override (ppd->inside_extra_args_tree_p,
+ true);
+ auto povr = make_temp_override (ppd->parameter_packs,
+ ¶meter_packs);
+ auto vovr = make_temp_override (ppd->visited,
+ &visited);
Why override these?
+ WALK_SUBTREE (t);
+ }
+
+ if (parameter_packs)
+ {
+ /* We found some parameter packs within this extra args tree. */
+ ppd->found_pack_within_extra_args_tree_p = true;
+ /* Walk them so that they get added to the overall list. */
+ WALK_SUBTREE (parameter_packs);
+ }
+
+ *walk_subtrees = 0;
+ return NULL_TREE;
+ }
+
if (TYPE_P (t))
cp_walk_tree (&TYPE_CONTEXT (t),
&find_parameter_packs_r, ppd, ppd->visited);
@@ -4229,6 +4284,11 @@ make_pack_expansion (tree arg, tsubst_flags_t complain)
PACK_EXPANSION_PARAMETER_PACKS (result) = parameter_packs;
PACK_EXPANSION_LOCAL_P (result) = at_function_scope_p ();
+ if (ppd.found_pack_within_extra_args_tree_p)
+ /* If the pattern of this pack expansion contains a parameter pack within
+ a subtree that uses the extra args mechanism, then the pack expansion
+ must also use the extra args mechanism (101764). */
+ PACK_EXPANSION_FORCE_EXTRA_ARGS_P (result) = true;
return result;
}
@@ -12392,10 +12452,15 @@ make_argument_pack_select (tree arg_pack, unsigned
index)
substitution. */
static bool
-use_pack_expansion_extra_args_p (tree parm_packs,
+use_pack_expansion_extra_args_p (tree t,
+ tree parm_packs,
int arg_pack_len,
bool has_empty_arg)
{
+ if (has_empty_arg
+ && PACK_EXPANSION_FORCE_EXTRA_ARGS_P (t))
+ return true;
+
/* If one pack has an expansion and another pack has a normal
argument or if one pack has an empty argument and an another
one hasn't then tsubst_pack_expansion cannot perform the
@@ -13148,7 +13213,7 @@ tsubst_pack_expansion (tree t, tree args,
tsubst_flags_t complain,
/* We cannot expand this expansion expression, because we don't have
all of the argument packs we need. */
- if (use_pack_expansion_extra_args_p (packs, len, unsubstituted_packs))
+ if (use_pack_expansion_extra_args_p (t, packs, len, unsubstituted_packs))
{
/* We got some full packs, but we can't substitute them in until we
have values for all the packs. So remember these until then. */
diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-if35.C
b/gcc/testsuite/g++.dg/cpp1z/constexpr-if35.C
new file mode 100644
index 00000000000..cae690ba82c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-if35.C
@@ -0,0 +1,18 @@
+// PR c++/101764
+// { dg-do compile { target c++17 } }
+
+void g(...);
+
+template<class>
+auto f() {
+ return [](auto... ts) {
+ g([] { if constexpr (decltype(ts){0}); }...);
+#if __cpp_concepts
+ g(requires { decltype(ts){0}; }...);
+#endif
+ };
+}
+
+int main() {
+ f<int>()('a', true);
+}
