Re: [PATCH] c++: premature analysis of requires-expression [PR96410]

[Jason Merrill via Gcc-patches]

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