[PATCH] coroutines: Add a cleanup expression for g-r-o when needed [PR95477].
Nathan Sidwell
nathan@acm.org
Mon Jun 8 19:23:08 GMT 2020
On 6/8/20 5:17 AM, Iain Sandoe wrote:
> Hi
>
> The PR reports that we fail to destroy the object initially created from
> the get-return-object call. Fixed by adding a cleanup when the DTOR is
> non-trivial. In addition, to meet the specific wording that the call to
> get_return_object creates the glvalue for the return, we must construct
> that in-place in the return object to avoid a second copy/move CTOR.
>
> tested on x86_64,powerpc64-linux, x86_64-darwin
> OK for master?
> 10.2?
> thanks
> Iain
>
> gcc/cp/ChangeLog:
>
> PR c++/95477
> * coroutines.cc (morph_fn_to_coro): Apply a cleanup to
> the get return object when the DTOR is non-trivial.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/coroutines/pr95477.C: New test.
> ---
> gcc/cp/coroutines.cc | 61 +++++++++++++++++++----
> gcc/testsuite/g++.dg/coroutines/pr95477.C | 37 ++++++++++++++
> 2 files changed, 89 insertions(+), 9 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95477.C
>
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index f2d7853477d..5a78bec1c9a 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -4284,12 +4284,34 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
>
> tree gro = NULL_TREE;
> tree gro_bind_vars = NULL_TREE;
> + tree gro_cleanup_stmt = NULL_TREE;
> /* We have to sequence the call to get_return_object before initial
> suspend. */
> if (gro_is_void_p)
> - finish_expr_stmt (get_ro);
> + r = get_ro;
> + else if (same_type_p (gro_type, fn_return_type))
> + {
> + /* [dcl.fct.def.coroutine] / 7
> + The expression promise.get_return_object() is used to initialize the
> + glvalue result or... (see below)
> + Construct the return result directly. */
> + if (TYPE_NEEDS_CONSTRUCTING (gro_type))
> + {
> + vec<tree, va_gc> *arg = make_tree_vector_single (get_ro);
> + r = build_special_member_call (DECL_RESULT (orig),
> + complete_ctor_identifier,
> + &arg, gro_type, LOOKUP_NORMAL,
> + tf_warning_or_error);
> + release_tree_vector (arg);
> + }
> + else
> + r = build2_loc (fn_start, INIT_EXPR, gro_type,
> + DECL_RESULT (orig), get_ro);
> + }
> else
> {
> + /* ... or ... Construct an object that will be used as the single
> + param to the CTOR for the return object. */
> gro = build_lang_decl (VAR_DECL, get_identifier ("coro.gro"), gro_type);
> DECL_CONTEXT (gro) = current_scope ();
> DECL_ARTIFICIAL (gro) = true;
> @@ -4306,8 +4328,21 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
> }
> else
> r = build2_loc (fn_start, INIT_EXPR, gro_type, gro, get_ro);
> - finish_expr_stmt (r);
> + /* The constructed object might require a cleanup. */
> + if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (gro_type))
> + {
> + tree cleanup
> + = build_special_member_call (gro, complete_dtor_identifier,
> + NULL, gro_type, LOOKUP_NORMAL,
> + tf_warning_or_error);
> + gro_cleanup_stmt = build_stmt (input_location, CLEANUP_STMT, NULL,
> + cleanup, gro);
> + }
> }
> + finish_expr_stmt (r);
> +
> + if (gro_cleanup_stmt)
> + CLEANUP_BODY (gro_cleanup_stmt) = push_stmt_list ();
>
> /* Initialize the resume_idx_name to 0, meaning "not started". */
> tree resume_idx_m
> @@ -4349,14 +4384,15 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
> promise was constructed. We now supply a reference to that var,
> either as the return value (if it's the same type) or to the CTOR
> for an object of the return type. */
> - if (gro_is_void_p)
> - r = NULL_TREE;
> - else
> - r = rvalue (gro);
>
> - if (!same_type_p (gro_type, fn_return_type))
> + if (same_type_p (gro_type, fn_return_type))
> + r = gro_is_void_p ? NULL_TREE : DECL_RESULT (orig);
> + else
> {
> - /* The return object is , even if the gro is void. */
> + /* If we have void gro and a non-class return type, then pick a
> + defensive initialisation value. */
> + r = gro_is_void_p ? integer_zero_node : rvalue (gro);
> + /* The return object is constructed, even if the gro is void. */
Would error_mark_node work here? I presume we've already diagnosed the
problem, so will result in no cascade of errors.
> if (CLASS_TYPE_P (fn_return_type))
> {
> vec<tree, va_gc> *args = NULL;
> @@ -4374,12 +4410,19 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
> if (args)
> release_tree_vector (args);
> }
> - else /* ??? suppose we have non-class return and void gro? */
> + else
> r = build1_loc (input_location, CONVERT_EXPR, fn_return_type, r);
> }
>
> finish_return_stmt (r);
>
> + if (gro_cleanup_stmt)
> + {
> + CLEANUP_BODY (gro_cleanup_stmt)
> + = pop_stmt_list (CLEANUP_BODY (gro_cleanup_stmt));
> + add_stmt (gro_cleanup_stmt);
> + }
> +
> /* Finish up the ramp function. */
> BIND_EXPR_VARS (gro_context_bind) = gro_bind_vars;
> BIND_EXPR_BODY (gro_context_bind) = pop_stmt_list (gro_context_body);
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr95477.C b/gcc/testsuite/g++.dg/coroutines/pr95477.C
> new file mode 100644
> index 00000000000..7050aee0078
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr95477.C
> @@ -0,0 +1,37 @@
> +// { dg-do run }
> +
> +#include "coro.h"
> +
> +struct simple {
> + static inline int alive = 0;
> + simple() { ++alive; }
> + simple(simple&&) { ++alive; }
> + ~simple() { --alive; }
> +
> + struct promise_type {
> + simple get_return_object() { return simple{}; }
> + void return_void() {}
> + void unhandled_exception() {}
> + auto initial_suspend() noexcept { return coro::suspend_never{}; }
> + auto final_suspend() noexcept { return coro::suspend_never{}; }
> + };
> +};
> +
> +simple
> +f()
> +{
> + co_return;
> +}
> +
> +int main() {
> + {
> + f();
> + }
> +
> + if (simple::alive != 0)
> + {
> + PRINTF ("something wrong with dtors: %d\n", simple::alive);
> + abort ();
> + }
> + return 0;
> +}
>
--
Nathan Sidwell
More information about the Gcc-patches
mailing list