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

Jason Merrill via Gcc-patches Mon, 10 Aug 2020 14:34:21 -0700

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 arequires-expression at all, we want to use thePACK_EXPANSION_EXTRA_ARGS/IF_STMT_EXTRA_ARGS mechanism to remember thearguments for later satisfaction?


Jason

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

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

Reply via email to