Hi Nathan,
Nathan Sidwell wrote:
> On 1/29/20 10:39 AM, Iain Sandoe wrote:
>> Hi Nathan,
>> + /* If we are missing fundmental information, such as the traits, then
>> don't
>> + emit this for every keyword in a TU. This particular error is per TU
>> + so we don't need to keep the indicator per function. */
>> + static bool traits_error_emitted = false;
>
> You can of course move this into the if's block scope.
done.
>> + if (traits_decl == error_mark_node)
>> +{
>> + if (!traits_error_emitted)
>> +error_at (kw, "cannot find % template");
>
> Give the name you were looking for:
> "%<%E::%E%> ...", std_node, coro_traits_identifier
I had the “complain” flag on to the lookup so that it was emitting an error with
that information.
however. ….
> also, what if you find something, but it's not a type template?
… I’ve switched the complain off on lookup_qualified_name and now check for
a type template.
I took the liberty of repeating this treatment in the coroutine handle lookup
code
(new testcases attached).
so the errors now look like:
"cannot find a valid coroutine traits template using 'std::coroutine_traits’”
and
"cannot find a valid coroutine handle class template using
'std::coroutine_handle’”
.. but open to tweaking them if there could be bette wording.
>>/* Coroutine traits template. */
>>coro_traits_templ = find_coro_traits_template_decl (loc);
>> - gcc_checking_assert (coro_traits_templ != NULL);
>> + if (coro_traits_templ == NULL_TREE
>> + || coro_traits_templ == error_mark_node)
>> +return false;
>
> ISTM that find_coro_traits_template_decl should be returning exactly one of
> NULL_TREE of error_mark_node on failure. Its users don't particularly care
> why it failed (not found vs found ambiguous/not template).
fair enough, settled for NULL_TREE.
> + /* Save the coroutine data on the side to avoid the overhead on every
>> + if (!coro_info->coro_ret_type_error_emitted)
>> +error_at (DECL_SOURCE_LOCATION (fndecl), "a coroutine must have a"
>> + " class or struct return type");
>
> Perhaps something like "coroutine return type %qT is not a class"? I.e. show
> them the type.
> (structs are classes, there's no need to say 'class or struct’)
yes, that’s nicer, done.
>> + coro_info->coro_ret_type_error_emitted = true;
>> + return false;
>> +}
>> +
>>tree templ_class = instantiate_coro_traits (fndecl, loc);
>> /* Find the promise type for that. */
>> @@ -422,7 +454,7 @@ coro_promise_type_found_p (tree fndecl, location_t loc)
>>/* Try to find the handle type for the promise. */
>>tree handle_type =
>> instantiate_coro_handle_for_promise_type (loc, coro_info->promise_type);
>> - if (handle_type == NULL_TREE)
>> + if (handle_type == NULL_TREE || handle_type == error_mark_node)
>
> similar to coro_traits_template_decl.
gave this a similar treatment.
As below with those changes and additional test cases,
OK / tweak error messages?
thanks
Iain.
Since coroutine-ness is discovered lazily, we encounter the diagnostics
during each keyword parse. We were not handling the case where a user code
failed to include fundamental information (e.g. the traits) in a graceful
manner.
Once we've emitted an error for this level of fail, then we suppress
additional copies (otherwise the same thing will be reported for every
coroutine keyword seen).
gcc/cp/ChangeLog:
2020-01-30 Iain Sandoe
* coroutines.cc (struct coroutine_info): Add a bool flag to note
that we emitted an error for a bad function return type.
(get_coroutine_info): Tolerate an unset info table in case of
missing traits.
(find_coro_traits_template_decl): In case of error or if we didn't
find a type template, note we emitted the error and suppress
duplicates.
(find_coro_handle_template_decl): Likewise.
(instantiate_coro_traits): Only check for error_mark_node in the
return from lookup_qualified_name.
(coro_promise_type_found_p): Reorder initialization so that we check
for the traits and their usability before allocation of the info
table. Check for a suitable return type and emit a diagnostic for
here instead of relying on the lookup machinery. This allows the
error to have a better location, and means we can suppress multiple
copies.
(coro_function_valid_p): Re-check for a valid promise (and thus the
traits) before proceeding. Tolerate missing info as a fatal error.
gcc/testsuite/ChangeLog:
2020-01-30 Iain Sandoe
* g++.dg/coroutines/pr93458-1-missing-traits.C: New test.
* g++.dg/coroutines/pr93458-2-bad-traits.C: New test.
* g++.dg/coroutines/pr93458-3-missing-handle.C: New test.
* g++.dg/coroutines/pr93458-4-bad-coro-handle.C: New test.
* g++.dg/coroutines/pr93458-5-bad-coro-type.C:
