Hi,
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 (nor tolerating that level of fail for subsequent diagnostics).
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).
tested on x86_64-darwin16,
OK for trunk?
thanks
iain
gcc/cp/ChangeLog:
2020-01-28 Iain Sandoe <i...@sandoe.co.uk>
* coroutines.cc (get_coroutine_info): Tolerate fatal errors that might
leave the info unset.
(find_coro_traits_template_decl): Note we emitted the error and
suppress duplicates.
(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-28 Iain Sandoe <i...@sandoe.co.uk>
* g++.dg/coroutines/pr93458-1-missing-traits.C: New test.
* g++.dg/coroutines/pr93458-2-bad-coro-type.C: New test.
---
gcc/cp/coroutines.cc | 76 ++++++++++++++-----
.../coroutines/pr93458-1-missing-traits.C | 10 +++
.../coroutines/pr93458-2-bad-coro-type.C | 12 +++
3 files changed, 78 insertions(+), 20 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr93458-1-missing-traits.C
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr93458-2-bad-coro-type.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index e8a6a4033f6..ca86c7e6950 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -169,7 +169,8 @@ get_or_insert_coroutine_info (tree fn_decl)
coroutine_info *
get_coroutine_info (tree fn_decl)
{
- gcc_checking_assert (coroutine_info_table != NULL);
+ if (coroutine_info_table == NULL)
+ return NULL;
coroutine_info **slot = coroutine_info_table->find_slot_with_hash
(fn_decl, coroutine_info_hasher::hash (fn_decl), NO_INSERT);
@@ -255,11 +256,16 @@ static GTY(()) tree void_coro_handle_type;
static tree
find_coro_traits_template_decl (location_t kw)
{
+ static bool error_emitted = false;
tree traits_decl = lookup_qualified_name (std_node, coro_traits_identifier,
0, true);
- if (traits_decl == NULL_TREE || traits_decl == error_mark_node)
+ /* If we are missing fundmental information, such as the traits, then don't
+ emit this for every keyword in a TU. */
+ if (!error_emitted &&
+ (traits_decl == NULL_TREE || traits_decl == error_mark_node))
{
error_at (kw, "cannot find %<coroutine traits%> template");
+ error_emitted = true;
return NULL_TREE;
}
else
@@ -370,34 +376,45 @@ coro_promise_type_found_p (tree fndecl, location_t loc)
{
gcc_assert (fndecl != NULL_TREE);
- /* Save the coroutine data on the side to avoid the overhead on every
- function decl. */
-
- /* We only need one entry per coroutine in a TU, the assumption here is that
- there are typically not 1000s. */
if (!coro_initialized)
{
- gcc_checking_assert (coroutine_info_table == NULL);
- /* A table to hold the state, per coroutine decl. */
- coroutine_info_table =
- hash_table<coroutine_info_hasher>::create_ggc (11);
- /* Set up the identifiers we will use. */
- gcc_checking_assert (coro_traits_identifier == NULL);
+ /* Trees we only need to create once.
+ Set up the identifiers we will use. */
coro_init_identifiers ();
- /* Trees we only need to create once. */
+
/* 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;
+
/* coroutine_handle<> template. */
coro_handle_templ = find_coro_handle_template_decl (loc);
- gcc_checking_assert (coro_handle_templ != NULL);
+ if (coro_handle_templ == NULL_TREE
+ || coro_handle_templ == error_mark_node)
+ return false;
+
/* We can also instantiate the void coroutine_handle<> */
void_coro_handle_type =
instantiate_coro_handle_for_promise_type (loc, NULL_TREE);
- gcc_checking_assert (void_coro_handle_type != NULL);
+ if (void_coro_handle_type == NULL_TREE
+ || void_coro_handle_type == error_mark_node)
+ return false;
+
+ /* A table to hold the state, per coroutine decl. */
+ gcc_checking_assert (coroutine_info_table == NULL);
+ coroutine_info_table =
+ hash_table<coroutine_info_hasher>::create_ggc (11);
+
+ if (coroutine_info_table == NULL)
+ return false;
+
coro_initialized = true;
}
+ /* Save the coroutine data on the side to avoid the overhead on every
+ function decl tree. */
+
coroutine_info *coro_info = get_or_insert_coroutine_info (fndecl);
/* Without this, we cannot really proceed. */
gcc_checking_assert (coro_info);
@@ -407,6 +424,19 @@ coro_promise_type_found_p (tree fndecl, location_t loc)
{
/* Get the coroutine traits template class instance for the function
signature we have - coroutine_traits <R, ...> */
+ if (!CLASS_TYPE_P (TREE_TYPE (TREE_TYPE (fndecl))))
+ {
+ static bool coro_type_error_emitted = false;
+ /* It makes more sense to show the function header for this, even
+ though we will have encountered it when processing a keyword.
+ Only emit the error once, not for every keyword we encounter. */
+ if (!coro_type_error_emitted)
+ error_at (DECL_SOURCE_LOCATION (fndecl), "a coroutine must have a"
+ " class or struct return type");
+ coro_type_error_emitted = true;
+ return false;
+ }
+
tree templ_class = instantiate_coro_traits (fndecl, loc);
/* Find the promise type for that. */
@@ -422,7 +452,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)
return false;
/* Complete this, we're going to use it. */
@@ -597,11 +627,17 @@ coro_function_valid_p (tree fndecl)
{
location_t f_loc = DECL_SOURCE_LOCATION (fndecl);
+ /* For cases where fundamental information cannot be found, e.g. the
+ coroutine traits are missing, we need to punt early. */
+ if (!coro_promise_type_found_p (fndecl, f_loc))
+ return false;
+
/* Since we think the function is a coroutine, that implies we parsed
a keyword that triggered this. Keywords check promise validity for
their context and thus the promise type should be known at this point. */
- gcc_checking_assert (get_coroutine_handle_type (fndecl) != NULL_TREE
- && get_coroutine_promise_type (fndecl) != NULL_TREE);
+ if (get_coroutine_handle_type (fndecl) == NULL_TREE
+ || get_coroutine_promise_type (fndecl) == NULL_TREE)
+ return false;
if (current_function_returns_value || current_function_returns_null)
{
diff --git a/gcc/testsuite/g++.dg/coroutines/pr93458-1-missing-traits.C
b/gcc/testsuite/g++.dg/coroutines/pr93458-1-missing-traits.C
new file mode 100644
index 00000000000..46adccded98
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr93458-1-missing-traits.C
@@ -0,0 +1,10 @@
+// { dg-additional-options "-fsyntax-only -fexceptions -w" }
+
+// Diagose missing traits (fail to include <coroutine>).
+
+int
+bad_coroutine (void)
+{
+ co_yield 5; // { dg-error {cannot find 'coroutine traits' template} }
+ co_return;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr93458-2-bad-coro-type.C
b/gcc/testsuite/g++.dg/coroutines/pr93458-2-bad-coro-type.C
new file mode 100644
index 00000000000..aab9b34b69a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr93458-2-bad-coro-type.C
@@ -0,0 +1,12 @@
+// { dg-additional-options "-fsyntax-only -fexceptions -w" }
+
+// Diagose bad coroutine function type.
+
+#include "coro.h"
+
+int
+bad_coroutine (void) // { dg-error {a coroutine must have a class or struct
return type} }
+{
+ co_yield 5;
+ co_return;
+}
--
2.24.1
