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" } -- 2.28.0.497.g54e85e7af1