On Mon, Jan 20, 2020 at 6:31 PM Iain Sandoe <i...@sandoe.co.uk> wrote:
>
> Hi Bin,
>
> Bin.Cheng <amker.ch...@gmail.com> wrote:
>
> > On Mon, Jan 20, 2020 at 5:28 PM Jakub Jelinek <ja...@redhat.com> wrote:
> >> On Mon, Jan 20, 2020 at 08:59:20AM +0000, Iain Sandoe wrote:
> >>> Hi Bin,
> >>>
> >>> bin.cheng <bin.ch...@linux.alibaba.com> wrote:
> >>>
> >>>> gcc/cp
> >>>> 2020-01-20 Bin Cheng <bin.li...@linux.alibaba.com>
> >>>>
> >>>> * coroutines.cc (build_co_await): Skip getting complete type for
> >>>> void.
> >>>>
> >>>> gcc/testsuite
> >>>> 2020-01-20 Bin Cheng <bin.li...@linux.alibaba.com>
> >>>>
> >>>> * g++.dg/coroutines/co-await-void_type.C: New
> >>>> test.<0001-Fix-ICE-when-co_awaiting-on-void-type.patch>
> >>>
> >>> This LGTM, (borderline obvious, in fact) but you will need to wait for
> >>> a C++
> >>> maintainer to approve,
> >>
> >> The patch actually looks wrong to me, there is nothing magic about void
> >> type here, it will ICE on any other incomplete type, because
> >> complete_type_or_else returns NULL if the type is incomplete.
> >>
> >> So, I think you want instead:
> >> tree o_type = complete_type_or_else (TREE_TYPE (o), o);
> >> + if (o_type == NULL_TREE)
> >> + return error_mark_node;
> >> if (TREE_CODE (o_type) != RECORD_TYPE)
> >>
> >> or similar (the diagnostics should be emitted already, so I don't see
> >> further point to emit it once again).
> >
> > Thanks very much for pointing out. I was trying to bypass generic
> > void error message to make it more related to coroutine, like:
> > e1.cc: In function ‘resumable foo(int)’:
> > e1.cc:45:3: error: awaitable type ‘void’ is not a structure
> > 45 | co_yield n + x + y;
> > | ^~~~~~~~
> >
> > vs.
> >
> > e1.cc: In function ‘resumable foo(int)’:
> > e1.cc:45:20: error: invalid use of ‘void’
> > 45 | co_yield n + x + y;
> > | ^
> >
> > Anyway I will update the patch.
>
> ...I had already made a start...
Thanks very much!
>
> The patch below gives the improved diagnostic while checking for NULL
> returns fom complete_type_or_else.
>
> OK?
> Iain
>
>
> gcc/cp
>
> 2020-01-20 Bin Cheng <bin.li...@linux.alibaba.com>
You can remove me from the ChangeLog or keep it as "bin.cheng" :-)
> Iain Sandoe <i...@sandoe.co.uk>
>
> * coroutines.cc (coro_promise_type_found_p): Check for NULL return
> from
> complete_type_or_else.
> (register_param_uses): Likewise.
> (build_co_await): Do not try to use complete_type_or_else for void
> types,
> otherwise for incomplete types, check for NULL return from
> complete_type_or_else.
>
> gcc/testsuite
> 2020-01-20 Bin Cheng <bin.li...@linux.alibaba.com>
>
> * g++.dg/coroutines/co-await-void_type.C: New
> test.<0001-Fix-ICE-when-co_awaiting-on-void-type.patch>
>
>
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index f0febfe0c8a..4e1ba81fb46 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -428,8 +428,9 @@ coro_promise_type_found_p (tree fndecl, location_t loc)
>
> /* Complete this, we're going to use it. */
> coro_info->handle_type = complete_type_or_else (handle_type, fndecl);
> +
> /* Diagnostic would be emitted by complete_type_or_else. */
> - if (coro_info->handle_type == error_mark_node)
> + if (!coro_info->handle_type)
> return false;
>
> /* Build a proxy for a handle to "self" as the param to
> @@ -651,8 +652,11 @@ build_co_await (location_t loc, tree a,
> suspend_point_kind suspend_kind)
> o = a; /* This is most likely about to fail anyway. */
>
> tree o_type = TREE_TYPE (o);
> - if (!VOID_TYPE_P (o_type))
> - o_type = complete_type_or_else (TREE_TYPE (o), o);
> + if (o_type && !VOID_TYPE_P (o_type) && !COMPLETE_TYPE_P (o_type))
IIUC, Jakub doesn't like checking void type specially here?
> + o_type = complete_type_or_else (o_type, o);
> +
> + if (!o_type)
> + return error_mark_node;
>
> if (TREE_CODE (o_type) != RECORD_TYPE)
> {
> @@ -2746,6 +2750,10 @@ register_param_uses (tree *stmt, int *do_subtree
> ATTRIBUTE_UNUSED, void *d)
> if (!COMPLETE_TYPE_P (actual_type))
> actual_type = complete_type_or_else (actual_type, *stmt);
>
> + if (actual_type == NULL_TREE)
> + /* Diagnostic emitted by complete_type_or_else. */
> + actual_type = error_mark_node;
> +
> if (TREE_CODE (actual_type) == REFERENCE_TYPE)
> actual_type = build_pointer_type (TREE_TYPE (actual_type));
>
>
Thanks,
bin
