On Mon, Jan 20, 2020 at 10:59 PM Iain Sandoe <i...@sandoe.co.uk> wrote:
>
> Hi Bin,
>
> bin.cheng <bin.ch...@linux.alibaba.com> wrote:
>
> > By standard, coroutine body should be encapsulated in try-catch block
> > as following:
> > try {
> > // coroutine body
> > } catch(...) {
> > promise.unhandled_exception();
> > }
> > Given above try-catch block is implemented in the coroutine actor
> > function called by coroutine ramp function, so the promise should
> > be accessed via actor function's coroutine frame pointer argument,
> > rather than the ramp function's coroutine frame variable.
>
> thanks, good catch!
> it has not triggered for me (including on some more complex test-cases that
> are not useable
> in the testsuite).
>
> > This patch also refactored code to make the fix easy, unfortunately,
>
> see below,
>
> > I failed to reduce a test case from cpproro.
>
> it would be good if we could try to find a reproducer. I’m happy to try and
> throw
> creduce at a preprocessed file, if you have one.
>
> > gcc/cp
> > 2020-01-20 Bin Cheng <bin.li...@linux.alibaba.com>
> >
> > * coroutines.cc (act_des_fn, wrap_coroutine_body): New.
> > (morph_fn_to_coro): Call act_des_fn to build actor/destroy decls, as
> > well as access promise via actor function's frame pointer argument.
> > Refactor code into above functions.
> > (build_actor_fn, build_destroy_fn): Use frame pointer argument
> + /* We still try to look for the promise method and warn if it's not
> + present. */
> + tree ueh_meth
> + = lookup_promise_method (orig, coro_unhandled_exception_identifier,
> + loc, /*musthave=*/ false);
> + if (!ueh_meth || ueh_meth == error_mark_node)
> + warning_at (loc, 0, "no member named %qE in %qT",
> + coro_unhandled_exception_identifier,
> + get_coroutine_promise_type (orig));
> + }
> + /* Else we don't check and don't care if the method is missing. */
> +
> + return fnbody;
> +}
>
> IMO this ^^^ obfuscates the fix, and I don’t think it should be done at the
> same time.
Sure, given we are in this stage, I split the patch and leave refactoring to
future. IMHO, the function is too long so self-contained operations worth
outlined function even it's called once.Patch updated as attached. Thanks, bin gcc/cp 2020-01-20 Bin Cheng <bin.li...@linux.alibaba.com> * coroutines.cc (act_des_fn): New. (morph_fn_to_coro): Call act_des_fn to build actor/destroy decls. Access promise via actor function's frame pointer argument. (build_actor_fn, build_destroy_fn): Use frame pointer argument.
0001-Use-promise-in-coroutine-frame-in-actor-function.patch
Description: Binary data
