[gcc r15-3492] c++, coroutines: Revise promise construction/destruction.

Iain D Sandoe iains@gcc.gnu.org
Thu Sep 5 19:20:07 GMT 2024


https://gcc.gnu.org/g:7d1483921941d21d91f929ef0d59a9794b1946b4

commit r15-3492-g7d1483921941d21d91f929ef0d59a9794b1946b4
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Sat Aug 31 13:08:42 2024 +0100

    c++, coroutines: Revise promise construction/destruction.
    
    In examining the coroutine testcases for unexpected diagnostic output
    for 'Wall', I found a 'statement has no effect' warning for the promise
    construction in one case.  In particular, the case is where the users
    promise type has an implicit CTOR but a user-provided DTOR. Further, the
    type does not actually need constructing.
    
    In very early versions of the coroutines code we used to check
    TYPE_NEEDS_CONSTRUCTING() to determine whether to attempt to build
    a constructor call for the promise.  During review, it was suggested
    to use type_build_ctor_call () instead.
    
    This latter call checks the constructors in the type (both user-defined
    and implicit) and returns true, amongst other cases if any of the found
    CTORs are marked as deprecated.
    
    In a number of places (for example [class.copy.ctor] / 6) the standard
    says that some version of an implicit CTOR is deprecated when the user
    provides a DTOR.
    
    Thus, for this specific arrangement of promise type, type_build_ctor_call
    returns true, because of (for example) a deprecated implicit copy CTOR.
    
    We are not going to use any of the deprecated CTORs and thus will not
    see warnings from this - however, since the call returned true, we have
    now determined that we should attempt to build a constructor call.
    
    Note as above, the type does not actually require construction and thus
    one might expect either a NULL_TREE or error_mark_node in response to
    the build_special_member_call ().  However, in practice the function
    returns the original instance object instead of a call or some error.
    
    When we add that as a statement it triggers the 'statement has no effect'
    warning.
    
    The patch here rearranges the promise construction/destruction code to
    allow for the case that a DTOR is required independently of a CTOR. In
    addition, we check that the return from build_special_member_call ()
    has side effects before we add it as a statement.
    
    gcc/cp/ChangeLog:
    
            * coroutines.cc
            (cp_coroutine_transform::build_ramp_function): Separate the
            build of promise constructor and destructor.  When evaluating
            the constructor, check that build_special_member_call returns
            an expression with side effects before adding it.
    
    Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

Diff:
---
 gcc/cp/coroutines.cc | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index e605eaec7a4..d6acf09326f 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -4896,16 +4896,12 @@ cp_coroutine_transform::build_ramp_function ()
   tree p = build_class_member_access_expr (deref_fp, promise_m, NULL_TREE,
 					   false, tf_warning_or_error);
 
-  tree promise_dtor = NULL_TREE;
   if (type_build_ctor_call (promise_type))
     {
-      /* Do a placement new constructor for the promise type (we never call
-	 the new operator, just the constructor on the object in place in the
-	 frame).
+      /* Construct the promise object [dcl.fct.def.coroutine] / 5.7.
 
-	 First try to find a constructor with the same parameter list as the
-	 original function (if it has params), failing that find a constructor
-	 with no parameter list.  */
+	 First try to find a constructor with an argument list comprised of
+	 the parameter copies.  */
 
       if (DECL_ARGUMENTS (orig_fn_decl))
 	{
@@ -4917,19 +4913,27 @@ cp_coroutine_transform::build_ramp_function ()
       else
 	r = NULL_TREE;
 
+      /* If that fails then the promise constructor argument list is empty.  */
       if (r == NULL_TREE || r == error_mark_node)
 	r = build_special_member_call (p, complete_ctor_identifier, NULL,
 				       promise_type, LOOKUP_NORMAL,
 				       tf_warning_or_error);
 
-      finish_expr_stmt (r);
+      /* If type_build_ctor_call() encounters deprecated implicit CTORs it will
+	 return true, and therefore we will execute this code path.  However,
+	 we might well not actually require a CTOR and under those conditions
+	 the build call above will not return a call expression, but the
+	 original instance object.  Do not attempt to add the statement unless
+	 it has side-effects.  */
+      if (r && r != error_mark_node && TREE_SIDE_EFFECTS (r))
+	finish_expr_stmt (r);
+    }
 
-      r = build_modify_expr (loc, coro_promise_live, boolean_type_node,
-			     INIT_EXPR, loc, boolean_true_node,
-			     boolean_type_node);
+  tree promise_dtor = cxx_maybe_build_cleanup (p, tf_warning_or_error);;
+  if (flag_exceptions && promise_dtor)
+    {
+      r = cp_build_init_expr (coro_promise_live, boolean_true_node);
       finish_expr_stmt (r);
-
-      promise_dtor = cxx_maybe_build_cleanup (p, tf_warning_or_error);
     }
 
   tree get_ro


More information about the Gcc-cvs mailing list