| Message ID | 20210708152804.2235731-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: requires-expr with dependent extra args [PR101181] | expand |
On 7/8/21 11:28 AM, Patrick Palka wrote: > Here we're crashing ultimately because the mechanism for delaying > substitution into a requires-expression (or constexpr if) doesn't > expect to see dependent args. But we end up capturing dependent > args here when substituting into the default template argument during > coerce_template_parms for the dependent specialization p<T>. > > This patch enables the commented out code in add_extra_args for > handling this situation. It turns out we also need to make a copy of > the captured arguments so that coerce_template_parms doesn't later > add to the argument, which would form an unexpected cycle. And we > need to make tsubst_template_args more forgiving about missing template > arguments, since the arguments we capture from coerce_template_parms are > incomplete. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk/11? > > PR c++/101181 > > gcc/cp/ChangeLog: > > * constraint.cc (tsubst_requires_expr): Pass complain/in_decl to > add_extra_args. > * cp-tree.h (add_extra_args): Add complain/in_decl parameters. > * pt.c (build_extra_args): Make a copy of args. > (add_extra_args): Add complain/in_decl parameters. Handle the > case where the extra arguments are dependent. > (tsubst_pack_expansion): Pass complain/in_decl to > add_extra_args. > (tsubst_template_args): Handle missing template arguments. > (tsubst_expr) <case IF_STMT>: Pass complain/in_decl to > add_extra_args. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp2a/concepts-requires26.C: New test. > * g++.dg/cpp2a/lambda-uneval16.C: New test. > --- > gcc/cp/constraint.cc | 3 +- > gcc/cp/cp-tree.h | 2 +- > gcc/cp/pt.c | 31 +++++++++---------- > .../g++.dg/cpp2a/concepts-requires26.C | 18 +++++++++++ > gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C | 22 +++++++++++++ > 5 files changed, 58 insertions(+), 18 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-requires26.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > index 99d3ccc6998..4ee5215df50 100644 > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -2266,7 +2266,8 @@ tsubst_requires_expr (tree t, tree args, sat_info info) > /* A requires-expression is an unevaluated context. */ > cp_unevaluated u; > > - args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args); > + args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args, > + info.complain, info.in_decl); > if (processing_template_decl) > { > /* We're partially instantiating a generic lambda. Substituting into > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index 58da7460001..0a5f13489cc 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -7289,7 +7289,7 @@ extern void add_mergeable_specialization (bool is_decl, bool is_alias, > tree outer, unsigned); > extern tree add_to_template_args (tree, tree); > extern tree add_outermost_template_args (tree, tree); > -extern tree add_extra_args (tree, tree); > +extern tree add_extra_args (tree, tree, tsubst_flags_t, tree); > extern tree build_extra_args (tree, tree, tsubst_flags_t); > > /* in rtti.c */ > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > index 06116d16887..e4bdac087ad 100644 > --- a/gcc/cp/pt.c > +++ b/gcc/cp/pt.c > @@ -12928,7 +12928,9 @@ extract_local_specs (tree pattern, tsubst_flags_t complain) > tree > build_extra_args (tree pattern, tree args, tsubst_flags_t complain) > { > - tree extra = args; > + /* Make a copy of the extra arguments so that they won't get changed > + from under us. */ > + tree extra = copy_template_args (args); > if (local_specializations) > if (tree locals = extract_local_specs (pattern, complain)) > extra = tree_cons (NULL_TREE, extra, locals); > @@ -12939,7 +12941,7 @@ build_extra_args (tree pattern, tree args, tsubst_flags_t complain) > normal template args to ARGS. */ > > tree > -add_extra_args (tree extra, tree args) > +add_extra_args (tree extra, tree args, tsubst_flags_t complain, tree in_decl) > { > if (extra && TREE_CODE (extra) == TREE_LIST) > { > @@ -12959,20 +12961,14 @@ add_extra_args (tree extra, tree args) > gcc_assert (!TREE_PURPOSE (extra)); > extra = TREE_VALUE (extra); > } > -#if 1 > - /* I think we should always be able to substitute dependent args into the > - pattern. If that turns out to be incorrect in some cases, enable the > - alternate code (and add complain/in_decl parms to this function). */ Ah, because these cases aren't pack expansions, so we aren't trying to do the substitution; I wonder if it would be feasible to do so. But this approach is probably simpler. OK. > - gcc_checking_assert (!uses_template_parms (extra)); > -#else > - if (!uses_template_parms (extra)) > + if (uses_template_parms (extra)) > { > - gcc_unreachable (); > + /* This can happen during dependent substitution into a requires-expr > + or a lambda that uses constexpr if. */ > extra = tsubst_template_args (extra, args, complain, in_decl); > args = add_outermost_template_args (args, extra); > } > else > -#endif > args = add_to_template_args (extra, args); > return args; > } > @@ -12998,7 +12994,7 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain, > pattern = PACK_EXPANSION_PATTERN (t); > > /* Add in any args remembered from an earlier partial instantiation. */ > - args = add_extra_args (PACK_EXPANSION_EXTRA_ARGS (t), args); > + args = add_extra_args (PACK_EXPANSION_EXTRA_ARGS (t), args, complain, in_decl); > > levels = TMPL_ARGS_DEPTH (args); > > @@ -13370,7 +13366,9 @@ tsubst_template_args (tree t, tree args, tsubst_flags_t complain, tree in_decl) > tree orig_arg = TREE_VEC_ELT (t, i); > tree new_arg; > > - if (TREE_CODE (orig_arg) == TREE_VEC) > + if (!orig_arg) > + new_arg = NULL_TREE; > + else if (TREE_CODE (orig_arg) == TREE_VEC) > new_arg = tsubst_template_args (orig_arg, args, complain, in_decl); > else if (PACK_EXPANSION_P (orig_arg)) > { > @@ -13420,8 +13418,9 @@ tsubst_template_args (tree t, tree args, tsubst_flags_t complain, tree in_decl) > } > for (i = 0, out = 0; i < len; i++) > { > - if ((PACK_EXPANSION_P (TREE_VEC_ELT (orig_t, i)) > - || ARGUMENT_PACK_P (TREE_VEC_ELT (orig_t, i))) > + tree orig_arg = TREE_VEC_ELT (orig_t, i); > + if (orig_arg > + && (PACK_EXPANSION_P (orig_arg) || ARGUMENT_PACK_P (orig_arg)) > && TREE_CODE (elts[i]) == TREE_VEC) > { > int idx; > @@ -18449,7 +18448,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl, > IF_STMT_CONSTEXPR_P (stmt) = IF_STMT_CONSTEXPR_P (t); > IF_STMT_CONSTEVAL_P (stmt) = IF_STMT_CONSTEVAL_P (t); > if (IF_STMT_CONSTEXPR_P (t)) > - args = add_extra_args (IF_STMT_EXTRA_ARGS (t), args); > + args = add_extra_args (IF_STMT_EXTRA_ARGS (t), args, complain, in_decl); > tmp = RECUR (IF_COND (t)); > tmp = finish_if_stmt_cond (tmp, stmt); > if (IF_STMT_CONSTEXPR_P (t) > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-requires26.C b/gcc/testsuite/g++.dg/cpp2a/concepts-requires26.C > new file mode 100644 > index 00000000000..824d23431f0 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-requires26.C > @@ -0,0 +1,18 @@ > +// PR c++/101181 > +// { dg-do compile { target c++20 } } > + > +template<class T, > + bool = requires { typename T::type; }> > +struct p { using type = void; }; > + > +template<class T> > +struct p<T, true> { using type = typename T::type; }; > + > +template<class T> using P = typename p<T>::type; > + > +using type1 = P<int>; > +using type1 = void; > + > +struct A { using type = char; }; > +using type2 = P<A>; > +using type2 = char; > diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C b/gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C > new file mode 100644 > index 00000000000..1113c6f02b9 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C > @@ -0,0 +1,22 @@ > +// PR c++/101181 > +// { dg-do compile { target c++20 } } > + > +template<class T, > + bool = [] () -> bool { > + if constexpr (requires { typename T::type; }) > + return true; > + return false; > + }()> > +struct p { using type = void; }; > + > +template<class T> > +struct p<T, true> { using type = typename T::type; }; > + > +template<class T> using P = typename p<T>::type; > + > +using type1 = P<int>; > +using type = void; > + > +struct A { using type = char; }; > +using type2 = P<A>; > +using type2 = char; >
On Thu, 8 Jul 2021, Jason Merrill wrote: > On 7/8/21 11:28 AM, Patrick Palka wrote: > > Here we're crashing ultimately because the mechanism for delaying > > substitution into a requires-expression (or constexpr if) doesn't > > expect to see dependent args. But we end up capturing dependent > > args here when substituting into the default template argument during > > coerce_template_parms for the dependent specialization p<T>. > > > > This patch enables the commented out code in add_extra_args for > > handling this situation. It turns out we also need to make a copy of > > the captured arguments so that coerce_template_parms doesn't later > > add to the argument, which would form an unexpected cycle. And we > > need to make tsubst_template_args more forgiving about missing template > > arguments, since the arguments we capture from coerce_template_parms are > > incomplete. > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > > trunk/11? > > > > PR c++/101181 > > > > gcc/cp/ChangeLog: > > > > * constraint.cc (tsubst_requires_expr): Pass complain/in_decl to > > add_extra_args. > > * cp-tree.h (add_extra_args): Add complain/in_decl parameters. > > * pt.c (build_extra_args): Make a copy of args. > > (add_extra_args): Add complain/in_decl parameters. Handle the > > case where the extra arguments are dependent. > > (tsubst_pack_expansion): Pass complain/in_decl to > > add_extra_args. > > (tsubst_template_args): Handle missing template arguments. > > (tsubst_expr) <case IF_STMT>: Pass complain/in_decl to > > add_extra_args. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/cpp2a/concepts-requires26.C: New test. > > * g++.dg/cpp2a/lambda-uneval16.C: New test. > > --- > > gcc/cp/constraint.cc | 3 +- > > gcc/cp/cp-tree.h | 2 +- > > gcc/cp/pt.c | 31 +++++++++---------- > > .../g++.dg/cpp2a/concepts-requires26.C | 18 +++++++++++ > > gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C | 22 +++++++++++++ > > 5 files changed, 58 insertions(+), 18 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-requires26.C > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > index 99d3ccc6998..4ee5215df50 100644 > > --- a/gcc/cp/constraint.cc > > +++ b/gcc/cp/constraint.cc > > @@ -2266,7 +2266,8 @@ tsubst_requires_expr (tree t, tree args, sat_info > > info) > > /* A requires-expression is an unevaluated context. */ > > cp_unevaluated u; > > - args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args); > > + args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args, > > + info.complain, info.in_decl); > > if (processing_template_decl) > > { > > /* We're partially instantiating a generic lambda. Substituting > > into > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > > index 58da7460001..0a5f13489cc 100644 > > --- a/gcc/cp/cp-tree.h > > +++ b/gcc/cp/cp-tree.h > > @@ -7289,7 +7289,7 @@ extern void add_mergeable_specialization (bool > > is_decl, bool is_alias, > > tree outer, unsigned); > > extern tree add_to_template_args (tree, tree); > > extern tree add_outermost_template_args (tree, tree); > > -extern tree add_extra_args (tree, tree); > > +extern tree add_extra_args (tree, tree, tsubst_flags_t, > > tree); > > extern tree build_extra_args (tree, tree, > > tsubst_flags_t); > > /* in rtti.c */ > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > > index 06116d16887..e4bdac087ad 100644 > > --- a/gcc/cp/pt.c > > +++ b/gcc/cp/pt.c > > @@ -12928,7 +12928,9 @@ extract_local_specs (tree pattern, tsubst_flags_t > > complain) > > tree > > build_extra_args (tree pattern, tree args, tsubst_flags_t complain) > > { > > - tree extra = args; > > + /* Make a copy of the extra arguments so that they won't get changed > > + from under us. */ > > + tree extra = copy_template_args (args); > > if (local_specializations) > > if (tree locals = extract_local_specs (pattern, complain)) > > extra = tree_cons (NULL_TREE, extra, locals); > > @@ -12939,7 +12941,7 @@ build_extra_args (tree pattern, tree args, > > tsubst_flags_t complain) > > normal template args to ARGS. */ > > tree > > -add_extra_args (tree extra, tree args) > > +add_extra_args (tree extra, tree args, tsubst_flags_t complain, tree > > in_decl) > > { > > if (extra && TREE_CODE (extra) == TREE_LIST) > > { > > @@ -12959,20 +12961,14 @@ add_extra_args (tree extra, tree args) > > gcc_assert (!TREE_PURPOSE (extra)); > > extra = TREE_VALUE (extra); > > } > > -#if 1 > > - /* I think we should always be able to substitute dependent args into the > > - pattern. If that turns out to be incorrect in some cases, enable the > > - alternate code (and add complain/in_decl parms to this function). */ > > Ah, because these cases aren't pack expansions, so we aren't trying to do the > substitution; I wonder if it would be feasible to do so. But this approach is > probably simpler. OK. For constexpr if, it seems feasible/safe to do the substitution by setting tf_partial when the arguments are dependent. But for requires-exprs, if one of the arguments is non-dependent then doing the substitution could cause us to evaluate the requirements out of order. So this current approach might be better for requires-exprs since it avoids out-of-order evaluation.
diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index 99d3ccc6998..4ee5215df50 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -2266,7 +2266,8 @@ tsubst_requires_expr (tree t, tree args, sat_info info) /* A requires-expression is an unevaluated context. */ cp_unevaluated u; - args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args); + args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args, + info.complain, info.in_decl); if (processing_template_decl) { /* We're partially instantiating a generic lambda. Substituting into diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 58da7460001..0a5f13489cc 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7289,7 +7289,7 @@ extern void add_mergeable_specialization (bool is_decl, bool is_alias, tree outer, unsigned); extern tree add_to_template_args (tree, tree); extern tree add_outermost_template_args (tree, tree); -extern tree add_extra_args (tree, tree); +extern tree add_extra_args (tree, tree, tsubst_flags_t, tree); extern tree build_extra_args (tree, tree, tsubst_flags_t); /* in rtti.c */ diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 06116d16887..e4bdac087ad 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -12928,7 +12928,9 @@ extract_local_specs (tree pattern, tsubst_flags_t complain) tree build_extra_args (tree pattern, tree args, tsubst_flags_t complain) { - tree extra = args; + /* Make a copy of the extra arguments so that they won't get changed + from under us. */ + tree extra = copy_template_args (args); if (local_specializations) if (tree locals = extract_local_specs (pattern, complain)) extra = tree_cons (NULL_TREE, extra, locals); @@ -12939,7 +12941,7 @@ build_extra_args (tree pattern, tree args, tsubst_flags_t complain) normal template args to ARGS. */ tree -add_extra_args (tree extra, tree args) +add_extra_args (tree extra, tree args, tsubst_flags_t complain, tree in_decl) { if (extra && TREE_CODE (extra) == TREE_LIST) { @@ -12959,20 +12961,14 @@ add_extra_args (tree extra, tree args) gcc_assert (!TREE_PURPOSE (extra)); extra = TREE_VALUE (extra); } -#if 1 - /* I think we should always be able to substitute dependent args into the - pattern. If that turns out to be incorrect in some cases, enable the - alternate code (and add complain/in_decl parms to this function). */ - gcc_checking_assert (!uses_template_parms (extra)); -#else - if (!uses_template_parms (extra)) + if (uses_template_parms (extra)) { - gcc_unreachable (); + /* This can happen during dependent substitution into a requires-expr + or a lambda that uses constexpr if. */ extra = tsubst_template_args (extra, args, complain, in_decl); args = add_outermost_template_args (args, extra); } else -#endif args = add_to_template_args (extra, args); return args; } @@ -12998,7 +12994,7 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain, pattern = PACK_EXPANSION_PATTERN (t); /* Add in any args remembered from an earlier partial instantiation. */ - args = add_extra_args (PACK_EXPANSION_EXTRA_ARGS (t), args); + args = add_extra_args (PACK_EXPANSION_EXTRA_ARGS (t), args, complain, in_decl); levels = TMPL_ARGS_DEPTH (args); @@ -13370,7 +13366,9 @@ tsubst_template_args (tree t, tree args, tsubst_flags_t complain, tree in_decl) tree orig_arg = TREE_VEC_ELT (t, i); tree new_arg; - if (TREE_CODE (orig_arg) == TREE_VEC) + if (!orig_arg) + new_arg = NULL_TREE; + else if (TREE_CODE (orig_arg) == TREE_VEC) new_arg = tsubst_template_args (orig_arg, args, complain, in_decl); else if (PACK_EXPANSION_P (orig_arg)) { @@ -13420,8 +13418,9 @@ tsubst_template_args (tree t, tree args, tsubst_flags_t complain, tree in_decl) } for (i = 0, out = 0; i < len; i++) { - if ((PACK_EXPANSION_P (TREE_VEC_ELT (orig_t, i)) - || ARGUMENT_PACK_P (TREE_VEC_ELT (orig_t, i))) + tree orig_arg = TREE_VEC_ELT (orig_t, i); + if (orig_arg + && (PACK_EXPANSION_P (orig_arg) || ARGUMENT_PACK_P (orig_arg)) && TREE_CODE (elts[i]) == TREE_VEC) { int idx; @@ -18449,7 +18448,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl, IF_STMT_CONSTEXPR_P (stmt) = IF_STMT_CONSTEXPR_P (t); IF_STMT_CONSTEVAL_P (stmt) = IF_STMT_CONSTEVAL_P (t); if (IF_STMT_CONSTEXPR_P (t)) - args = add_extra_args (IF_STMT_EXTRA_ARGS (t), args); + args = add_extra_args (IF_STMT_EXTRA_ARGS (t), args, complain, in_decl); tmp = RECUR (IF_COND (t)); tmp = finish_if_stmt_cond (tmp, stmt); if (IF_STMT_CONSTEXPR_P (t) diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-requires26.C b/gcc/testsuite/g++.dg/cpp2a/concepts-requires26.C new file mode 100644 index 00000000000..824d23431f0 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-requires26.C @@ -0,0 +1,18 @@ +// PR c++/101181 +// { dg-do compile { target c++20 } } + +template<class T, + bool = requires { typename T::type; }> +struct p { using type = void; }; + +template<class T> +struct p<T, true> { using type = typename T::type; }; + +template<class T> using P = typename p<T>::type; + +using type1 = P<int>; +using type1 = void; + +struct A { using type = char; }; +using type2 = P<A>; +using type2 = char; diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C b/gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C new file mode 100644 index 00000000000..1113c6f02b9 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/lambda-uneval16.C @@ -0,0 +1,22 @@ +// PR c++/101181 +// { dg-do compile { target c++20 } } + +template<class T, + bool = [] () -> bool { + if constexpr (requires { typename T::type; }) + return true; + return false; + }()> +struct p { using type = void; }; + +template<class T> +struct p<T, true> { using type = typename T::type; }; + +template<class T> using P = typename p<T>::type; + +using type1 = P<int>; +using type = void; + +struct A { using type = char; }; +using type2 = P<A>; +using type2 = char;