Hi Adrian,
Again thanks for working on this and getting it into a suitable state for
posting.
It’s a good idea to CC relevant maintainer(s) on patches [see MAINTAINERS] and
I’d welcome cc on any coroutines ones (it’s easy to miss a patch otherwise).
> On 28 Nov 2022, at 11:55, Adrian Perl via Gcc-patches
> wrote:
> please have a look at the patch below for a potential fix that addresses the
> incorrect 'promotion'
> of members found in temporaries within co_await statements.
>
> To summarize my post in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99576#c5, the recursive
> promotion of temporaries is too eager and also recurses into constructor
> statements where it
> finds the initialization of members. This patch prevents the recursion into
> constructors and
> skips the remaining subtree.
>
> It fixes the issues in bug reports [PR99576, PR100611, PR101976, PR101367,
> PR107288] which are all
> related to incorrect 'promotions' (and manifest in too many destructor calls).
>
> I have added test applications based on examples in the PRs, which I have
> only slightly
> annotated and refactored to allow automatic testing. They are all basically
> quiet similar.
> The main difference is how the temporaries are used in the co_await (and
> co_yield) statements.
>
> Bootstrapping and running the testsuite on x86_64 was successfull. No
> regression occured.
This looks resonable to me, as said in the PR. I’d like to test a little wider
with some larger
codebases, if you could bear with me for a few days.
I think this patch is small enough to accept without any copyright machinery,
but (for reference and
in the hope that larger patches might be coming) .. at some stage, you’d need
to do one of:
1. get an FSF copyright assignment
2. release your patch under the DCO.
See: “Legal Prerequisites” here https://gcc.gnu.org/contribute.html.
thanks again for the patch
Iain
> Please let me know if you need more information.
>
>PR 100611
>PR 101367
>PR 101976
>PR 99576
>
>gcc/cp/ChangeLog:
>
>* coroutines.cc (find_interesting_subtree): Prevent recursion into
> constructor
>
>gcc/testsuite/ChangeLog:
>
>* g++.dg/coroutines/pr100611.C: New test.
>* g++.dg/coroutines/pr101367.C: New test.
>* g++.dg/coroutines/pr101976.C: New test.
>* g++.dg/coroutines/pr99576_1.C: New test.
>* g++.dg/coroutines/pr99576_2.C: New test.
>
> ---
> gcc/cp/coroutines.cc| 2 +
> gcc/testsuite/g++.dg/coroutines/pr100611.C | 93 +++
> gcc/testsuite/g++.dg/coroutines/pr101367.C | 78 +
> gcc/testsuite/g++.dg/coroutines/pr101976.C | 76
> gcc/testsuite/g++.dg/coroutines/pr99576_1.C | 123
> gcc/testsuite/g++.dg/coroutines/pr99576_2.C | 71 +++
> 6 files changed, 443 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr100611.C
> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr101367.C
> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr101976.C
> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr99576_1.C
> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr99576_2.C
>
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index 01a3e831ee5..a87ea7fe60a 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -2684,6 +2684,8 @@ find_interesting_subtree (tree *expr_p, int *dosub,
> void *d)
> return expr;
> }
> }
> + else if (TREE_CODE(expr) == CONSTRUCTOR)
> +*dosub = 0; /* We don't need to consider this any further. */
> else if (tmp_target_expr_p (expr)
> && !p->temps_used->contains (expr))
> {
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr100611.C
> b/gcc/testsuite/g++.dg/coroutines/pr100611.C
> new file mode 100644
> index 000..5fbcfa7e6ec
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr100611.C
> @@ -0,0 +1,93 @@
> +/*
> + Test that instances created in capture clauses within co_await statements
> do not
> + get 'promoted'. This would lead to the members destructor getting called
> more
> + than once.
> +
> + Correct output should look like:
> + Foo(23) 0xf042d8
> + Foo(const& 23) 0xf042ec
> + ~Foo(23) 0xf042ec
> + After co_await
> + ~Foo(23) 0xf042d8
> +*/
> +#include
> +#include
> +
> +static unsigned int struct_Foo_destructor_counter = 0;
> +static bool lambda_was_executed = false;
> +
> +class Task {
> +public:
> + struct promise_type {
> +Task get_return_object() {
> + return {std::coroutine_handle::from_promise(*this)};
> +}
> +
> +std::suspend_never initial_suspend() { return {}; }
> +std::suspend_always final_suspend() noexcept { return {}; }
> +void unhandled_exception() {}
> +void return_void() {}
> + };
> +
> + ~Task() {
> +if (handle_) {
> + handle_.destroy();
> +}
> + }
> +
> + bool await_ready() { return false; }
>