[PATCH] c++: premature analysis of requires-expression [PR96410]
Patrick Palka
ppalka@redhat.com
Wed Sep 16 17:34:01 GMT 2020
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" }
--
2.28.0.497.g54e85e7af1
> >
> > 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" }
> > > >
> > >
> > >
> >
>
>
More information about the Gcc-patches
mailing list