[PATCH] c++: premature analysis of requires-expression [PR96410]
Jason Merrill
jason@redhat.com
Thu Sep 17 02:59:31 GMT 2020
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" }
>
More information about the Gcc-patches
mailing list