| Message ID | 20200810161432.1779298-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: premature analysis of requires-expression [PR96410] | expand |
On Mon, 10 Aug 2020, Patrick Palka wrote: > In the below testcase, semantic analysis of the requires-expressions in > the generic lambda must be delayed until instantiation of the lambda > because the requirements depend on the lambda's template arguments. But > tsubst_requires_expr does semantic analysis even during regeneration of > the lambda, which leads to various bogus errors and ICEs since some > subroutines aren't prepared to handle dependent/template trees. > > This patch adjusts subroutines of tsubst_requires_expr to avoid doing > some problematic semantic analyses when processing_template_decl. > In particular, expr_noexcept_p generally can't be checked on a dependent > expression. Next, tsubst_nested_requirement should avoid checking > satisfaction when processing_template_decl. And similarly for > convert_to_void (called from tsubst_valid_expression_requirement). > > Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested > against the cmcstl2 project. Does this look OK to commit? > > gcc/cp/ChangeLog: > > PR c++/96409 > PR c++/96410 > * constraint.cc (tsubst_compound_requirement): When > processing_template_decl, don't check noexcept of the > substituted expression. > (tsubst_nested_requirement): Just substitute into the constraint > when processing_template_decl. > * cvt.c (convert_to_void): Don't resolve concept checks when > processing_template_decl. > > gcc/testsuite/ChangeLog: > > PR c++/96409 > PR c++/96410 > * g++.dg/cpp2a/concepts-lambda13.C: New test. > --- > gcc/cp/constraint.cc | 9 ++++++- > gcc/cp/cvt.c | 2 +- > .../g++.dg/cpp2a/concepts-lambda13.C | 25 +++++++++++++++++++ > 3 files changed, 34 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > index e4aace596e7..db2036502a7 100644 > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args, subst_info info) > > /* Check the noexcept condition. */ > bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t); > - if (noexcept_p && !expr_noexcept_p (expr, tf_none)) > + if (!processing_template_decl > + && noexcept_p && !expr_noexcept_p (expr, tf_none)) > return error_mark_node; > > /* Substitute through the type expression, if any. */ > @@ -2023,6 +2024,12 @@ static tree > tsubst_nested_requirement (tree t, tree args, subst_info info) > { > /* Ensure that we're in an evaluation context prior to satisfaction. */ > + if (processing_template_decl) > + { > + tree r = tsubst_constraint (TREE_OPERAND (t, 0), args, > + info.complain, info.in_decl); Oops, the patch is missing a check for error_mark_node here, so that upon substitution failure we immediately resolve the requires-expression to false. Here's an updated patch with the check and a regression test added: -- >8 -- gcc/cp/ChangeLog: PR c++/96409 PR c++/96410 * constraint.cc (tsubst_compound_requirement): When processing_template_decl, don't check noexcept of the substituted expression. (tsubst_nested_requirement): Just substitute into the constraint when processing_template_decl. * cvt.c (convert_to_void): Don't resolve concept checks when processing_template_decl. gcc/testsuite/ChangeLog: PR c++/96409 PR c++/96410 * g++.dg/cpp2a/concepts-lambda13.C: New test. * g++.dg/cpp2a/concepts-lambda14.C: New test. --- gcc/cp/constraint.cc | 11 +++++++- gcc/cp/cvt.c | 2 +- .../g++.dg/cpp2a/concepts-lambda13.C | 25 +++++++++++++++++++ .../g++.dg/cpp2a/concepts-lambda14.C | 10 ++++++++ 4 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index e4aace596e7..5b4964dd36e 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args, subst_info info) /* Check the noexcept condition. */ bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t); - if (noexcept_p && !expr_noexcept_p (expr, tf_none)) + if (!processing_template_decl + && noexcept_p && !expr_noexcept_p (expr, tf_none)) return error_mark_node; /* Substitute through the type expression, if any. */ @@ -2023,6 +2024,14 @@ static tree tsubst_nested_requirement (tree t, tree args, subst_info info) { /* Ensure that we're in an evaluation context prior to satisfaction. */ + if (processing_template_decl) + { + tree r = tsubst_constraint (TREE_OPERAND (t, 0), args, + info.complain, info.in_decl); + if (r == error_mark_node) + return error_mark_node; + return finish_nested_requirement (EXPR_LOCATION (t), r); + } tree norm = TREE_TYPE (t); tree result = satisfy_constraint (norm, args, info); if (result == error_mark_node && info.quiet ()) diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c index c9e7b1ff044..1d2c2d3351c 100644 --- a/gcc/cp/cvt.c +++ b/gcc/cp/cvt.c @@ -1159,7 +1159,7 @@ convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain) /* Explicitly evaluate void-converted concept checks since their satisfaction may produce ill-formed programs. */ - if (concept_check_p (expr)) + if (!processing_template_decl && concept_check_p (expr)) expr = evaluate_concept_check (expr, tf_warning_or_error); if (VOID_TYPE_P (TREE_TYPE (expr))) diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C new file mode 100644 index 00000000000..9757ce49d67 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C @@ -0,0 +1,25 @@ +// PR c++/96410 +// { dg-do compile { target c++20 } } + +struct S { using blah = void; }; + +template <typename T> constexpr bool trait = !__is_same(T, S); +template <typename T> concept C = trait<T>; + +template<typename T> +void foo() noexcept(!__is_same(T, void)) { } + +template<typename U> +auto f() { + return []<typename T>(T){ + static_assert(requires { requires C<U> && (C<T> || C<T>); }); // { dg-error "assert" } + static_assert(requires { C<T>; }); + static_assert(requires { { foo<T>() } noexcept -> C; }); + static_assert(!requires { typename T::blah; }); // { dg-error "assert" } + return 0; + }; +} + +auto g = f<int>(); // { dg-bogus "" } +int n = g(0); // { dg-bogus "" } +int m = g(S{}); // { dg-message "required from here" } diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C new file mode 100644 index 00000000000..e6ae73c4872 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C @@ -0,0 +1,10 @@ +// { dg-do compile { target c++20 } } + +template<typename T> +auto f() { + return [](auto){ + static_assert(requires { requires T::fail; }); // { dg-error "assert" } + }; +} + +auto g = f<int>(); // { dg-message "required from here" }
On 8/10/20 2:18 PM, Patrick Palka wrote: > On Mon, 10 Aug 2020, Patrick Palka wrote: > >> In the below testcase, semantic analysis of the requires-expressions in >> the generic lambda must be delayed until instantiation of the lambda >> because the requirements depend on the lambda's template arguments. But >> tsubst_requires_expr does semantic analysis even during regeneration of >> the lambda, which leads to various bogus errors and ICEs since some >> subroutines aren't prepared to handle dependent/template trees. >> >> This patch adjusts subroutines of tsubst_requires_expr to avoid doing >> some problematic semantic analyses when processing_template_decl. >> In particular, expr_noexcept_p generally can't be checked on a dependent >> expression. Next, tsubst_nested_requirement should avoid checking >> satisfaction when processing_template_decl. And similarly for >> convert_to_void (called from tsubst_valid_expression_requirement). I wonder if, instead of trying to do a partial substitution into a requires-expression at all, we want to use the PACK_EXPANSION_EXTRA_ARGS/IF_STMT_EXTRA_ARGS mechanism to remember the arguments for later satisfaction? Jason >> Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested >> against the cmcstl2 project. Does this look OK to commit? >> >> gcc/cp/ChangeLog: >> >> PR c++/96409 >> PR c++/96410 >> * constraint.cc (tsubst_compound_requirement): When >> processing_template_decl, don't check noexcept of the >> substituted expression. >> (tsubst_nested_requirement): Just substitute into the constraint >> when processing_template_decl. >> * cvt.c (convert_to_void): Don't resolve concept checks when >> processing_template_decl. >> >> gcc/testsuite/ChangeLog: >> >> PR c++/96409 >> PR c++/96410 >> * g++.dg/cpp2a/concepts-lambda13.C: New test. >> --- >> gcc/cp/constraint.cc | 9 ++++++- >> gcc/cp/cvt.c | 2 +- >> .../g++.dg/cpp2a/concepts-lambda13.C | 25 +++++++++++++++++++ >> 3 files changed, 34 insertions(+), 2 deletions(-) >> create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C >> >> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc >> index e4aace596e7..db2036502a7 100644 >> --- a/gcc/cp/constraint.cc >> +++ b/gcc/cp/constraint.cc >> @@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args, subst_info info) >> >> /* Check the noexcept condition. */ >> bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t); >> - if (noexcept_p && !expr_noexcept_p (expr, tf_none)) >> + if (!processing_template_decl >> + && noexcept_p && !expr_noexcept_p (expr, tf_none)) >> return error_mark_node; >> >> /* Substitute through the type expression, if any. */ >> @@ -2023,6 +2024,12 @@ static tree >> tsubst_nested_requirement (tree t, tree args, subst_info info) >> { >> /* Ensure that we're in an evaluation context prior to satisfaction. */ >> + if (processing_template_decl) >> + { >> + tree r = tsubst_constraint (TREE_OPERAND (t, 0), args, >> + info.complain, info.in_decl); > > Oops, the patch is missing a check for error_mark_node here, so that > upon substitution failure we immediately resolve the requires-expression > to false. Here's an updated patch with the check and a regression test > added: > > -- >8 -- > > gcc/cp/ChangeLog: > > PR c++/96409 > PR c++/96410 > * constraint.cc (tsubst_compound_requirement): When > processing_template_decl, don't check noexcept of the > substituted expression. > (tsubst_nested_requirement): Just substitute into the constraint > when processing_template_decl. > * cvt.c (convert_to_void): Don't resolve concept checks when > processing_template_decl. > > gcc/testsuite/ChangeLog: > > PR c++/96409 > PR c++/96410 > * g++.dg/cpp2a/concepts-lambda13.C: New test. > * g++.dg/cpp2a/concepts-lambda14.C: New test. > --- > gcc/cp/constraint.cc | 11 +++++++- > gcc/cp/cvt.c | 2 +- > .../g++.dg/cpp2a/concepts-lambda13.C | 25 +++++++++++++++++++ > .../g++.dg/cpp2a/concepts-lambda14.C | 10 ++++++++ > 4 files changed, 46 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > index e4aace596e7..5b4964dd36e 100644 > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args, subst_info info) > > /* Check the noexcept condition. */ > bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t); > - if (noexcept_p && !expr_noexcept_p (expr, tf_none)) > + if (!processing_template_decl > + && noexcept_p && !expr_noexcept_p (expr, tf_none)) > return error_mark_node; > > /* Substitute through the type expression, if any. */ > @@ -2023,6 +2024,14 @@ static tree > tsubst_nested_requirement (tree t, tree args, subst_info info) > { > /* Ensure that we're in an evaluation context prior to satisfaction. */ > + if (processing_template_decl) > + { > + tree r = tsubst_constraint (TREE_OPERAND (t, 0), args, > + info.complain, info.in_decl); > + if (r == error_mark_node) > + return error_mark_node; > + return finish_nested_requirement (EXPR_LOCATION (t), r); > + } > tree norm = TREE_TYPE (t); > tree result = satisfy_constraint (norm, args, info); > if (result == error_mark_node && info.quiet ()) > diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c > index c9e7b1ff044..1d2c2d3351c 100644 > --- a/gcc/cp/cvt.c > +++ b/gcc/cp/cvt.c > @@ -1159,7 +1159,7 @@ convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain) > > /* Explicitly evaluate void-converted concept checks since their > satisfaction may produce ill-formed programs. */ > - if (concept_check_p (expr)) > + if (!processing_template_decl && concept_check_p (expr)) > expr = evaluate_concept_check (expr, tf_warning_or_error); > > if (VOID_TYPE_P (TREE_TYPE (expr))) > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > new file mode 100644 > index 00000000000..9757ce49d67 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > @@ -0,0 +1,25 @@ > +// PR c++/96410 > +// { dg-do compile { target c++20 } } > + > +struct S { using blah = void; }; > + > +template <typename T> constexpr bool trait = !__is_same(T, S); > +template <typename T> concept C = trait<T>; > + > +template<typename T> > +void foo() noexcept(!__is_same(T, void)) { } > + > +template<typename U> > +auto f() { > + return []<typename T>(T){ > + static_assert(requires { requires C<U> && (C<T> || C<T>); }); // { dg-error "assert" } > + static_assert(requires { C<T>; }); > + static_assert(requires { { foo<T>() } noexcept -> C; }); > + static_assert(!requires { typename T::blah; }); // { dg-error "assert" } > + return 0; > + }; > +} > + > +auto g = f<int>(); // { dg-bogus "" } > +int n = g(0); // { dg-bogus "" } > +int m = g(S{}); // { dg-message "required from here" } > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C > new file mode 100644 > index 00000000000..e6ae73c4872 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C > @@ -0,0 +1,10 @@ > +// { dg-do compile { target c++20 } } > + > +template<typename T> > +auto f() { > + return [](auto){ > + static_assert(requires { requires T::fail; }); // { dg-error "assert" } > + }; > +} > + > +auto g = f<int>(); // { dg-message "required from here" } >
On Mon, 10 Aug 2020, Jason Merrill wrote: > On 8/10/20 2:18 PM, Patrick Palka wrote: > > On Mon, 10 Aug 2020, Patrick Palka wrote: > > > > > In the below testcase, semantic analysis of the requires-expressions in > > > the generic lambda must be delayed until instantiation of the lambda > > > because the requirements depend on the lambda's template arguments. But > > > tsubst_requires_expr does semantic analysis even during regeneration of > > > the lambda, which leads to various bogus errors and ICEs since some > > > subroutines aren't prepared to handle dependent/template trees. > > > > > > This patch adjusts subroutines of tsubst_requires_expr to avoid doing > > > some problematic semantic analyses when processing_template_decl. > > > In particular, expr_noexcept_p generally can't be checked on a dependent > > > expression. Next, tsubst_nested_requirement should avoid checking > > > satisfaction when processing_template_decl. And similarly for > > > convert_to_void (called from tsubst_valid_expression_requirement). > > I wonder if, instead of trying to do a partial substitution into a > requires-expression at all, we want to use the > PACK_EXPANSION_EXTRA_ARGS/IF_STMT_EXTRA_ARGS mechanism to remember the > arguments for later satisfaction? IIUC, avoiding partial substitution into a requires-expression would mean we'd go from currently accepting the following testcase to rejecting it, because we'd now instantiate B<int>::type as part of the first requirement before first noticing the SFINAE error in the second requirement (which depends only on the outer template argument, and which would determine the value of the requires-expression): template<typename T> struct B { using type = T::fatal; }; template<typename T> constexpr auto foo() { return [] <typename U> (U) { return requires { typename B<U>::type; typename T::type; }; }; }; int i = foo<int>()(0); I guess this is exactly the kind of testcase that motivates using the PACK_EXPANSION_EXTRA_ARGS/IF_STMT_EXTRA_ARGS mechanism for requires-expressions? > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested > > > against the cmcstl2 project. Does this look OK to commit? > > > > > > gcc/cp/ChangeLog: > > > > > > PR c++/96409 > > > PR c++/96410 > > > * constraint.cc (tsubst_compound_requirement): When > > > processing_template_decl, don't check noexcept of the > > > substituted expression. > > > (tsubst_nested_requirement): Just substitute into the constraint > > > when processing_template_decl. > > > * cvt.c (convert_to_void): Don't resolve concept checks when > > > processing_template_decl. > > > > > > gcc/testsuite/ChangeLog: > > > > > > PR c++/96409 > > > PR c++/96410 > > > * g++.dg/cpp2a/concepts-lambda13.C: New test. > > > --- > > > gcc/cp/constraint.cc | 9 ++++++- > > > gcc/cp/cvt.c | 2 +- > > > .../g++.dg/cpp2a/concepts-lambda13.C | 25 +++++++++++++++++++ > > > 3 files changed, 34 insertions(+), 2 deletions(-) > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > > > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > > index e4aace596e7..db2036502a7 100644 > > > --- a/gcc/cp/constraint.cc > > > +++ b/gcc/cp/constraint.cc > > > @@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args, > > > subst_info info) > > > /* Check the noexcept condition. */ > > > bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t); > > > - if (noexcept_p && !expr_noexcept_p (expr, tf_none)) > > > + if (!processing_template_decl > > > + && noexcept_p && !expr_noexcept_p (expr, tf_none)) > > > return error_mark_node; > > > /* Substitute through the type expression, if any. */ > > > @@ -2023,6 +2024,12 @@ static tree > > > tsubst_nested_requirement (tree t, tree args, subst_info info) > > > { > > > /* Ensure that we're in an evaluation context prior to satisfaction. > > > */ > > > + if (processing_template_decl) > > > + { > > > + tree r = tsubst_constraint (TREE_OPERAND (t, 0), args, > > > + info.complain, info.in_decl); > > > > Oops, the patch is missing a check for error_mark_node here, so that > > upon substitution failure we immediately resolve the requires-expression > > to false. Here's an updated patch with the check and a regression test > > added: > > > > -- >8 -- > > > > gcc/cp/ChangeLog: > > > > PR c++/96409 > > PR c++/96410 > > * constraint.cc (tsubst_compound_requirement): When > > processing_template_decl, don't check noexcept of the > > substituted expression. > > (tsubst_nested_requirement): Just substitute into the constraint > > when processing_template_decl. > > * cvt.c (convert_to_void): Don't resolve concept checks when > > processing_template_decl. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/96409 > > PR c++/96410 > > * g++.dg/cpp2a/concepts-lambda13.C: New test. > > * g++.dg/cpp2a/concepts-lambda14.C: New test. > > --- > > gcc/cp/constraint.cc | 11 +++++++- > > gcc/cp/cvt.c | 2 +- > > .../g++.dg/cpp2a/concepts-lambda13.C | 25 +++++++++++++++++++ > > .../g++.dg/cpp2a/concepts-lambda14.C | 10 ++++++++ > > 4 files changed, 46 insertions(+), 2 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > index e4aace596e7..5b4964dd36e 100644 > > --- a/gcc/cp/constraint.cc > > +++ b/gcc/cp/constraint.cc > > @@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args, > > subst_info info) > > /* Check the noexcept condition. */ > > bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t); > > - if (noexcept_p && !expr_noexcept_p (expr, tf_none)) > > + if (!processing_template_decl > > + && noexcept_p && !expr_noexcept_p (expr, tf_none)) > > return error_mark_node; > > /* Substitute through the type expression, if any. */ > > @@ -2023,6 +2024,14 @@ static tree > > tsubst_nested_requirement (tree t, tree args, subst_info info) > > { > > /* Ensure that we're in an evaluation context prior to satisfaction. */ > > + if (processing_template_decl) > > + { > > + tree r = tsubst_constraint (TREE_OPERAND (t, 0), args, > > + info.complain, info.in_decl); > > + if (r == error_mark_node) > > + return error_mark_node; > > + return finish_nested_requirement (EXPR_LOCATION (t), r); > > + } > > tree norm = TREE_TYPE (t); > > tree result = satisfy_constraint (norm, args, info); > > if (result == error_mark_node && info.quiet ()) > > diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c > > index c9e7b1ff044..1d2c2d3351c 100644 > > --- a/gcc/cp/cvt.c > > +++ b/gcc/cp/cvt.c > > @@ -1159,7 +1159,7 @@ convert_to_void (tree expr, impl_conv_void implicit, > > tsubst_flags_t complain) > > /* Explicitly evaluate void-converted concept checks since their > > satisfaction may produce ill-formed programs. */ > > - if (concept_check_p (expr)) > > + if (!processing_template_decl && concept_check_p (expr)) > > expr = evaluate_concept_check (expr, tf_warning_or_error); > > if (VOID_TYPE_P (TREE_TYPE (expr))) > > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > > b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > > new file mode 100644 > > index 00000000000..9757ce49d67 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > > @@ -0,0 +1,25 @@ > > +// PR c++/96410 > > +// { dg-do compile { target c++20 } } > > + > > +struct S { using blah = void; }; > > + > > +template <typename T> constexpr bool trait = !__is_same(T, S); > > +template <typename T> concept C = trait<T>; > > + > > +template<typename T> > > +void foo() noexcept(!__is_same(T, void)) { } > > + > > +template<typename U> > > +auto f() { > > + return []<typename T>(T){ > > + static_assert(requires { requires C<U> && (C<T> || C<T>); }); // { > > dg-error "assert" } > > + static_assert(requires { C<T>; }); > > + static_assert(requires { { foo<T>() } noexcept -> C; }); > > + static_assert(!requires { typename T::blah; }); // { dg-error "assert" > > } > > + return 0; > > + }; > > +} > > + > > +auto g = f<int>(); // { dg-bogus "" } > > +int n = g(0); // { dg-bogus "" } > > +int m = g(S{}); // { dg-message "required from here" } > > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C > > b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C > > new file mode 100644 > > index 00000000000..e6ae73c4872 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C > > @@ -0,0 +1,10 @@ > > +// { dg-do compile { target c++20 } } > > + > > +template<typename T> > > +auto f() { > > + return [](auto){ > > + static_assert(requires { requires T::fail; }); // { dg-error "assert" } > > + }; > > +} > > + > > +auto g = f<int>(); // { dg-message "required from here" } > > > >
On 8/13/20 11:21 AM, Patrick Palka wrote: > On Mon, 10 Aug 2020, Jason Merrill wrote: > >> On 8/10/20 2:18 PM, Patrick Palka wrote: >>> On Mon, 10 Aug 2020, Patrick Palka wrote: >>> >>>> In the below testcase, semantic analysis of the requires-expressions in >>>> the generic lambda must be delayed until instantiation of the lambda >>>> because the requirements depend on the lambda's template arguments. But >>>> tsubst_requires_expr does semantic analysis even during regeneration of >>>> the lambda, which leads to various bogus errors and ICEs since some >>>> subroutines aren't prepared to handle dependent/template trees. >>>> >>>> This patch adjusts subroutines of tsubst_requires_expr to avoid doing >>>> some problematic semantic analyses when processing_template_decl. >>>> In particular, expr_noexcept_p generally can't be checked on a dependent >>>> expression. Next, tsubst_nested_requirement should avoid checking >>>> satisfaction when processing_template_decl. And similarly for >>>> convert_to_void (called from tsubst_valid_expression_requirement). >> >> I wonder if, instead of trying to do a partial substitution into a >> requires-expression at all, we want to use the >> PACK_EXPANSION_EXTRA_ARGS/IF_STMT_EXTRA_ARGS mechanism to remember the >> arguments for later satisfaction? > > IIUC, avoiding partial substitution into a requires-expression would > mean we'd go from currently accepting the following testcase to > rejecting it, because we'd now instantiate B<int>::type as part of the > first requirement before first noticing the SFINAE error in the second > requirement (which depends only on the outer template argument, and > which would determine the value of the requires-expression): > > template<typename T> > struct B { using type = T::fatal; }; > > template<typename T> > constexpr auto foo() { > return [] <typename U> (U) { > return requires { typename B<U>::type; typename T::type; }; > }; > }; > > int i = foo<int>()(0); > > I guess this is exactly the kind of testcase that motivates using the > PACK_EXPANSION_EXTRA_ARGS/IF_STMT_EXTRA_ARGS mechanism for > requires-expressions? I think so, yes. >> >>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested >>>> against the cmcstl2 project. Does this look OK to commit? >>>> >>>> gcc/cp/ChangeLog: >>>> >>>> PR c++/96409 >>>> PR c++/96410 >>>> * constraint.cc (tsubst_compound_requirement): When >>>> processing_template_decl, don't check noexcept of the >>>> substituted expression. >>>> (tsubst_nested_requirement): Just substitute into the constraint >>>> when processing_template_decl. >>>> * cvt.c (convert_to_void): Don't resolve concept checks when >>>> processing_template_decl. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> PR c++/96409 >>>> PR c++/96410 >>>> * g++.dg/cpp2a/concepts-lambda13.C: New test. >>>> --- >>>> gcc/cp/constraint.cc | 9 ++++++- >>>> gcc/cp/cvt.c | 2 +- >>>> .../g++.dg/cpp2a/concepts-lambda13.C | 25 +++++++++++++++++++ >>>> 3 files changed, 34 insertions(+), 2 deletions(-) >>>> create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C >>>> >>>> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc >>>> index e4aace596e7..db2036502a7 100644 >>>> --- a/gcc/cp/constraint.cc >>>> +++ b/gcc/cp/constraint.cc >>>> @@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args, >>>> subst_info info) >>>> /* Check the noexcept condition. */ >>>> bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t); >>>> - if (noexcept_p && !expr_noexcept_p (expr, tf_none)) >>>> + if (!processing_template_decl >>>> + && noexcept_p && !expr_noexcept_p (expr, tf_none)) >>>> return error_mark_node; >>>> /* Substitute through the type expression, if any. */ >>>> @@ -2023,6 +2024,12 @@ static tree >>>> tsubst_nested_requirement (tree t, tree args, subst_info info) >>>> { >>>> /* Ensure that we're in an evaluation context prior to satisfaction. >>>> */ >>>> + if (processing_template_decl) >>>> + { >>>> + tree r = tsubst_constraint (TREE_OPERAND (t, 0), args, >>>> + info.complain, info.in_decl); >>> >>> Oops, the patch is missing a check for error_mark_node here, so that >>> upon substitution failure we immediately resolve the requires-expression >>> to false. Here's an updated patch with the check and a regression test >>> added: >>> >>> -- >8 -- >>> >>> gcc/cp/ChangeLog: >>> >>> PR c++/96409 >>> PR c++/96410 >>> * constraint.cc (tsubst_compound_requirement): When >>> processing_template_decl, don't check noexcept of the >>> substituted expression. >>> (tsubst_nested_requirement): Just substitute into the constraint >>> when processing_template_decl. >>> * cvt.c (convert_to_void): Don't resolve concept checks when >>> processing_template_decl. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR c++/96409 >>> PR c++/96410 >>> * g++.dg/cpp2a/concepts-lambda13.C: New test. >>> * g++.dg/cpp2a/concepts-lambda14.C: New test. >>> --- >>> gcc/cp/constraint.cc | 11 +++++++- >>> gcc/cp/cvt.c | 2 +- >>> .../g++.dg/cpp2a/concepts-lambda13.C | 25 +++++++++++++++++++ >>> .../g++.dg/cpp2a/concepts-lambda14.C | 10 ++++++++ >>> 4 files changed, 46 insertions(+), 2 deletions(-) >>> create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C >>> create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C >>> >>> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc >>> index e4aace596e7..5b4964dd36e 100644 >>> --- a/gcc/cp/constraint.cc >>> +++ b/gcc/cp/constraint.cc >>> @@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args, >>> subst_info info) >>> /* Check the noexcept condition. */ >>> bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t); >>> - if (noexcept_p && !expr_noexcept_p (expr, tf_none)) >>> + if (!processing_template_decl >>> + && noexcept_p && !expr_noexcept_p (expr, tf_none)) >>> return error_mark_node; >>> /* Substitute through the type expression, if any. */ >>> @@ -2023,6 +2024,14 @@ static tree >>> tsubst_nested_requirement (tree t, tree args, subst_info info) >>> { >>> /* Ensure that we're in an evaluation context prior to satisfaction. */ >>> + if (processing_template_decl) >>> + { >>> + tree r = tsubst_constraint (TREE_OPERAND (t, 0), args, >>> + info.complain, info.in_decl); >>> + if (r == error_mark_node) >>> + return error_mark_node; >>> + return finish_nested_requirement (EXPR_LOCATION (t), r); >>> + } >>> tree norm = TREE_TYPE (t); >>> tree result = satisfy_constraint (norm, args, info); >>> if (result == error_mark_node && info.quiet ()) >>> diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c >>> index c9e7b1ff044..1d2c2d3351c 100644 >>> --- a/gcc/cp/cvt.c >>> +++ b/gcc/cp/cvt.c >>> @@ -1159,7 +1159,7 @@ convert_to_void (tree expr, impl_conv_void implicit, >>> tsubst_flags_t complain) >>> /* Explicitly evaluate void-converted concept checks since their >>> satisfaction may produce ill-formed programs. */ >>> - if (concept_check_p (expr)) >>> + if (!processing_template_decl && concept_check_p (expr)) >>> expr = evaluate_concept_check (expr, tf_warning_or_error); >>> if (VOID_TYPE_P (TREE_TYPE (expr))) >>> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C >>> b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C >>> new file mode 100644 >>> index 00000000000..9757ce49d67 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C >>> @@ -0,0 +1,25 @@ >>> +// PR c++/96410 >>> +// { dg-do compile { target c++20 } } >>> + >>> +struct S { using blah = void; }; >>> + >>> +template <typename T> constexpr bool trait = !__is_same(T, S); >>> +template <typename T> concept C = trait<T>; >>> + >>> +template<typename T> >>> +void foo() noexcept(!__is_same(T, void)) { } >>> + >>> +template<typename U> >>> +auto f() { >>> + return []<typename T>(T){ >>> + static_assert(requires { requires C<U> && (C<T> || C<T>); }); // { >>> dg-error "assert" } >>> + static_assert(requires { C<T>; }); >>> + static_assert(requires { { foo<T>() } noexcept -> C; }); >>> + static_assert(!requires { typename T::blah; }); // { dg-error "assert" >>> } >>> + return 0; >>> + }; >>> +} >>> + >>> +auto g = f<int>(); // { dg-bogus "" } >>> +int n = g(0); // { dg-bogus "" } >>> +int m = g(S{}); // { dg-message "required from here" } >>> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C >>> b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C >>> new file mode 100644 >>> index 00000000000..e6ae73c4872 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C >>> @@ -0,0 +1,10 @@ >>> +// { dg-do compile { target c++20 } } >>> + >>> +template<typename T> >>> +auto f() { >>> + return [](auto){ >>> + static_assert(requires { requires T::fail; }); // { dg-error "assert" } >>> + }; >>> +} >>> + >>> +auto g = f<int>(); // { dg-message "required from here" } >>> >> >> >
On Thu, 13 Aug 2020, Jason Merrill wrote: > On 8/13/20 11:21 AM, Patrick Palka wrote: > > On Mon, 10 Aug 2020, Jason Merrill wrote: > > > > > On 8/10/20 2:18 PM, Patrick Palka wrote: > > > > On Mon, 10 Aug 2020, Patrick Palka wrote: > > > > > > > > > In the below testcase, semantic analysis of the requires-expressions > > > > > in > > > > > the generic lambda must be delayed until instantiation of the lambda > > > > > because the requirements depend on the lambda's template arguments. > > > > > But > > > > > tsubst_requires_expr does semantic analysis even during regeneration > > > > > of > > > > > the lambda, which leads to various bogus errors and ICEs since some > > > > > subroutines aren't prepared to handle dependent/template trees. > > > > > > > > > > This patch adjusts subroutines of tsubst_requires_expr to avoid doing > > > > > some problematic semantic analyses when processing_template_decl. > > > > > In particular, expr_noexcept_p generally can't be checked on a > > > > > dependent > > > > > expression. Next, tsubst_nested_requirement should avoid checking > > > > > satisfaction when processing_template_decl. And similarly for > > > > > convert_to_void (called from tsubst_valid_expression_requirement). > > > > > > I wonder if, instead of trying to do a partial substitution into a > > > requires-expression at all, we want to use the > > > PACK_EXPANSION_EXTRA_ARGS/IF_STMT_EXTRA_ARGS mechanism to remember the > > > arguments for later satisfaction? Like this? -- >8 -- Subject: [PATCH] c++: requires-expressions and partial instantiation [PR96410] This patch makes tsubst_requires_expr avoid substituting into a requires-expression when partially instantiating a generic lambda. This is necessary in general to ensure that we check its requirements in lexical order (see the first testcase below). Instead, template arguments are saved in the requires-expression until all arguments can be substituted into it at once, using a mechanism similar to PACK_EXPANSION_EXTRA_ARGS. This change incidentally also fixes the two mentioned PRs -- the problem there is that tsubst_requires_expr was performing semantic checks on templated trees, and some of the checks are not prepared to handle such trees. With this patch tsubst_requires_expr, no longer does any semantic checking at all when processing_template_decl. gcc/cp/ChangeLog: PR c++/96409 PR c++/96410 * constraint.cc (tsubst_requires_expr): Use REQUIRES_EXPR_PARMS and REQUIRES_EXPR_REQS. Use REQUIRES_EXPR_EXTRA_ARGS and add_extra_args to defer substitution until we have all the template arguments. (finish_requires_expr): Adjust the call to build_min so that REQUIRES_EXPR_EXTRA_ARGS gets set to NULL_TREE. * cp-tree.def (REQUIRES_EXPR): Give it a third operand. * cp-tree.h (REQUIRES_EXPR_PARMS, REQUIRES_EXPR_REQS, REQUIRES_EXPR_EXTRA_ARGS): Define. (add_extra_args): Declare. gcc/testsuite/ChangeLog: PR c++/96409 PR c++/96410 * g++.dg/cpp2a/concepts-lambda13.C: New test. * g++.dg/cpp2a/concepts-lambda14.C: New test. --- gcc/cp/constraint.cc | 21 +++++++++++----- gcc/cp/cp-tree.def | 8 +++--- gcc/cp/cp-tree.h | 16 ++++++++++++ .../g++.dg/cpp2a/concepts-lambda13.C | 18 +++++++++++++ .../g++.dg/cpp2a/concepts-lambda14.C | 25 +++++++++++++++++++ 5 files changed, 79 insertions(+), 9 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index ad9d47070e3..9a06d763554 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -2175,7 +2175,19 @@ tsubst_requires_expr (tree t, tree args, /* A requires-expression is an unevaluated context. */ cp_unevaluated u; - tree parms = TREE_OPERAND (t, 0); + args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args); + if (processing_template_decl) + { + /* We're partially instantiating a generic lambda. Substituting into + this requires-expression now may cause its requirements to get + checked out of order, so instead just remember the template + arguments and wait until we can substitute them all at once. */ + t = copy_node (t); + REQUIRES_EXPR_EXTRA_ARGS (t) = args; + return t; + } + + tree parms = REQUIRES_EXPR_PARMS (t); if (parms) { parms = tsubst_constraint_variables (parms, args, info); @@ -2183,14 +2195,11 @@ tsubst_requires_expr (tree t, tree args, return boolean_false_node; } - tree reqs = TREE_OPERAND (t, 1); + tree reqs = REQUIRES_EXPR_REQS (t); reqs = tsubst_requirement_body (reqs, args, info); if (reqs == error_mark_node) return boolean_false_node; - if (processing_template_decl) - return finish_requires_expr (cp_expr_location (t), parms, reqs); - return boolean_true_node; } @@ -2933,7 +2942,7 @@ finish_requires_expr (location_t loc, tree parms, tree reqs) } /* Build the node. */ - tree r = build_min (REQUIRES_EXPR, boolean_type_node, parms, reqs); + tree r = build_min (REQUIRES_EXPR, boolean_type_node, parms, reqs, NULL_TREE); TREE_SIDE_EFFECTS (r) = false; TREE_CONSTANT (r) = true; SET_EXPR_LOCATION (r, loc); diff --git a/gcc/cp/cp-tree.def b/gcc/cp/cp-tree.def index 31be2cf41a3..6eabe0d6d8f 100644 --- a/gcc/cp/cp-tree.def +++ b/gcc/cp/cp-tree.def @@ -524,11 +524,13 @@ DEFTREECODE (CONSTRAINT_INFO, "constraint_info", tcc_exceptional, 0) of the wildcard. */ DEFTREECODE (WILDCARD_DECL, "wildcard_decl", tcc_declaration, 0) -/* A requires-expr is a binary expression. The first operand is +/* A requires-expr has three operands. The first operand is its parameter list (possibly NULL). The second is a list of requirements, which are denoted by the _REQ* tree codes - below. */ -DEFTREECODE (REQUIRES_EXPR, "requires_expr", tcc_expression, 2) + below. The third is a TREE_VEC of template arguments to + be applied when substituting into the parameter list and + requirements, set by tsubst_requires_expr for partial instantiations. */ +DEFTREECODE (REQUIRES_EXPR, "requires_expr", tcc_expression, 3) /* A requirement for an expression. */ DEFTREECODE (SIMPLE_REQ, "simple_req", tcc_expression, 1) diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 6e4de7d0c4b..9c36e96ead6 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -1618,6 +1618,21 @@ check_constraint_info (tree t) #define CONSTRAINED_PARM_PROTOTYPE(NODE) \ DECL_INITIAL (TYPE_DECL_CHECK (NODE)) +/* The list of local parameters introduced by this requires-expression, + in the form of a chain of PARM_DECLs. */ +#define REQUIRES_EXPR_PARMS(NODE) \ + TREE_OPERAND (TREE_CHECK (NODE, REQUIRES_EXPR), 0) + +/* A TREE_LIST of the requirements for this requires-expression. + The requirements are stored in lexical order within the TREE_VALUE + of each TREE_LIST node. The TREE_PURPOSE of each node is unused. */ +#define REQUIRES_EXPR_REQS(NODE) \ + TREE_OPERAND (TREE_CHECK (NODE, REQUIRES_EXPR), 1) + +/* Like PACK_EXPANSION_EXTRA_ARGS, for requires-expressions. */ +#define REQUIRES_EXPR_EXTRA_ARGS(NODE) \ + TREE_OPERAND (TREE_CHECK (NODE, REQUIRES_EXPR), 2) + enum cp_tree_node_structure_enum { TS_CP_GENERIC, TS_CP_IDENTIFIER, @@ -7013,6 +7028,7 @@ extern bool template_guide_p (const_tree); extern bool builtin_guide_p (const_tree); extern void store_explicit_specifier (tree, tree); extern tree add_outermost_template_args (tree, tree); +extern tree add_extra_args (tree, tree); /* in rtti.c */ /* A vector of all tinfo decls that haven't been emitted yet. */ diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C new file mode 100644 index 00000000000..d4bed30a900 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C @@ -0,0 +1,18 @@ +// { dg-do compile { target c++20 } } + +template<typename T> +struct S { + using type = T::type; // { dg-bogus "" } +}; + +template<typename T> +auto f() { + return [] <typename U> (U) { + // Verify that partial instantiation of this generic lambda doesn't cause + // these requirements to get checked out of order. + static_assert(!requires { typename U::type; typename S<T>::type; }); + return 0; + }; +} + +int a = f<int>()(0); diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C new file mode 100644 index 00000000000..bdc893da857 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C @@ -0,0 +1,25 @@ +// PR c++/96410 +// { dg-do compile { target c++20 } } + +struct S { using blah = void; }; + +template <typename T> constexpr bool trait = !__is_same(T, S); +template <typename T> concept C = trait<T>; + +template<typename T> +void foo() noexcept(!__is_same(T, void)) { } + +template<typename U> +auto f() { + return []<typename T>(T, bool a = requires { C<T>; }){ + static_assert(requires { requires C<U> && (C<T> || C<T>); }); // { dg-error "assert" } + static_assert(requires { C<T>; }); + static_assert(requires { { foo<T>() } noexcept -> C; }); + static_assert(!requires { typename T::blah; }); // { dg-error "assert" } + return 0; + }; +} + +auto g = f<int>(); // { dg-bogus "" } +int n = g(0); // { dg-bogus "" } +int m = g(S{}); // { dg-message "required from here" }
On 9/16/20 1:34 PM, Patrick Palka wrote: > On Thu, 13 Aug 2020, Jason Merrill wrote: > >> On 8/13/20 11:21 AM, Patrick Palka wrote: >>> On Mon, 10 Aug 2020, Jason Merrill wrote: >>> >>>> On 8/10/20 2:18 PM, Patrick Palka wrote: >>>>> On Mon, 10 Aug 2020, Patrick Palka wrote: >>>>> >>>>>> In the below testcase, semantic analysis of the requires-expressions >>>>>> in >>>>>> the generic lambda must be delayed until instantiation of the lambda >>>>>> because the requirements depend on the lambda's template arguments. >>>>>> But >>>>>> tsubst_requires_expr does semantic analysis even during regeneration >>>>>> of >>>>>> the lambda, which leads to various bogus errors and ICEs since some >>>>>> subroutines aren't prepared to handle dependent/template trees. >>>>>> >>>>>> This patch adjusts subroutines of tsubst_requires_expr to avoid doing >>>>>> some problematic semantic analyses when processing_template_decl. >>>>>> In particular, expr_noexcept_p generally can't be checked on a >>>>>> dependent >>>>>> expression. Next, tsubst_nested_requirement should avoid checking >>>>>> satisfaction when processing_template_decl. And similarly for >>>>>> convert_to_void (called from tsubst_valid_expression_requirement). >>>> >>>> I wonder if, instead of trying to do a partial substitution into a >>>> requires-expression at all, we want to use the >>>> PACK_EXPANSION_EXTRA_ARGS/IF_STMT_EXTRA_ARGS mechanism to remember the >>>> arguments for later satisfaction? > > Like this? > > -- >8 -- > > Subject: [PATCH] c++: requires-expressions and partial instantiation [PR96410] > > This patch makes tsubst_requires_expr avoid substituting into a > requires-expression when partially instantiating a generic lambda. This > is necessary in general to ensure that we check its requirements in > lexical order (see the first testcase below). Instead, template > arguments are saved in the requires-expression until all arguments can > be substituted into it at once, using a mechanism similar to > PACK_EXPANSION_EXTRA_ARGS. > > This change incidentally also fixes the two mentioned PRs -- the problem > there is that tsubst_requires_expr was performing semantic checks on > templated trees, and some of the checks are not prepared to handle such > trees. With this patch tsubst_requires_expr, no longer does any > semantic checking at all when processing_template_decl. > > gcc/cp/ChangeLog: > > PR c++/96409 > PR c++/96410 > * constraint.cc (tsubst_requires_expr): Use REQUIRES_EXPR_PARMS > and REQUIRES_EXPR_REQS. Use REQUIRES_EXPR_EXTRA_ARGS and > add_extra_args to defer substitution until we have all the > template arguments. > (finish_requires_expr): Adjust the call to build_min so that > REQUIRES_EXPR_EXTRA_ARGS gets set to NULL_TREE. > * cp-tree.def (REQUIRES_EXPR): Give it a third operand. > * cp-tree.h (REQUIRES_EXPR_PARMS, REQUIRES_EXPR_REQS, > REQUIRES_EXPR_EXTRA_ARGS): Define. > (add_extra_args): Declare. > > gcc/testsuite/ChangeLog: > > PR c++/96409 > PR c++/96410 > * g++.dg/cpp2a/concepts-lambda13.C: New test. > * g++.dg/cpp2a/concepts-lambda14.C: New test. > --- > gcc/cp/constraint.cc | 21 +++++++++++----- > gcc/cp/cp-tree.def | 8 +++--- > gcc/cp/cp-tree.h | 16 ++++++++++++ > .../g++.dg/cpp2a/concepts-lambda13.C | 18 +++++++++++++ > .../g++.dg/cpp2a/concepts-lambda14.C | 25 +++++++++++++++++++ > 5 files changed, 79 insertions(+), 9 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > index ad9d47070e3..9a06d763554 100644 > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -2175,7 +2175,19 @@ tsubst_requires_expr (tree t, tree args, > /* A requires-expression is an unevaluated context. */ > cp_unevaluated u; > > - tree parms = TREE_OPERAND (t, 0); > + args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args); > + if (processing_template_decl) > + { > + /* We're partially instantiating a generic lambda. Substituting into > + this requires-expression now may cause its requirements to get > + checked out of order, so instead just remember the template > + arguments and wait until we can substitute them all at once. */ > + t = copy_node (t); > + REQUIRES_EXPR_EXTRA_ARGS (t) = args; Don't we need build_extra_args, in case the requires_expr is in a local_specializations context like a function body? > + return t; > + } > + > + tree parms = REQUIRES_EXPR_PARMS (t); > if (parms) > { > parms = tsubst_constraint_variables (parms, args, info); > @@ -2183,14 +2195,11 @@ tsubst_requires_expr (tree t, tree args, > return boolean_false_node; > } > > - tree reqs = TREE_OPERAND (t, 1); > + tree reqs = REQUIRES_EXPR_REQS (t); > reqs = tsubst_requirement_body (reqs, args, info); > if (reqs == error_mark_node) > return boolean_false_node; > > - if (processing_template_decl) > - return finish_requires_expr (cp_expr_location (t), parms, reqs); > - > return boolean_true_node; > } > > @@ -2933,7 +2942,7 @@ finish_requires_expr (location_t loc, tree parms, tree reqs) > } > > /* Build the node. */ > - tree r = build_min (REQUIRES_EXPR, boolean_type_node, parms, reqs); > + tree r = build_min (REQUIRES_EXPR, boolean_type_node, parms, reqs, NULL_TREE); > TREE_SIDE_EFFECTS (r) = false; > TREE_CONSTANT (r) = true; > SET_EXPR_LOCATION (r, loc); > diff --git a/gcc/cp/cp-tree.def b/gcc/cp/cp-tree.def > index 31be2cf41a3..6eabe0d6d8f 100644 > --- a/gcc/cp/cp-tree.def > +++ b/gcc/cp/cp-tree.def > @@ -524,11 +524,13 @@ DEFTREECODE (CONSTRAINT_INFO, "constraint_info", tcc_exceptional, 0) > of the wildcard. */ > DEFTREECODE (WILDCARD_DECL, "wildcard_decl", tcc_declaration, 0) > > -/* A requires-expr is a binary expression. The first operand is > +/* A requires-expr has three operands. The first operand is > its parameter list (possibly NULL). The second is a list of > requirements, which are denoted by the _REQ* tree codes > - below. */ > -DEFTREECODE (REQUIRES_EXPR, "requires_expr", tcc_expression, 2) > + below. The third is a TREE_VEC of template arguments to > + be applied when substituting into the parameter list and > + requirements, set by tsubst_requires_expr for partial instantiations. */ > +DEFTREECODE (REQUIRES_EXPR, "requires_expr", tcc_expression, 3) > > /* A requirement for an expression. */ > DEFTREECODE (SIMPLE_REQ, "simple_req", tcc_expression, 1) > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index 6e4de7d0c4b..9c36e96ead6 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -1618,6 +1618,21 @@ check_constraint_info (tree t) > #define CONSTRAINED_PARM_PROTOTYPE(NODE) \ > DECL_INITIAL (TYPE_DECL_CHECK (NODE)) > > +/* The list of local parameters introduced by this requires-expression, > + in the form of a chain of PARM_DECLs. */ > +#define REQUIRES_EXPR_PARMS(NODE) \ > + TREE_OPERAND (TREE_CHECK (NODE, REQUIRES_EXPR), 0) > + > +/* A TREE_LIST of the requirements for this requires-expression. > + The requirements are stored in lexical order within the TREE_VALUE > + of each TREE_LIST node. The TREE_PURPOSE of each node is unused. */ > +#define REQUIRES_EXPR_REQS(NODE) \ > + TREE_OPERAND (TREE_CHECK (NODE, REQUIRES_EXPR), 1) > + > +/* Like PACK_EXPANSION_EXTRA_ARGS, for requires-expressions. */ > +#define REQUIRES_EXPR_EXTRA_ARGS(NODE) \ > + TREE_OPERAND (TREE_CHECK (NODE, REQUIRES_EXPR), 2) > + > enum cp_tree_node_structure_enum { > TS_CP_GENERIC, > TS_CP_IDENTIFIER, > @@ -7013,6 +7028,7 @@ extern bool template_guide_p (const_tree); > extern bool builtin_guide_p (const_tree); > extern void store_explicit_specifier (tree, tree); > extern tree add_outermost_template_args (tree, tree); > +extern tree add_extra_args (tree, tree); > > /* in rtti.c */ > /* A vector of all tinfo decls that haven't been emitted yet. */ > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > new file mode 100644 > index 00000000000..d4bed30a900 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > @@ -0,0 +1,18 @@ > +// { dg-do compile { target c++20 } } > + > +template<typename T> > +struct S { > + using type = T::type; // { dg-bogus "" } > +}; > + > +template<typename T> > +auto f() { > + return [] <typename U> (U) { > + // Verify that partial instantiation of this generic lambda doesn't cause > + // these requirements to get checked out of order. > + static_assert(!requires { typename U::type; typename S<T>::type; }); > + return 0; > + }; > +} > + > +int a = f<int>()(0); > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C > new file mode 100644 > index 00000000000..bdc893da857 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C > @@ -0,0 +1,25 @@ > +// PR c++/96410 > +// { dg-do compile { target c++20 } } > + > +struct S { using blah = void; }; > + > +template <typename T> constexpr bool trait = !__is_same(T, S); > +template <typename T> concept C = trait<T>; > + > +template<typename T> > +void foo() noexcept(!__is_same(T, void)) { } > + > +template<typename U> > +auto f() { > + return []<typename T>(T, bool a = requires { C<T>; }){ > + static_assert(requires { requires C<U> && (C<T> || C<T>); }); // { dg-error "assert" } > + static_assert(requires { C<T>; }); > + static_assert(requires { { foo<T>() } noexcept -> C; }); > + static_assert(!requires { typename T::blah; }); // { dg-error "assert" } > + return 0; > + }; > +} > + > +auto g = f<int>(); // { dg-bogus "" } > +int n = g(0); // { dg-bogus "" } > +int m = g(S{}); // { dg-message "required from here" } >
On Wed, 16 Sep 2020, Jason Merrill wrote: > On 9/16/20 1:34 PM, Patrick Palka wrote: > > On Thu, 13 Aug 2020, Jason Merrill wrote: > > > > > On 8/13/20 11:21 AM, Patrick Palka wrote: > > > > On Mon, 10 Aug 2020, Jason Merrill wrote: > > > > > > > > > On 8/10/20 2:18 PM, Patrick Palka wrote: > > > > > > On Mon, 10 Aug 2020, Patrick Palka wrote: > > > > > > > > > > > > > In the below testcase, semantic analysis of the > > > > > > > requires-expressions > > > > > > > in > > > > > > > the generic lambda must be delayed until instantiation of the > > > > > > > lambda > > > > > > > because the requirements depend on the lambda's template > > > > > > > arguments. > > > > > > > But > > > > > > > tsubst_requires_expr does semantic analysis even during > > > > > > > regeneration > > > > > > > of > > > > > > > the lambda, which leads to various bogus errors and ICEs since > > > > > > > some > > > > > > > subroutines aren't prepared to handle dependent/template trees. > > > > > > > > > > > > > > This patch adjusts subroutines of tsubst_requires_expr to avoid > > > > > > > doing > > > > > > > some problematic semantic analyses when processing_template_decl. > > > > > > > In particular, expr_noexcept_p generally can't be checked on a > > > > > > > dependent > > > > > > > expression. Next, tsubst_nested_requirement should avoid checking > > > > > > > satisfaction when processing_template_decl. And similarly for > > > > > > > convert_to_void (called from tsubst_valid_expression_requirement). > > > > > > > > > > I wonder if, instead of trying to do a partial substitution into a > > > > > requires-expression at all, we want to use the > > > > > PACK_EXPANSION_EXTRA_ARGS/IF_STMT_EXTRA_ARGS mechanism to remember the > > > > > arguments for later satisfaction? > > > > Like this? > > > > -- >8 -- > > > > Subject: [PATCH] c++: requires-expressions and partial instantiation > > [PR96410] > > > > This patch makes tsubst_requires_expr avoid substituting into a > > requires-expression when partially instantiating a generic lambda. This > > is necessary in general to ensure that we check its requirements in > > lexical order (see the first testcase below). Instead, template > > arguments are saved in the requires-expression until all arguments can > > be substituted into it at once, using a mechanism similar to > > PACK_EXPANSION_EXTRA_ARGS. > > > > This change incidentally also fixes the two mentioned PRs -- the problem > > there is that tsubst_requires_expr was performing semantic checks on > > templated trees, and some of the checks are not prepared to handle such > > trees. With this patch tsubst_requires_expr, no longer does any > > semantic checking at all when processing_template_decl. > > > > gcc/cp/ChangeLog: > > > > PR c++/96409 > > PR c++/96410 > > * constraint.cc (tsubst_requires_expr): Use REQUIRES_EXPR_PARMS > > and REQUIRES_EXPR_REQS. Use REQUIRES_EXPR_EXTRA_ARGS and > > add_extra_args to defer substitution until we have all the > > template arguments. > > (finish_requires_expr): Adjust the call to build_min so that > > REQUIRES_EXPR_EXTRA_ARGS gets set to NULL_TREE. > > * cp-tree.def (REQUIRES_EXPR): Give it a third operand. > > * cp-tree.h (REQUIRES_EXPR_PARMS, REQUIRES_EXPR_REQS, > > REQUIRES_EXPR_EXTRA_ARGS): Define. > > (add_extra_args): Declare. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/96409 > > PR c++/96410 > > * g++.dg/cpp2a/concepts-lambda13.C: New test. > > * g++.dg/cpp2a/concepts-lambda14.C: New test. > > --- > > gcc/cp/constraint.cc | 21 +++++++++++----- > > gcc/cp/cp-tree.def | 8 +++--- > > gcc/cp/cp-tree.h | 16 ++++++++++++ > > .../g++.dg/cpp2a/concepts-lambda13.C | 18 +++++++++++++ > > .../g++.dg/cpp2a/concepts-lambda14.C | 25 +++++++++++++++++++ > > 5 files changed, 79 insertions(+), 9 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > index ad9d47070e3..9a06d763554 100644 > > --- a/gcc/cp/constraint.cc > > +++ b/gcc/cp/constraint.cc > > @@ -2175,7 +2175,19 @@ tsubst_requires_expr (tree t, tree args, > > /* A requires-expression is an unevaluated context. */ > > cp_unevaluated u; > > - tree parms = TREE_OPERAND (t, 0); > > + args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args); > > + if (processing_template_decl) > > + { > > + /* We're partially instantiating a generic lambda. Substituting into > > + this requires-expression now may cause its requirements to get > > + checked out of order, so instead just remember the template > > + arguments and wait until we can substitute them all at once. */ > > + t = copy_node (t); > > + REQUIRES_EXPR_EXTRA_ARGS (t) = args; > > Don't we need build_extra_args, in case the requires_expr is in a > local_specializations context like a function body? Ah yes probably... though I haven't yet been able to come up with a concrete testcase that demonstrates we need it. It seems we get away without it because a requires-expression is an unevaluated operand, and many callers of retrieve_local_specialization have fallback code that triggers only when inside an unevaluated operand, e.g. tsubst_copy <case PARM_DECL> does: r = retrieve_local_specialization (t); if (r == NULL_TREE) { ... /* This can happen for a parameter name used later in a function declaration (such as in a late-specified return type). Just make a dummy decl, since it's only used for its type. */ gcc_assert (cp_unevaluated_operand != 0); r = tsubst_decl (t, args, complain); .... I am testing the following which adds a call to build_extra_args: -- >8 -- gcc/cp/ChangeLog: PR c++/96409 PR c++/96410 * constraint.cc (tsubst_requires_expr): Use REQUIRES_EXPR_PARMS and REQUIRES_EXPR_REQS. Use REQUIRES_EXPR_EXTRA_ARGS, add_extra_args and build_extra_args to defer substitution until we have all the template arguments. (finish_requires_expr): Adjust the call to build_min so that REQUIRES_EXPR_EXTRA_ARGS gets set to NULL_TREE. * cp-tree.def (REQUIRES_EXPR): Give it a third operand. * cp-tree.h (REQUIRES_EXPR_PARMS, REQUIRES_EXPR_REQS, REQUIRES_EXPR_EXTRA_ARGS): Define. (add_extra_args, build_extra_args): Declare. gcc/testsuite/ChangeLog: PR c++/96409 PR c++/96410 * g++.dg/cpp2a/concepts-lambda13.C: New test. * g++.dg/cpp2a/concepts-lambda14.C: New test. --- gcc/cp/constraint.cc | 21 +++++++++++----- gcc/cp/cp-tree.def | 8 +++--- gcc/cp/cp-tree.h | 17 +++++++++++++ .../g++.dg/cpp2a/concepts-lambda13.C | 18 +++++++++++++ .../g++.dg/cpp2a/concepts-lambda14.C | 25 +++++++++++++++++++ 5 files changed, 80 insertions(+), 9 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index ad9d47070e3..0aab3073cc1 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -2175,7 +2175,19 @@ tsubst_requires_expr (tree t, tree args, /* A requires-expression is an unevaluated context. */ cp_unevaluated u; - tree parms = TREE_OPERAND (t, 0); + args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args); + if (processing_template_decl) + { + /* We're partially instantiating a generic lambda. Substituting into + this requires-expression now may cause its requirements to get + checked out of order, so instead just remember the template + arguments and wait until we can substitute them all at once. */ + t = copy_node (t); + REQUIRES_EXPR_EXTRA_ARGS (t) = build_extra_args (t, args, complain); + return t; + } + + tree parms = REQUIRES_EXPR_PARMS (t); if (parms) { parms = tsubst_constraint_variables (parms, args, info); @@ -2183,14 +2195,11 @@ tsubst_requires_expr (tree t, tree args, return boolean_false_node; } - tree reqs = TREE_OPERAND (t, 1); + tree reqs = REQUIRES_EXPR_REQS (t); reqs = tsubst_requirement_body (reqs, args, info); if (reqs == error_mark_node) return boolean_false_node; - if (processing_template_decl) - return finish_requires_expr (cp_expr_location (t), parms, reqs); - return boolean_true_node; } @@ -2933,7 +2942,7 @@ finish_requires_expr (location_t loc, tree parms, tree reqs) } /* Build the node. */ - tree r = build_min (REQUIRES_EXPR, boolean_type_node, parms, reqs); + tree r = build_min (REQUIRES_EXPR, boolean_type_node, parms, reqs, NULL_TREE); TREE_SIDE_EFFECTS (r) = false; TREE_CONSTANT (r) = true; SET_EXPR_LOCATION (r, loc); diff --git a/gcc/cp/cp-tree.def b/gcc/cp/cp-tree.def index 31be2cf41a3..6eabe0d6d8f 100644 --- a/gcc/cp/cp-tree.def +++ b/gcc/cp/cp-tree.def @@ -524,11 +524,13 @@ DEFTREECODE (CONSTRAINT_INFO, "constraint_info", tcc_exceptional, 0) of the wildcard. */ DEFTREECODE (WILDCARD_DECL, "wildcard_decl", tcc_declaration, 0) -/* A requires-expr is a binary expression. The first operand is +/* A requires-expr has three operands. The first operand is its parameter list (possibly NULL). The second is a list of requirements, which are denoted by the _REQ* tree codes - below. */ -DEFTREECODE (REQUIRES_EXPR, "requires_expr", tcc_expression, 2) + below. The third is a TREE_VEC of template arguments to + be applied when substituting into the parameter list and + requirements, set by tsubst_requires_expr for partial instantiations. */ +DEFTREECODE (REQUIRES_EXPR, "requires_expr", tcc_expression, 3) /* A requirement for an expression. */ DEFTREECODE (SIMPLE_REQ, "simple_req", tcc_expression, 1) diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 5b727348e51..08976d8527c 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -1618,6 +1618,21 @@ check_constraint_info (tree t) #define CONSTRAINED_PARM_PROTOTYPE(NODE) \ DECL_INITIAL (TYPE_DECL_CHECK (NODE)) +/* The list of local parameters introduced by this requires-expression, + in the form of a chain of PARM_DECLs. */ +#define REQUIRES_EXPR_PARMS(NODE) \ + TREE_OPERAND (TREE_CHECK (NODE, REQUIRES_EXPR), 0) + +/* A TREE_LIST of the requirements for this requires-expression. + The requirements are stored in lexical order within the TREE_VALUE + of each TREE_LIST node. The TREE_PURPOSE of each node is unused. */ +#define REQUIRES_EXPR_REQS(NODE) \ + TREE_OPERAND (TREE_CHECK (NODE, REQUIRES_EXPR), 1) + +/* Like PACK_EXPANSION_EXTRA_ARGS, for requires-expressions. */ +#define REQUIRES_EXPR_EXTRA_ARGS(NODE) \ + TREE_OPERAND (TREE_CHECK (NODE, REQUIRES_EXPR), 2) + enum cp_tree_node_structure_enum { TS_CP_GENERIC, TS_CP_IDENTIFIER, @@ -7013,6 +7028,8 @@ extern bool template_guide_p (const_tree); extern bool builtin_guide_p (const_tree); extern void store_explicit_specifier (tree, tree); extern tree add_outermost_template_args (tree, tree); +extern tree add_extra_args (tree, tree); +extern tree build_extra_args (tree, tree, tsubst_flags_t); /* in rtti.c */ /* A vector of all tinfo decls that haven't been emitted yet. */ diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C new file mode 100644 index 00000000000..d4bed30a900 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C @@ -0,0 +1,18 @@ +// { dg-do compile { target c++20 } } + +template<typename T> +struct S { + using type = T::type; // { dg-bogus "" } +}; + +template<typename T> +auto f() { + return [] <typename U> (U) { + // Verify that partial instantiation of this generic lambda doesn't cause + // these requirements to get checked out of order. + static_assert(!requires { typename U::type; typename S<T>::type; }); + return 0; + }; +} + +int a = f<int>()(0); diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C new file mode 100644 index 00000000000..bdc893da857 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C @@ -0,0 +1,25 @@ +// PR c++/96410 +// { dg-do compile { target c++20 } } + +struct S { using blah = void; }; + +template <typename T> constexpr bool trait = !__is_same(T, S); +template <typename T> concept C = trait<T>; + +template<typename T> +void foo() noexcept(!__is_same(T, void)) { } + +template<typename U> +auto f() { + return []<typename T>(T, bool a = requires { C<T>; }){ + static_assert(requires { requires C<U> && (C<T> || C<T>); }); // { dg-error "assert" } + static_assert(requires { C<T>; }); + static_assert(requires { { foo<T>() } noexcept -> C; }); + static_assert(!requires { typename T::blah; }); // { dg-error "assert" } + return 0; + }; +} + +auto g = f<int>(); // { dg-bogus "" } +int n = g(0); // { dg-bogus "" } +int m = g(S{}); // { dg-message "required from here" }
On 9/16/20 6:11 PM, Patrick Palka wrote: > On Wed, 16 Sep 2020, Jason Merrill wrote: > >> On 9/16/20 1:34 PM, Patrick Palka wrote: >>> On Thu, 13 Aug 2020, Jason Merrill wrote: >>> >>>> On 8/13/20 11:21 AM, Patrick Palka wrote: >>>>> On Mon, 10 Aug 2020, Jason Merrill wrote: >>>>> >>>>>> On 8/10/20 2:18 PM, Patrick Palka wrote: >>>>>>> On Mon, 10 Aug 2020, Patrick Palka wrote: >>>>>>> >>>>>>>> In the below testcase, semantic analysis of the >>>>>>>> requires-expressions >>>>>>>> in >>>>>>>> the generic lambda must be delayed until instantiation of the >>>>>>>> lambda >>>>>>>> because the requirements depend on the lambda's template >>>>>>>> arguments. >>>>>>>> But >>>>>>>> tsubst_requires_expr does semantic analysis even during >>>>>>>> regeneration >>>>>>>> of >>>>>>>> the lambda, which leads to various bogus errors and ICEs since >>>>>>>> some >>>>>>>> subroutines aren't prepared to handle dependent/template trees. >>>>>>>> >>>>>>>> This patch adjusts subroutines of tsubst_requires_expr to avoid >>>>>>>> doing >>>>>>>> some problematic semantic analyses when processing_template_decl. >>>>>>>> In particular, expr_noexcept_p generally can't be checked on a >>>>>>>> dependent >>>>>>>> expression. Next, tsubst_nested_requirement should avoid checking >>>>>>>> satisfaction when processing_template_decl. And similarly for >>>>>>>> convert_to_void (called from tsubst_valid_expression_requirement). >>>>>> >>>>>> I wonder if, instead of trying to do a partial substitution into a >>>>>> requires-expression at all, we want to use the >>>>>> PACK_EXPANSION_EXTRA_ARGS/IF_STMT_EXTRA_ARGS mechanism to remember the >>>>>> arguments for later satisfaction? >>> >>> Like this? >>> >>> -- >8 -- >>> >>> Subject: [PATCH] c++: requires-expressions and partial instantiation >>> [PR96410] >>> >>> This patch makes tsubst_requires_expr avoid substituting into a >>> requires-expression when partially instantiating a generic lambda. This >>> is necessary in general to ensure that we check its requirements in >>> lexical order (see the first testcase below). Instead, template >>> arguments are saved in the requires-expression until all arguments can >>> be substituted into it at once, using a mechanism similar to >>> PACK_EXPANSION_EXTRA_ARGS. >>> >>> This change incidentally also fixes the two mentioned PRs -- the problem >>> there is that tsubst_requires_expr was performing semantic checks on >>> templated trees, and some of the checks are not prepared to handle such >>> trees. With this patch tsubst_requires_expr, no longer does any >>> semantic checking at all when processing_template_decl. >>> >>> gcc/cp/ChangeLog: >>> >>> PR c++/96409 >>> PR c++/96410 >>> * constraint.cc (tsubst_requires_expr): Use REQUIRES_EXPR_PARMS >>> and REQUIRES_EXPR_REQS. Use REQUIRES_EXPR_EXTRA_ARGS and >>> add_extra_args to defer substitution until we have all the >>> template arguments. >>> (finish_requires_expr): Adjust the call to build_min so that >>> REQUIRES_EXPR_EXTRA_ARGS gets set to NULL_TREE. >>> * cp-tree.def (REQUIRES_EXPR): Give it a third operand. >>> * cp-tree.h (REQUIRES_EXPR_PARMS, REQUIRES_EXPR_REQS, >>> REQUIRES_EXPR_EXTRA_ARGS): Define. >>> (add_extra_args): Declare. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR c++/96409 >>> PR c++/96410 >>> * g++.dg/cpp2a/concepts-lambda13.C: New test. >>> * g++.dg/cpp2a/concepts-lambda14.C: New test. >>> --- >>> gcc/cp/constraint.cc | 21 +++++++++++----- >>> gcc/cp/cp-tree.def | 8 +++--- >>> gcc/cp/cp-tree.h | 16 ++++++++++++ >>> .../g++.dg/cpp2a/concepts-lambda13.C | 18 +++++++++++++ >>> .../g++.dg/cpp2a/concepts-lambda14.C | 25 +++++++++++++++++++ >>> 5 files changed, 79 insertions(+), 9 deletions(-) >>> create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C >>> create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C >>> >>> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc >>> index ad9d47070e3..9a06d763554 100644 >>> --- a/gcc/cp/constraint.cc >>> +++ b/gcc/cp/constraint.cc >>> @@ -2175,7 +2175,19 @@ tsubst_requires_expr (tree t, tree args, >>> /* A requires-expression is an unevaluated context. */ >>> cp_unevaluated u; >>> - tree parms = TREE_OPERAND (t, 0); >>> + args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args); >>> + if (processing_template_decl) >>> + { >>> + /* We're partially instantiating a generic lambda. Substituting into >>> + this requires-expression now may cause its requirements to get >>> + checked out of order, so instead just remember the template >>> + arguments and wait until we can substitute them all at once. */ >>> + t = copy_node (t); >>> + REQUIRES_EXPR_EXTRA_ARGS (t) = args; >> >> Don't we need build_extra_args, in case the requires_expr is in a >> local_specializations context like a function body? > > Ah yes probably... though I haven't yet been able to come up with a > concrete testcase that demonstrates we need it. It seems we get away > without it because a requires-expression is an unevaluated operand, and > many callers of retrieve_local_specialization have fallback code that > triggers only when inside an unevaluated operand, e.g. > tsubst_copy <case PARM_DECL> does: > > r = retrieve_local_specialization (t); > if (r == NULL_TREE) > { > ... > > /* This can happen for a parameter name used later in a function > declaration (such as in a late-specified return type). Just > make a dummy decl, since it's only used for its type. */ > gcc_assert (cp_unevaluated_operand != 0); > r = tsubst_decl (t, args, complain); > .... > > I am testing the following which adds a call to build_extra_args: OK if it passes. > -- >8 -- > > gcc/cp/ChangeLog: > > PR c++/96409 > PR c++/96410 > * constraint.cc (tsubst_requires_expr): Use REQUIRES_EXPR_PARMS > and REQUIRES_EXPR_REQS. Use REQUIRES_EXPR_EXTRA_ARGS, > add_extra_args and build_extra_args to defer substitution until > we have all the template arguments. > (finish_requires_expr): Adjust the call to build_min so that > REQUIRES_EXPR_EXTRA_ARGS gets set to NULL_TREE. > * cp-tree.def (REQUIRES_EXPR): Give it a third operand. > * cp-tree.h (REQUIRES_EXPR_PARMS, REQUIRES_EXPR_REQS, > REQUIRES_EXPR_EXTRA_ARGS): Define. > (add_extra_args, build_extra_args): Declare. > > gcc/testsuite/ChangeLog: > > PR c++/96409 > PR c++/96410 > * g++.dg/cpp2a/concepts-lambda13.C: New test. > * g++.dg/cpp2a/concepts-lambda14.C: New test. > --- > gcc/cp/constraint.cc | 21 +++++++++++----- > gcc/cp/cp-tree.def | 8 +++--- > gcc/cp/cp-tree.h | 17 +++++++++++++ > .../g++.dg/cpp2a/concepts-lambda13.C | 18 +++++++++++++ > .../g++.dg/cpp2a/concepts-lambda14.C | 25 +++++++++++++++++++ > 5 files changed, 80 insertions(+), 9 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > index ad9d47070e3..0aab3073cc1 100644 > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -2175,7 +2175,19 @@ tsubst_requires_expr (tree t, tree args, > /* A requires-expression is an unevaluated context. */ > cp_unevaluated u; > > - tree parms = TREE_OPERAND (t, 0); > + args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args); > + if (processing_template_decl) > + { > + /* We're partially instantiating a generic lambda. Substituting into > + this requires-expression now may cause its requirements to get > + checked out of order, so instead just remember the template > + arguments and wait until we can substitute them all at once. */ > + t = copy_node (t); > + REQUIRES_EXPR_EXTRA_ARGS (t) = build_extra_args (t, args, complain); > + return t; > + } > + > + tree parms = REQUIRES_EXPR_PARMS (t); > if (parms) > { > parms = tsubst_constraint_variables (parms, args, info); > @@ -2183,14 +2195,11 @@ tsubst_requires_expr (tree t, tree args, > return boolean_false_node; > } > > - tree reqs = TREE_OPERAND (t, 1); > + tree reqs = REQUIRES_EXPR_REQS (t); > reqs = tsubst_requirement_body (reqs, args, info); > if (reqs == error_mark_node) > return boolean_false_node; > > - if (processing_template_decl) > - return finish_requires_expr (cp_expr_location (t), parms, reqs); > - > return boolean_true_node; > } > > @@ -2933,7 +2942,7 @@ finish_requires_expr (location_t loc, tree parms, tree reqs) > } > > /* Build the node. */ > - tree r = build_min (REQUIRES_EXPR, boolean_type_node, parms, reqs); > + tree r = build_min (REQUIRES_EXPR, boolean_type_node, parms, reqs, NULL_TREE); > TREE_SIDE_EFFECTS (r) = false; > TREE_CONSTANT (r) = true; > SET_EXPR_LOCATION (r, loc); > diff --git a/gcc/cp/cp-tree.def b/gcc/cp/cp-tree.def > index 31be2cf41a3..6eabe0d6d8f 100644 > --- a/gcc/cp/cp-tree.def > +++ b/gcc/cp/cp-tree.def > @@ -524,11 +524,13 @@ DEFTREECODE (CONSTRAINT_INFO, "constraint_info", tcc_exceptional, 0) > of the wildcard. */ > DEFTREECODE (WILDCARD_DECL, "wildcard_decl", tcc_declaration, 0) > > -/* A requires-expr is a binary expression. The first operand is > +/* A requires-expr has three operands. The first operand is > its parameter list (possibly NULL). The second is a list of > requirements, which are denoted by the _REQ* tree codes > - below. */ > -DEFTREECODE (REQUIRES_EXPR, "requires_expr", tcc_expression, 2) > + below. The third is a TREE_VEC of template arguments to > + be applied when substituting into the parameter list and > + requirements, set by tsubst_requires_expr for partial instantiations. */ > +DEFTREECODE (REQUIRES_EXPR, "requires_expr", tcc_expression, 3) > > /* A requirement for an expression. */ > DEFTREECODE (SIMPLE_REQ, "simple_req", tcc_expression, 1) > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index 5b727348e51..08976d8527c 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -1618,6 +1618,21 @@ check_constraint_info (tree t) > #define CONSTRAINED_PARM_PROTOTYPE(NODE) \ > DECL_INITIAL (TYPE_DECL_CHECK (NODE)) > > +/* The list of local parameters introduced by this requires-expression, > + in the form of a chain of PARM_DECLs. */ > +#define REQUIRES_EXPR_PARMS(NODE) \ > + TREE_OPERAND (TREE_CHECK (NODE, REQUIRES_EXPR), 0) > + > +/* A TREE_LIST of the requirements for this requires-expression. > + The requirements are stored in lexical order within the TREE_VALUE > + of each TREE_LIST node. The TREE_PURPOSE of each node is unused. */ > +#define REQUIRES_EXPR_REQS(NODE) \ > + TREE_OPERAND (TREE_CHECK (NODE, REQUIRES_EXPR), 1) > + > +/* Like PACK_EXPANSION_EXTRA_ARGS, for requires-expressions. */ > +#define REQUIRES_EXPR_EXTRA_ARGS(NODE) \ > + TREE_OPERAND (TREE_CHECK (NODE, REQUIRES_EXPR), 2) > + > enum cp_tree_node_structure_enum { > TS_CP_GENERIC, > TS_CP_IDENTIFIER, > @@ -7013,6 +7028,8 @@ extern bool template_guide_p (const_tree); > extern bool builtin_guide_p (const_tree); > extern void store_explicit_specifier (tree, tree); > extern tree add_outermost_template_args (tree, tree); > +extern tree add_extra_args (tree, tree); > +extern tree build_extra_args (tree, tree, tsubst_flags_t); > > /* in rtti.c */ > /* A vector of all tinfo decls that haven't been emitted yet. */ > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > new file mode 100644 > index 00000000000..d4bed30a900 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C > @@ -0,0 +1,18 @@ > +// { dg-do compile { target c++20 } } > + > +template<typename T> > +struct S { > + using type = T::type; // { dg-bogus "" } > +}; > + > +template<typename T> > +auto f() { > + return [] <typename U> (U) { > + // Verify that partial instantiation of this generic lambda doesn't cause > + // these requirements to get checked out of order. > + static_assert(!requires { typename U::type; typename S<T>::type; }); > + return 0; > + }; > +} > + > +int a = f<int>()(0); > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C > new file mode 100644 > index 00000000000..bdc893da857 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda14.C > @@ -0,0 +1,25 @@ > +// PR c++/96410 > +// { dg-do compile { target c++20 } } > + > +struct S { using blah = void; }; > + > +template <typename T> constexpr bool trait = !__is_same(T, S); > +template <typename T> concept C = trait<T>; > + > +template<typename T> > +void foo() noexcept(!__is_same(T, void)) { } > + > +template<typename U> > +auto f() { > + return []<typename T>(T, bool a = requires { C<T>; }){ > + static_assert(requires { requires C<U> && (C<T> || C<T>); }); // { dg-error "assert" } > + static_assert(requires { C<T>; }); > + static_assert(requires { { foo<T>() } noexcept -> C; }); > + static_assert(!requires { typename T::blah; }); // { dg-error "assert" } > + return 0; > + }; > +} > + > +auto g = f<int>(); // { dg-bogus "" } > +int n = g(0); // { dg-bogus "" } > +int m = g(S{}); // { dg-message "required from here" } >
diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index e4aace596e7..db2036502a7 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -1993,7 +1993,8 @@ tsubst_compound_requirement (tree t, tree args, subst_info info) /* Check the noexcept condition. */ bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t); - if (noexcept_p && !expr_noexcept_p (expr, tf_none)) + if (!processing_template_decl + && noexcept_p && !expr_noexcept_p (expr, tf_none)) return error_mark_node; /* Substitute through the type expression, if any. */ @@ -2023,6 +2024,12 @@ static tree tsubst_nested_requirement (tree t, tree args, subst_info info) { /* Ensure that we're in an evaluation context prior to satisfaction. */ + if (processing_template_decl) + { + tree r = tsubst_constraint (TREE_OPERAND (t, 0), args, + info.complain, info.in_decl); + return finish_nested_requirement (EXPR_LOCATION (t), r); + } tree norm = TREE_TYPE (t); tree result = satisfy_constraint (norm, args, info); if (result == error_mark_node && info.quiet ()) diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c index c9e7b1ff044..1d2c2d3351c 100644 --- a/gcc/cp/cvt.c +++ b/gcc/cp/cvt.c @@ -1159,7 +1159,7 @@ convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain) /* Explicitly evaluate void-converted concept checks since their satisfaction may produce ill-formed programs. */ - if (concept_check_p (expr)) + if (!processing_template_decl && concept_check_p (expr)) expr = evaluate_concept_check (expr, tf_warning_or_error); if (VOID_TYPE_P (TREE_TYPE (expr))) diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C new file mode 100644 index 00000000000..9757ce49d67 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda13.C @@ -0,0 +1,25 @@ +// PR c++/96410 +// { dg-do compile { target c++20 } } + +struct S { using blah = void; }; + +template <typename T> constexpr bool trait = !__is_same(T, S); +template <typename T> concept C = trait<T>; + +template<typename T> +void foo() noexcept(!__is_same(T, void)) { } + +template<typename U> +auto f() { + return []<typename T>(T){ + static_assert(requires { requires C<U> && (C<T> || C<T>); }); // { dg-error "assert" } + static_assert(requires { C<T>; }); + static_assert(requires { { foo<T>() } noexcept -> C; }); + static_assert(!requires { typename T::blah; }); // { dg-error "assert" } + return 0; + }; +} + +auto g = f<int>(); // { dg-bogus "" } +int n = g(0); // { dg-bogus "" } +int m = g(S{}); // { dg-message "required from here" }