| Message ID | 20240724011619.2013098-1-arsen@aarsen.me |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] cp/coroutines: do not rewrite parameters in unevaluated contexts | expand |
On 7/23/24 7:41 PM, Arsen Arsenović wrote: > It is possible to use parameters of a parent function of a lambda in > unevaluated contexts without capturing them. By not capturing them, we > work around the usual mechanism we use to prevent rewriting captured > parameters. Prevent this by simply skipping rewrites in unevaluated > contexts. Those won't mind the value not being present anyway. > > gcc/cp/ChangeLog: > > PR c++/111728 > * coroutines.cc (rewrite_param_uses): Skip unevaluated > subexpressions. > > gcc/testsuite/ChangeLog: > > PR c++/111728 > * g++.dg/coroutines/pr111728.C: New test. > --- > Evening! > > This 'series' contains two patches for the coroutine implementation to > address two unrelated PRs. When you're explicitly dividing your description between stuff that goes in the commit message and stuff that doesn't, the stuff that doesn't go in the commit message should come first, followed by a "scissors" line, per git-mailinfo(1): > --scissors > Remove everything in body before a scissors line (e.g. "-- >8 --"). The line > represents scissors and perforation marks, and is used to request the reader > to cut the message at that line. If that line appears in the body of the > message before the patch, everything before it (including the scissors line > itself) is ignored when this option is used. > > This is useful if you want to begin your message in a discussion thread with > comments and suggestions on the message you are responding to, and to > conclude it with a patch submission, separating the discussion and the > beginning of the proposed commit log message with a scissors line. > The first prevents an ICE during coroutine parameter substitution by not > performing it in unevaluated contexts. Those contexts can contain names > that were not captured by lambdas but *are* parameters to coroutines. > In the testcase from the PR, the rewriting machinery finds a param in > the body of the coroutine, which it did not previously encounter while > processing the coroutine declaration, and that does not have a > DECL_VALUE_EXPR, and fails. > > Since it is not really useful to rewrite parameter uses in unevaluated > contexts, we can just ignore those, preventing confusion (and the ICE). All of the explanation of the change rationale can merge into the commit message. > Regression tested on x86_64-pc-linux-gnu. > > OK for trunk? > > TIA, have a lovely night. > > gcc/cp/coroutines.cc | 6 +++++ > gcc/testsuite/g++.dg/coroutines/pr111728.C | 29 ++++++++++++++++++++++ > 2 files changed, 35 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/coroutines/pr111728.C > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > index e8f028df3ad..fb8f24e6c61 100644 > --- a/gcc/cp/coroutines.cc > +++ b/gcc/cp/coroutines.cc > @@ -3755,6 +3755,12 @@ rewrite_param_uses (tree *stmt, int *do_subtree ATTRIBUTE_UNUSED, void *d) > return cp_walk_tree (&t, rewrite_param_uses, d, NULL); > } > > + if (unevaluated_p (TREE_CODE (*stmt))) > + { > + *do_subtree = 0; // Nothing to do. We tend to avoid C++-style comments; I'm not sure any comment is needed here, but "/* No odr-uses in unevaluated operands. */" would be clearer IMO. OK either way. Jason
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index e8f028df3ad..fb8f24e6c61 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -3755,6 +3755,12 @@ rewrite_param_uses (tree *stmt, int *do_subtree ATTRIBUTE_UNUSED, void *d) return cp_walk_tree (&t, rewrite_param_uses, d, NULL); } + if (unevaluated_p (TREE_CODE (*stmt))) + { + *do_subtree = 0; // Nothing to do. + return NULL_TREE; + } + if (TREE_CODE (*stmt) != PARM_DECL) return NULL_TREE; diff --git a/gcc/testsuite/g++.dg/coroutines/pr111728.C b/gcc/testsuite/g++.dg/coroutines/pr111728.C new file mode 100644 index 00000000000..c1fee4b36a1 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr111728.C @@ -0,0 +1,29 @@ +// { dg-do compile } +// https://gcc.gnu.org/PR111728 +#include <coroutine> +struct promise; +struct coroutine : std::coroutine_handle<promise> +{ + using promise_type = ::promise; + bool await_ready() { return false; } + void await_suspend(coroutine_handle h) {} + int await_resume() { return {} ;} +}; +struct promise +{ + coroutine get_return_object() { return {coroutine::from_promise(*this)}; } + std::suspend_always initial_suspend() noexcept { return {}; } + std::suspend_always final_suspend() noexcept { return {}; } + void return_void() {} + void unhandled_exception() {} +}; +coroutine +write_fields() { + int static_buffer[10]; + co_await [](auto) + -> coroutine + { + if (sizeof(static_buffer)); + co_return; + }(0); +}