Hi,

PR96251 (and dups) is a rejects-valid bug against coroutines (but I’m
not sure that the problem is really there).

 * coroutines cannot be constexpr.

* Part of the way that the coroutine implementation works is a
consequence of the "lazy discovery of coroutine-ness”;  whenever
we first encounter a coroutine keyword...

.. we mark the function as a coroutine, and then we can deal with
diagnostics etc. that change under these circumstances.  This
marking of the function is independent of whether keyword
expressions are type-dependent or not.

* when we encounter a coroutine keyword in a constexpr context
we error.

* the constexpr machinery also has been taught that coroutine
expressions should cause constexpr-ness to be rejected when
checking for "potentially constexpr".

So why is there a problem?

* lambdas are implicitly constexpr from C++17.

* the constexpr machinery tends to bail early *with a conservative
  assumption that the expression is potentially constexpr* when it
  finds a type-dependent expression - without evaluating sub-
  expressions to see if they are valid (thus missing coroutine exprs.
  in such positions).

The combination of these ^^ two things, means that for many generic
lambdas with non-trivial bodies - we then enter instantiation with a
constexpr type-dependent coroutine (which is a Thing that Should not
Be).  As soon as we try to tsub any coroutine keyword expression
encountered, we error out.

* I was not able to see any way in which the instantiation process
  could be made to bail in this case and re-try for non-constexpr.

* Nor able to see somewhere else where that decision could be made.

^^ these two points reflect that I'm not familiar with the constexpr
    machinery.

* Proposed solution.

Since coroutines cannot be constexpr (not even sure what that would
mean in any practicable implementation).  The fix proposed is to
reject constexpr-ness for coroutine functions as early as possible.

* a) We can prevent a generic lambda coroutine from being converted
  to constexpr in finish_function ().  Likewise, via the tests below, we can
  avoid it for regular lambdas.

  b) We can also reject coroutine function decls early in both
  is_valid_constexpr_fn () and potential_constant_expression_1 ().

So - is there some alternate solution that would be better?

====

(in some ways it seems pointless to delay rejection of a coroutine
 function to some later point).

OTOH, I suppose that one could have some weird code where coroutine
coroutine keywords only appeared in a non-constexpr paths of the code
- but our current lowering is not prepared for such a circumstance.

AFAIU the standard, there's no dispensation from the "cannot be
constexpr" for such a code construction .. but ICBW.

In any event, the cases reported (and fixed by this patch) are not trying
anything so fiendishly clever.

Tested on x86_64-darwin.

Suggestions?
OK for master (after wider testing)?
thanks
Iain

= ----

coroutines cannot be constexpr, but generic lambdas (from C++17)
are implicitly assumed to be, and the processing of type-
dependent lambda bodies often terminates before any coroutine
expressions are encountered.  For example, the PR notes cases
where the coro expressions are hidden by type-dependent for or
switch expressions.

The solution proposed is to deny coroutine generic lambdas.  We also
tighten up the checks for is_valid_constexpr_fn() and do an early
test for coroutine functions in checking for potential constant
expressions.

gcc/cp/ChangeLog:

        PR c++/96251
        * constexpr.c (is_valid_constexpr_fn): Test for a
        coroutine before the chekc for lambda.
        (potential_constant_expression_1): Disallow coroutine
        function decls.
        * decl.c (finish_function): Do not mark generic
        coroutine lambda templates as constexpr.

gcc/testsuite/ChangeLog:

        PR c++/96251
        * g++.dg/coroutines/pr96251.C: New test.
---
 gcc/cp/constexpr.c                        |  7 ++++++-
 gcc/cp/decl.c                             |  2 +-
 gcc/testsuite/g++.dg/coroutines/pr96251.C | 22 ++++++++++++++++++++++
 3 files changed, 29 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr96251.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 377fe322ee8..036bf04efa9 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -210,7 +210,9 @@ is_valid_constexpr_fn (tree fun, bool complain)
          }
     }

-  if (LAMBDA_TYPE_P (CP_DECL_CONTEXT (fun)) && cxx_dialect < cxx17)
+  if (DECL_COROUTINE_P (fun))
+    ret = false;
+  else if (LAMBDA_TYPE_P (CP_DECL_CONTEXT (fun)) && cxx_dialect < cxx17)
     {
       ret = false;
       if (complain)
@@ -7827,6 +7829,9 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
   switch (TREE_CODE (t))
     {
     case FUNCTION_DECL:
+      if (DECL_COROUTINE_P (t))
+       return false;
+       /* FALLTHROUGH.  */
     case BASELINK:
     case TEMPLATE_DECL:
     case OVERLOAD:
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 7fa8f52d667..3a089b5bee5 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -17374,7 +17374,7 @@ finish_function (bool inline_p)
   if (cxx_dialect >= cxx17
       && LAMBDA_TYPE_P (CP_DECL_CONTEXT (fndecl)))
     DECL_DECLARED_CONSTEXPR_P (fndecl)
-      = ((processing_template_decl
+      = (((processing_template_decl && !DECL_COROUTINE_P(fndecl))
          || is_valid_constexpr_fn (fndecl, /*complain*/false))
         && potential_constant_expression (DECL_SAVED_TREE (fndecl)));

diff --git a/gcc/testsuite/g++.dg/coroutines/pr96251.C b/gcc/testsuite/g++.dg/coroutines/pr96251.C
new file mode 100644
index 00000000000..6824d783d5f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr96251.C
@@ -0,0 +1,22 @@
+#include <coroutine>
+
+struct coroutine {
+  struct promise_type {
+    auto get_return_object() { return coroutine(); }
+    auto initial_suspend() { return std::suspend_always(); }
+    auto yield_value(int) { return std::suspend_always(); }
+    void return_void() {}
+    auto final_suspend() noexcept { return std::suspend_always(); }
+    void unhandled_exception() {}
+  };
+};
+
+int main() {
+  auto f = [](auto max) -> coroutine {
+    for (int i = 0; i < max; ++i) {
+       co_yield i;
+    }
+  };
+
+  f(10);
+}
--
2.24.1


Reply via email to