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" }
> >
>
>
