Bin.Cheng <amker.ch...@gmail.com> wrote:
> 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" :-)
ack.
>> 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?
that is necessary if we want the coro-specific diagnostic (which is more
meaningful
than the vanilla void one, however, if there’s opposition to that, I don’t have
such a
strong opinion.
Iain
>> + 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
