On 11/17/19 5:25 AM, Iain Sandoe wrote:
+++ b/gcc/cp/coroutines.cc
+/* FIXME: minimise headers.. */
FIXED?
+/* DEBUG remove me. */
+extern void debug_tree (tree);
?
+static tree find_coro_traits_template_decl (location_t);
+static tree find_coro_handle_type (location_t, tree);
+static tree find_promise_type (tree);
+static tree
+lookup_promise_member (tree, const char *, location_t, bool);
bad line break?
+/* Lookup std::experimental. */
+static tree
+find_std_experimental (location_t loc)
Would this be better caching into a global_tree?
+{
+ /* we want std::experimental::coroutine_traits class template decl. */
... or is the template_decl the thing to cache? (Oh I see this comment
is incomplete as there's promise_type too).
+/* Lookup the coroutine_traits template decl.
+ Instantiate that for the function signature. */
+
+static tree
+find_coro_traits_template_decl (location_t kw)
+{
+
+ tree traits_name = get_identifier ("coroutine_traits");
+ tree traits_decl
+ = lookup_template_class (traits_name, targ,
+ /* in_decl */ NULL_TREE,
+ /* context */ exp_ns,
+ /* entering scope */ false, tf_none);
You should be able to pass the TEMPLATE_DECL into lookup_template_class.
So, yes cache the TEMPLATE_DECL std::experimental::coroutine_traits on
first lookup into a global tree.
+
+ if (traits_decl == error_mark_node)
+ {
+ error_at (kw, "couldn't instantiate coroutine_traits");
couldn't -> cannot (or something else non-apostrophey)
+static tree
+find_coro_handle_type (location_t kw, tree promise_type)
+{
+ tree exp_ns = find_std_experimental (kw);
+ if (!exp_ns)
+ return NULL_TREE;
+
+ /* So now build up a type list for the template, one entry, the promise. */
+ tree targ = make_tree_vec (1);
+ TREE_VEC_ELT (targ, 0) = promise_type;
+ tree handle_name = get_identifier ("coroutine_handle");
+ tree handle_type
+ = lookup_template_class (handle_name, targ,
As with the traits, cache the TEMPLATE_DECL.
+/* Look for the promise_type in the instantiated. */
+
+static tree
+find_promise_type (tree handle_type)
+{
+ tree promise_name = get_identifier ("promise_type");
could you add these identifiers to cp_global_trees? (use-once
identifiers fine not to, but there;s no point continually rehashing
multi-use ones).
+/* The state that we collect during parsing (and template expansion) for
+ a coroutine. */
+typedef struct coroutine_info
+{
+ tree promise_type;
+ tree handle_type;
+ tree self_h_proxy;
+ tree promise_proxy;
+ location_t first_coro_keyword;
+} coroutine_info_t;
typedef struct X{} X_t; is C-like. Please comment the fields though.
Doesn't this need GTYing? What keeps those trees alive across GC
otherwise? (try running with gc-always and see what happens?)
+/* These function assumes that the caller has verified that the state for
function->functions
+ the decl has been initialised, we try to minimise work here. */
IIUC american/oxford-english IZED?
+static tree
+get_coroutine_promise_type (tree decl)
+{
+ gcc_checking_assert (fn_to_coro_info);
+
+ coroutine_info_t *info = fn_to_coro_info->get (decl);
+ if (!info)
+ return NULL_TREE;
+ return info->promise_type;
idiomatic C++ would be
if (coroutine_info_t *info = ...)
return info->promis_type;
return NULL_TREE;
your call.
+/* Here we check the constraints that are common to all keywords (since the
+ presence of a coroutine keyword makes the function into a coroutine). */
+
+ /* This is arranged in order of prohibitions in the std. */
could you add a [clause.subname] reference maybe?
+ if (DECL_MAIN_P (fndecl))
+/* Here we will check the constraints that are not per keyword. */
will-> ""
+
+static bool
+coro_function_valid_p (tree fndecl)
+{
+ location_t f_loc = DECL_SOURCE_LOCATION (fndecl);
+
+ /* 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.
+ */
unfortunate line break?
+ gcc_assert (get_coroutine_handle_type (fndecl) != NULL_TREE
+ && get_coroutine_promise_type (fndecl) != NULL_TREE);
+
+ if (current_function_returns_value || current_function_returns_null)
+ /* TODO: record or extract positions of returns (and the first coro
+ keyword) so that we can add notes to the diagnostic about where
+ the bad keyword is and what made the function into a coro. */
+ error_at (f_loc, "return statement not allowed in coroutine;"
<%return%> not allowed ...?
+/* This performs [expr.await] bullet 3.3 and validates the interface obtained.
+ It is also used to build the initial and final suspend points.
+
+ A is the original await expr.
+ MODE:
+ 0 = regular function body co_await
+ 1 = await from a co_yield
+ 2 = initial await
+ 3 = final await.
+*/
Would MODE be better as an enum, from whence you cons up the INTEGER_CST
you need for the calls? I'll bet that makes calling code clearer. MODE
probably not a good name, given it's usual meaning of machine_mode.
+static tree
+build_co_await (location_t loc, tree a, tree mode)
+{
+ tree o_type = complete_type_or_else (TREE_TYPE (o), o);
+ if (TREE_CODE (o_type) != RECORD_TYPE)
+ {
+ error_at (loc,
+ "member reference base type %qT is not a"
+ " structure or union",
"member reference base type" sounds confusing to me.
+ /* To complete the lookups, we need an instance of 'e' which is built from
+ 'o' according to [expr.await] 3.4. However, we don't want to materialise
ise->ize?
+ /* The suspend method has constraints on its return type. */
hm, as 'constraint' is now a term of art wrt concepts, perhaps
'requirements'?
+ bool OK = false;
OK ->ok?
+ tree susp_return_type = TYPE_CANONICAL (TREE_TYPE (TREE_TYPE (awsp_func)));
Why are you looking at TYPE_CANONICAL, that seems fishy.
+ if (same_type_p (susp_return_type, void_type_node))
+ OK = true;
+ else if (same_type_p (susp_return_type, boolean_type_node))
+ OK = true;
+ else if (TREE_CODE (susp_return_type) == RECORD_TYPE)
+ /* TODO: this isn't enough of a test. */
What would complete it?
+ if (!OK)
+ {
+ fprintf (stderr, "didn't grok the suspend return : ");
+ debug_tree (susp_return_type);
what's going on here?
+tree
+finish_co_await_expr (location_t kw, tree expr)
+{
+
+ if (expr == NULL_TREE)
+ {
+ error_at (kw, "%<co_await%> requires an expression.");
isn't this a syntactic restriction? How can we get here without already
going wrong?
+ if (at_meth)
+ {
+ /* try to build a = p.await_transform (e). */
+ tree at_fn = NULL_TREE;
+ vec<tree, va_gc> *args = make_tree_vector_single (expr);
+ a = build_new_method_call (get_coroutine_promise_proxy (
+ current_function_decl),
+ at_meth, &args, NULL_TREE, LOOKUP_NORMAL,
+ &at_fn, tf_warning_or_error);
+
+ /* Probably it's not an error to fail here, although possibly a bit odd
+ to find await_transform but not a valid one? */
Is this comment still accurate? I don't think this is any kind of
SFINAE-like context, and we will have emitted an error.
+ /* Belt and braces, we should never get here, the expression should be
+ required in the parser. */
+ if (expr == NULL_TREE)
Then assert!
+/* placeholder; in case we really need something more than the contextual
+ checks. */
Is this still needed?
+static tree
+check_co_return_expr (tree retval, bool *no_warning)
+/* Check that it's valid to have a co_return keyword here.
+ If it is, then check and build the p.return_{void(),value(expr)}.
+ These are built against the promise proxy, but saved for expand time. */
+
+tree
+finish_co_return_stmt (location_t kw, tree expr)
+{
+ if (expr == error_mark_node)
+ return error_mark_node;
+
+ if (!coro_common_keyword_context_valid_p (current_function_decl, kw,
+ "co_return"))
+ return error_mark_node;
+
+ /* The current function has now become a coroutine, if it wasn't
+ already. */
+ DECL_COROUTINE_P (current_function_decl) = 1;
+
+ if (processing_template_decl)
+ {
+ current_function_returns_value = 1;
+
+ if (check_for_bare_parameter_packs (expr))
+ return error_mark_node;
+
+ tree functype = TREE_TYPE (TREE_TYPE (current_function_decl));
+ /* If we don't know the promise type, we can't proceed, return the
+ expression as it is. */
+ if (dependent_type_p (functype) || type_dependent_expression_p (expr))
+ {
+ expr
+ = build2_loc (kw, CO_RETRN_EXPR, void_type_node, expr, NULL_TREE);
+ expr = maybe_cleanup_point_expr_void (expr);
+ expr = add_stmt (expr);
+ return expr;
+ }
+ }
finish_co_{await,yield,return} all look very similar, or at least start
that way. Is there an underlying helper fn waiting?
+ /* If the promise object doesn't have the correct return call then
+ there's a mis-match between the co_return <expr> and this. */
+ tree co_ret_call = NULL_TREE;
+ if (expr == NULL_TREE || VOID_TYPE_P (TREE_TYPE (expr)))
+ {
+ tree crv_meth
+ = lookup_promise_member (current_function_decl, "return_void", kw,
+ true /*musthave*/);
idiom is /*musthave=*/true
+
+ expr = build2_loc (kw, CO_RETRN_EXPR, void_type_node, expr, co_ret_call);
oh, please name it CO_RETURN_EXPR
+/* ================= Morph and Expand. =================
I'll review more later ...
nathan
--
Nathan Sidwell
