Hi Patrick
> On 13 Aug 2024, at 03:01, Patrick Palka <[email protected]> wrote:
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk/14?
>
> -- >8 --
>
> In r15-2210 we got rid of the unnecessary cast to lvalue reference when
> passing *this to the promise type ctor, and as a drive-by change we also
> simplified the code to use cp_build_fold_indirect_ref.
>
> But cp_build_fold_indirect_ref apparently does too much here, namely
> it has a shortcut for returning current_class_ref if the operand is
> current_class_ptr. The problem with that shortcut is current_class_ref
> might have gotten clobbered earlier if it appeared in the function body,
> since rewrite_param_uses walks and rewrites in-place all local variable
> uses to their corresponding frame copy.
>
> So later this cp_build_fold_indirect_ref for *__closure will instead return
> the mutated current_class_ref i.e. *frame_ptr->__closure, which doesn't
> make sense here since we're in the ramp function and not the actor function
> where frame_ptr is in scope.
>
> This patch fixes this by building INDIRECT_REF directly instead of using
> cp_build_fold_indirect_ref. (Another approach might be to restore an
> unshare_expr'd current_class_ref after doing coro_rewrite_function_body
> to avoid it remaining clobbered after the rewriting process. Yet
> another more ambitious approach might be to avoid this tree sharing in
> the first place by returning unshared versions of current_class_ref from
> maybe_dummy_object etc.)
This actually fixes a regression in a local test so thanks. I tested with folly
too - no changes there,
Iain
>
> PR c++/116327
> PR c++/104981
> PR c++/115550
>
> gcc/cp/ChangeLog:
>
> * coroutines.cc (morph_fn_to_coro): Build INDIRECT_REF directly
> instead of using cp_build_fold_indirect_ref when doing *this.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/coroutines/pr116327-preview-this.C: New test.
> ---
> gcc/cp/coroutines.cc | 6 +++--
> .../g++.dg/coroutines/pr116327-preview-this.C | 22 +++++++++++++++++++
> 2 files changed, 26 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr116327-preview-this.C
>
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index 145ec4b1d16..58be456ce00 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -4850,7 +4850,8 @@ morph_fn_to_coro (tree orig, tree *resumer, tree
> *destroyer)
> if (parm_i->this_ptr || parm_i->lambda_cobj)
> {
> /* We pass a reference to *this to the allocator lookup. */
> - tree this_ref = cp_build_fold_indirect_ref (arg);
> + tree tt = TREE_TYPE (TREE_TYPE (arg));
> + tree this_ref = build1 (INDIRECT_REF, tt, arg);
> vec_safe_push (args, this_ref);
> }
> else
> @@ -5070,7 +5071,8 @@ morph_fn_to_coro (tree orig, tree *resumer, tree
> *destroyer)
> if (parm.this_ptr || parm.lambda_cobj)
> {
> /* We pass a reference to *this to the param preview. */
> - tree this_ref = cp_build_fold_indirect_ref (arg);
> + tree tt = TREE_TYPE (TREE_TYPE (arg));
> + tree this_ref = build1 (INDIRECT_REF, tt, arg);
> vec_safe_push (promise_args, this_ref);
> }
> else if (parm.rv_ref)
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr116327-preview-this.C
> b/gcc/testsuite/g++.dg/coroutines/pr116327-preview-this.C
> new file mode 100644
> index 00000000000..27b69a41392
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr116327-preview-this.C
> @@ -0,0 +1,22 @@
> +// PR c++/116327 - ICE in coroutine with parameter preview on lambda with
> captures
> +
> +#include <coroutine>
> +
> +struct coroutine{
> + struct promise_type{
> + promise_type(const auto &...){}
> + std::suspend_never initial_suspend(){ return {}; }
> + std::suspend_always final_suspend()noexcept{ return {}; }
> + void unhandled_exception(){}
> + coroutine get_return_object(){ return {}; }
> + void return_value(int)noexcept{}
> + };
> +};
> +
> +int main(){
> + auto f = [a=0](auto) -> coroutine {
> + co_return 2;
> + };
> + f(0);
> + return 0;
> +}
> --
> 2.46.0.77.g25673b1c47
>
