c++, coroutines, contracts: Handle coroutine and void functions [PR110871, PR110872, PR115434].

This patch came out of a discussion on Mattermost about how to deal
with contracts/coroutines integration.  Actually, it would also allow
some semantic checking to be deferred until the same spot - at which
time there are no dependent types, which can simplify the process.

NOTE: this is a fix for bugs in the existing '2a' contracts impl. it
does not attempt to make any of the changes required by P2900 to 
either code-gen or constexpr handling.

Tested on x86_64-darwin, so far, OK for trunk if testing succeeds on
x86_64/powerpc64 linux too?
thanks,
Iain

--- 8< ---

The current implementation of contracts emits the checks into function
bodies in three places; for pre-conditions at the start of the body,
for asserts in-line in the function body and for post-conditions as an
addition to return statements.

In general (at least with existing "2a" contract semantics) the in-line
contract asserts behave as expected.

However, the mechanism is not applicable to:

 * Handling pre conditions in coroutines since, for those, the standard
  specifies a wrapping of the original function body by functionality
  implementing initial and final suspends (along with some housekeeping
  to route exceptions).  Thus for such transformed function bodies, the
  preconditions then get actioned after the initial suspend, which does
  not behave as intended.

  * Handling post conditions in functions that do not have return
    statements (which applies to coroutines and void functions).

In the following, we identify a potentially transformed function body
(in the case of coroutines, this is usually called the "ramp()" function).

The patch here re-implements the code insertion in one of the two following
ways (code for exposition only):

  * For functions with no post-conditions we wrap the potentially
    transformed function as follows:

  {
     handle_pre_condition_checking ();
     potentially_transformed_function_body ();
  }

  This implements the intent that the preconditions are processed after
  the function parameters are initialised but before any other actions.

  * For functions with post-conditions:

  try
   {
     if (preconditions_exist)
       handle_pre_condition_checking ();
     potentially_transformed_function_body ();
   }
  finally
   {
     handle_post_condition_checking ();
   }
  else [only if the function is not marked noexcept(true) ]
   {
     __rethrow ();
   }

In this, post-conditions [that might apply to the return value etc.]
are evaluated on every non-exceptional edge out of the function.

At present, the model here is that exceptions thrown by the function
propagate upwards as if there were no contracts present.  If the desired
semantic becomes that an exception is counted as equivalent to a contract
violation - then we can add a second handler in place of the rethrow.

At constexpr time we need to evaluate the contract conditions, but not
the exceptional path, which is handled by a flag on the EH_ELSE_EXPR that
indicates it is in use for contract handling.

This patch specifically does not address changes to code-gen and constexpr
handling that are contained in P2900.

	PR c++/115434
	PR c++/110871
	PR c++/110872

gcc/cp/ChangeLog:

	* constexpr.cc (cxx_eval_constant_expression): Handle EH_ELSE_EXPR.
	* contracts.cc (finish_contract_attribute): Remove excess line.
	(build_contract_condition_function): Post condition handlers are
	void now.
	(emit_postconditions_cleanup): Remove.
	(emit_postconditions): New.
	(add_pre_condition_fn_call): New.
	(add_post_condition_fn_call): New.
	(apply_preconditions): New.
	(apply_postconditions): New.
	(maybe_apply_function_contracts): New.
	(apply_postcondition_to_return): Remove.
	* contracts.h (apply_postcondition_to_return): Remove.
	(maybe_apply_function_contracts): Add.
	* coroutines.cc (coro_build_actor_or_destroy_function): Do not
	copy contracts to coroutine helpers.
	* cp-tree.h (CONTRACT_EH_ELSE_P): New.
	* decl.cc (finish_function): Handle wrapping a possibly
	transformed function body in contract checks.
	* typeck.cc (check_return_expr): Remove handling of post
	conditions on return expressions.

gcc/ChangeLog:

	* gimplify.cc (struct gimplify_ctx): Add a flag to show we are
	expending a handler.
	(gimplify_expr): When we are expanding a handler, and the body
	transforms might have re-written DECL_RESULT into a gimple var,
	ensure that hander references to DECL_RESULT are also re-written
	to refer to the gimple var.

gcc/testsuite/ChangeLog:

	* g++.dg/contracts/pr115434.C: New test.
	* g++.dg/coroutines/pr110871.C: New test.
	* g++.dg/coroutines/pr110872.C: New test.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
---
 gcc/cp/constexpr.cc                        |  16 ++
 gcc/cp/contracts.cc                        | 249 ++++++++++++---------
 gcc/cp/contracts.h                         |   3 +-
 gcc/cp/coroutines.cc                       |   2 +
 gcc/cp/cp-tree.h                           |   3 +
 gcc/cp/decl.cc                             |  23 +-
 gcc/cp/typeck.cc                           |  13 +-
 gcc/gimplify.cc                            |  13 +-
 gcc/testsuite/g++.dg/contracts/pr115434.C  |  16 ++
 gcc/testsuite/g++.dg/coroutines/pr110871.C |  62 +++++
 gcc/testsuite/g++.dg/coroutines/pr110872.C |  49 ++++
 11 files changed, 319 insertions(+), 130 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/contracts/pr115434.C
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr110871.C
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr110872.C

> On 17 Jun 2024, at 13:15, Iain Sandoe <iains.gcc@gmail.com> wrote:
> 
> This patch came out of a discussion on Mattermost about how to deal
> with contracts/coroutines integration.  Actually, it would also allow
> some semantic checking to be deferred until the same spot - at which
> time there are no dependent types, which can simplify the process.
> 
> NOTE: this is a fix for bugs in the existing '2a' contracts impl. it
> does not attempt to make any of the changes required by P2900 to 
> either code-gen or constexpr handling.
> 
> Tested on x86_64-darwin, so far, OK for trunk if testing succeeds on
> x86_64/powerpc64 linux too?

Testing was sucessful and these patches have been on the contracts-noattr
compiler explorer instance for a while now.

OK for trunk?
thanks
Iain

> thanks,
> Iain
> 
> --- 8< ---
> 
> The current implementation of contracts emits the checks into function
> bodies in three places; for pre-conditions at the start of the body,
> for asserts in-line in the function body and for post-conditions as an
> addition to return statements.
> 
> In general (at least with existing "2a" contract semantics) the in-line
> contract asserts behave as expected.
> 
> However, the mechanism is not applicable to:
> 
> * Handling pre conditions in coroutines since, for those, the standard
>  specifies a wrapping of the original function body by functionality
>  implementing initial and final suspends (along with some housekeeping
>  to route exceptions).  Thus for such transformed function bodies, the
>  preconditions then get actioned after the initial suspend, which does
>  not behave as intended.
> 
>  * Handling post conditions in functions that do not have return
>    statements (which applies to coroutines and void functions).
> 
> In the following, we identify a potentially transformed function body
> (in the case of coroutines, this is usually called the "ramp()" function).
> 
> The patch here re-implements the code insertion in one of the two following
> ways (code for exposition only):
> 
>  * For functions with no post-conditions we wrap the potentially
>    transformed function as follows:
> 
>  {
>     handle_pre_condition_checking ();
>     potentially_transformed_function_body ();
>  }
> 
>  This implements the intent that the preconditions are processed after
>  the function parameters are initialised but before any other actions.
> 
>  * For functions with post-conditions:
> 
>  try
>   {
>     if (preconditions_exist)
>       handle_pre_condition_checking ();
>     potentially_transformed_function_body ();
>   }
>  finally
>   {
>     handle_post_condition_checking ();
>   }
>  else [only if the function is not marked noexcept(true) ]
>   {
>     __rethrow ();
>   }
> 
> In this, post-conditions [that might apply to the return value etc.]
> are evaluated on every non-exceptional edge out of the function.
> 
> At present, the model here is that exceptions thrown by the function
> propagate upwards as if there were no contracts present.  If the desired
> semantic becomes that an exception is counted as equivalent to a contract
> violation - then we can add a second handler in place of the rethrow.
> 
> At constexpr time we need to evaluate the contract conditions, but not
> the exceptional path, which is handled by a flag on the EH_ELSE_EXPR that
> indicates it is in use for contract handling.
> 
> This patch specifically does not address changes to code-gen and constexpr
> handling that are contained in P2900.
> 
> 	PR c++/115434
> 	PR c++/110871
> 	PR c++/110872
> 
> gcc/cp/ChangeLog:
> 
> 	* constexpr.cc (cxx_eval_constant_expression): Handle EH_ELSE_EXPR.
> 	* contracts.cc (finish_contract_attribute): Remove excess line.
> 	(build_contract_condition_function): Post condition handlers are
> 	void now.
> 	(emit_postconditions_cleanup): Remove.
> 	(emit_postconditions): New.
> 	(add_pre_condition_fn_call): New.
> 	(add_post_condition_fn_call): New.
> 	(apply_preconditions): New.
> 	(apply_postconditions): New.
> 	(maybe_apply_function_contracts): New.
> 	(apply_postcondition_to_return): Remove.
> 	* contracts.h (apply_postcondition_to_return): Remove.
> 	(maybe_apply_function_contracts): Add.
> 	* coroutines.cc (coro_build_actor_or_destroy_function): Do not
> 	copy contracts to coroutine helpers.
> 	* cp-tree.h (CONTRACT_EH_ELSE_P): New.
> 	* decl.cc (finish_function): Handle wrapping a possibly
> 	transformed function body in contract checks.
> 	* typeck.cc (check_return_expr): Remove handling of post
> 	conditions on return expressions.
> 
> gcc/ChangeLog:
> 
> 	* gimplify.cc (struct gimplify_ctx): Add a flag to show we are
> 	expending a handler.
> 	(gimplify_expr): When we are expanding a handler, and the body
> 	transforms might have re-written DECL_RESULT into a gimple var,
> 	ensure that hander references to DECL_RESULT are also re-written
> 	to refer to the gimple var.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/contracts/pr115434.C: New test.
> 	* g++.dg/coroutines/pr110871.C: New test.
> 	* g++.dg/coroutines/pr110872.C: New test.
> 
> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
> ---
> gcc/cp/constexpr.cc                        |  16 ++
> gcc/cp/contracts.cc                        | 249 ++++++++++++---------
> gcc/cp/contracts.h                         |   3 +-
> gcc/cp/coroutines.cc                       |   2 +
> gcc/cp/cp-tree.h                           |   3 +
> gcc/cp/decl.cc                             |  23 +-
> gcc/cp/typeck.cc                           |  13 +-
> gcc/gimplify.cc                            |  13 +-
> gcc/testsuite/g++.dg/contracts/pr115434.C  |  16 ++
> gcc/testsuite/g++.dg/coroutines/pr110871.C |  62 +++++
> gcc/testsuite/g++.dg/coroutines/pr110872.C |  49 ++++
> 11 files changed, 319 insertions(+), 130 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/contracts/pr115434.C
> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr110871.C
> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr110872.C
> 
> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> index bd72533491e..9b66b1753ad 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -7808,6 +7808,22 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
> 				      non_constant_p, overflow_p);
>       break;
> 
> +    case EH_ELSE_EXPR:
> +      /* Evaluate any cleanup that applies to non-EH exits, this only for
> +	 the output of the diagnostics ??? what is really meant to happen
> +	 at constexpr-time?...  */
> +      cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0), vc_discard,
> +				      non_constant_p, overflow_p);
> +
> +      /* The presence of a contract should not affect the constexpr.  */
> +      if (CONTRACT_EH_ELSE_P (t))
> +	break;
> +      if (!*non_constant_p)
> +	/* Also evaluate the EH handler.  */
> +	cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 1), vc_discard,
> +				      non_constant_p, overflow_p);
> +      break;
> +
>     case CLEANUP_STMT:
>       r = cxx_eval_constant_expression (ctx, CLEANUP_BODY (t), lval,
> 					non_constant_p, overflow_p,
> diff --git a/gcc/cp/contracts.cc b/gcc/cp/contracts.cc
> index 0822624a910..064f0b6a9d8 100644
> --- a/gcc/cp/contracts.cc
> +++ b/gcc/cp/contracts.cc
> @@ -88,64 +88,7 @@ along with GCC; see the file COPYING3.  If not see
>        return -v;
>      }
> 
> -   The original decl is left alone and instead calls are generated to pre/post
> -   functions within the body:
> -
> -     void fun.pre(int v)
> -     {
> -       [[ assert: v > 0 ]];
> -     }
> -     int fun.post(int v, int __r)
> -     {
> -       [[ assert: __r < 0 ]];
> -       return __r;
> -     }
> -     int fun(int v)
> -     {
> -       fun.pre(v);
> -       return fun.post(v, -v);
> -     }
> -
> -   If fun returns in memory, the return value is not passed through the post
> -   function; instead, the return object is initialized directly and then passed
> -   to the post function by invisible reference.
> -
> -   This sides steps a number of issues with having to rewrite the bodies or
> -   rewrite the parsed conditions as the parameters to the original function
> -   changes (as happens during redeclaration). The ultimate goal is to get
> -   something that optimizes well along the lines of
> -
> -     int fun(int v)
> -     {
> -       [[ assert: v > 0 ]];
> -       auto &&__r = -v;
> -       goto out;
> -     out:
> -       [[ assert: __r < 0 ]];
> -       return __r;
> -     }
> -
> -   With the idea being that multiple return statements could collapse the
> -   function epilogue after inlining the pre/post functions. clang is able
> -   to collapse common function epilogues, while gcc needs -O3 -Os combined.
> -
> -   Directly laying the pre contracts down in the function body doesn't have
> -   many issues. The post contracts may need to be repeated multiple times, once
> -   for each return, or a goto epilogue would need to be generated.
> -   For this initial implementation, generating function calls and letting
> -   later optimizations decide whether to inline and duplicate the actual
> -   checks or whether to collapse the shared epilogue was chosen.
> -
> -   For cdtors a post contract is implemented using a CLEANUP_STMT.
> -
> -   FIXME the compiler already shores cleanup code on multiple exit paths, so
> -   this outlining seems unnecessary if we represent the postcondition as a
> -   cleanup for all functions.
> -
> -   More helpful for optimization might be to make the contracts a wrapper
> -   function (for non-variadic functions), that could be inlined into a
> -   caller while preserving the call to the actual function?  Either that or
> -   mirror a never-continue post contract with an assume in the caller.  */
> +  TODO: reiterate the revised implementation.  */
> 
> #include "config.h"
> #include "system.h"
> @@ -801,7 +744,6 @@ finish_contract_attribute (tree identifier, tree contract)
>   tree attribute = build_tree_list (build_tree_list (NULL_TREE, identifier),
> 				    build_tree_list (NULL_TREE, contract));
> 
> -
>   /* Mark the attribute as dependent if the condition is dependent.
> 
>      TODO: I'm not sure this is strictly necessary. It's going to be marked as
> @@ -1444,10 +1386,8 @@ build_contract_condition_function (tree fndecl, bool pre)
>       *last = build_tree_list (NULL_TREE, value_type);
>       TREE_CHAIN (*last) = void_list_node;
> 
> -      if (aggregate_value_p (value_type, fndecl))
> -	/* If FNDECL returns in memory, don't return the value from the
> -	   postcondition.  */
> -	value_type = void_type_node;
> +      /* The handler is a void return.  */
> +      value_type = void_type_node;
>     }
> 
>   TREE_TYPE (fn) = build_function_type (value_type, arg_types);
> @@ -1882,15 +1822,12 @@ emit_preconditions (tree attr)
>   return emit_contract_conditions (attr, PRECONDITION_STMT);
> }
> 
> -/* Emit statements for postcondition attributes.  */
> +/* Emit statements for precondition attributes.  */
> 
> static void
> -emit_postconditions_cleanup (tree contracts)
> +emit_postconditions (tree attr)
> {
> -  tree stmts = push_stmt_list ();
> -  emit_contract_conditions (contracts, POSTCONDITION_STMT);
> -  stmts = pop_stmt_list (stmts);
> -  push_cleanup (NULL_TREE, stmts, /*eh_only*/false);
> +  return emit_contract_conditions (attr, POSTCONDITION_STMT);
> }
> 
> /* We're compiling the pre/postcondition function CONDFN; remap any FN
> @@ -1993,32 +1930,152 @@ start_function_contracts (tree decl1)
>   if (!handle_contracts_p (decl1))
>     return;
> 
> +  /* For cdtors, we evaluate the contracts check inline.  */
>   if (!outline_contracts_p (decl1))
> -    {
> -      emit_preconditions (DECL_CONTRACTS (current_function_decl));
> -      emit_postconditions_cleanup (DECL_CONTRACTS (current_function_decl));
> -      return;
> -    }
> +    return;
> 
>   /* Contracts may have just been added without a chance to parse them, though
>      we still need the PRE_FN available to generate a call to it.  */
>   if (!DECL_PRE_FN (decl1))
>     build_contract_function_decls (decl1);
> 
> +}
> +
> +/* If we have a precondition function and it's valid, call it.  */
> +
> +static void
> +add_pre_condition_fn_call (tree fndecl)
> +{
>   /* If we're starting a guarded function with valid contracts, we need to
>      insert a call to the pre function.  */
> -  if (DECL_PRE_FN (decl1)
> -      && DECL_PRE_FN (decl1) != error_mark_node)
> +  gcc_checking_assert (DECL_PRE_FN (fndecl)
> +      && DECL_PRE_FN (fndecl) != error_mark_node);
> +
> +  releasing_vec args = build_arg_list (fndecl);
> +  tree call = build_call_a (DECL_PRE_FN (fndecl), args->length (),
> +			    args->address ());
> +  CALL_FROM_THUNK_P (call) = true;
> +  finish_expr_stmt (call);
> +}
> +
> +/* Build and add a call to the post-condition checking function, when that
> +   is in use.  */
> +
> +static void
> +add_post_condition_fn_call (tree fndecl)
> +{
> +  gcc_checking_assert (DECL_POST_FN (fndecl)
> +      && DECL_POST_FN (fndecl) != error_mark_node);
> +
> +  releasing_vec args = build_arg_list (fndecl);
> +  if (get_postcondition_result_parameter (fndecl))
> +    vec_safe_push (args, DECL_RESULT (fndecl));
> +  tree call = build_call_a (DECL_POST_FN (fndecl), args->length (),
> +			    args->address ());
> +  CALL_FROM_THUNK_P (call) = true;
> +  finish_expr_stmt (call);
> +}
> +
> +/* Add a call or a direct evaluation of the pre checks.  */
> +
> +static void
> +apply_preconditions (tree fndecl)
> +{
> +  if (outline_contracts_p (fndecl))
> +    add_pre_condition_fn_call (fndecl);
> +  else
> +    emit_preconditions (DECL_CONTRACTS (fndecl));
> +}
> +
> +/* Add a call or a direct evaluation of the post checks.  */
> +
> +static void
> +apply_postconditions (tree fndecl)
> +{
> +  if (outline_contracts_p (fndecl))
> +    add_post_condition_fn_call (fndecl);
> +  else
> +    emit_postconditions (DECL_CONTRACTS (fndecl));
> +}
> +
> +/* Add contract handling to the function in FNDECL.
> +
> +   When we have only pre-conditions, this simply prepends a call (or a direct
> +   evaluation, for cdtors) to the existing function body.
> +
> +   When we have post conditions we build a try-finally block.
> +   If the function might throw then the handler in the try-finally is an
> +   EH_ELSE expression, where the post condition check is applied to the
> +   non-exceptional path, and a rethrow is applied to the EH path.  If the
> +   function has a non-throwing eh spec, then the handler is simply the post
> +   contract checker (either a call or, for cdtors, a direct evaluation).  */
> +
> +void
> +maybe_apply_function_contracts (tree fndecl)
> +{
> +  if (!handle_contracts_p (fndecl))
> +    /* We did nothing and the original function body statement list will be
> +       popped by our caller.  */
> +    return;
> +
> +  bool do_pre = has_active_preconditions (fndecl);
> +  bool do_post = has_active_postconditions (fndecl);
> +  /* We should not have reached here with nothing to do... */
> +  gcc_checking_assert (do_pre || do_post);
> +
> +  /* This copies the approach used for function try blocks.  */
> +  tree fnbody = pop_stmt_list (DECL_SAVED_TREE (fndecl));
> +  DECL_SAVED_TREE (fndecl) = push_stmt_list ();
> +  tree compound_stmt = begin_compound_stmt (0);
> +  current_binding_level->artificial = 1;
> +
> +  /* FIXME : find some better location to use - perhaps the position of the
> +     function opening brace, if that is available.  */
> +  location_t loc = UNKNOWN_LOCATION;
> +
> +  /* For other cases, we call a function to process the check.  */
> +
> +  /* IF we hsve a pre - but not a post, then just emit that.  */
> +  if (!do_post)
>     {
> -      releasing_vec args = build_arg_list (decl1);
> -      tree call = build_call_a (DECL_PRE_FN (decl1),
> -				args->length (),
> -				args->address ());
> -      CALL_FROM_THUNK_P (call) = true;
> -      finish_expr_stmt (call);
> +      apply_preconditions (fndecl);
> +      add_stmt (fnbody);
> +      finish_compound_stmt (compound_stmt);
> +      return;
>     }
> +
> +  tree try_fin = build_stmt (loc, TRY_FINALLY_EXPR, NULL_TREE, NULL_TREE);
> +  add_stmt (try_fin);
> +  TREE_OPERAND (try_fin, 0) = push_stmt_list ();
> +  if (do_pre)
> +    /* Add a precondition call, if we have one. */
> +    apply_preconditions (fndecl);
> +  add_stmt (fnbody);
> +  TREE_OPERAND (try_fin, 0) = pop_stmt_list (TREE_OPERAND (try_fin, 0));
> +  TREE_OPERAND (try_fin, 1) = push_stmt_list ();
> +  if (!type_noexcept_p (TREE_TYPE (fndecl)))
> +    {
> +      tree eh_else = build_stmt (loc, EH_ELSE_EXPR, NULL_TREE, NULL_TREE);
> +      /* We need to ignore this in constexpr considerations.  */
> +      CONTRACT_EH_ELSE_P (eh_else) = true;
> +      add_stmt (eh_else);
> +      TREE_OPERAND (eh_else, 0) = push_stmt_list ();
> +      apply_postconditions (fndecl);
> +      TREE_OPERAND (eh_else, 0) = pop_stmt_list (TREE_OPERAND (eh_else, 0));
> +      TREE_OPERAND (eh_else, 1) = push_stmt_list ();
> +      tree rethrow = build_throw (input_location, NULL_TREE, tf_warning_or_error);
> +      suppress_warning (rethrow);
> +      finish_expr_stmt (rethrow);
> +      TREE_OPERAND (eh_else, 1) = pop_stmt_list (TREE_OPERAND (eh_else, 1));
> +    }
> +  else
> +    apply_postconditions (fndecl);
> +  TREE_OPERAND (try_fin, 1) = pop_stmt_list (TREE_OPERAND (try_fin, 1));
> +  finish_compound_stmt (compound_stmt);
> +  /* The DECL_SAVED_TREE stmt list will be popped by our caller.  */
> }
> 
> +
> /* Finish up the pre & post function definitions for a guarded FNDECL,
>    and compile those functions all the way to assembler language output.  */
> 
> @@ -2074,34 +2131,6 @@ finish_function_contracts (tree fndecl)
>     }
> }
> 
> -/* Rewrite the expression of a returned expression so that it invokes the
> -   postcondition function as needed.  */
> -
> -tree
> -apply_postcondition_to_return (tree expr)
> -{
> -  tree fn = current_function_decl;
> -  tree post = DECL_POST_FN (fn);
> -  if (!post)
> -    return NULL_TREE;
> -
> -  /* If FN returns in memory, POST has a void return type and we call it when
> -     EXPR is DECL_RESULT (fn).  If FN returns a scalar, POST has the same
> -     return type and we call it when EXPR is the value being returned.  */
> -  if (VOID_TYPE_P (TREE_TYPE (TREE_TYPE (post)))
> -      != (expr == DECL_RESULT (fn)))
> -    return NULL_TREE;
> -
> -  releasing_vec args = build_arg_list (fn);
> -  if (get_postcondition_result_parameter (fn))
> -    vec_safe_push (args, expr);
> -  tree call = build_call_a (post,
> -			    args->length (),
> -			    args->address ());
> -  CALL_FROM_THUNK_P (call) = true;
> -
> -  return call;
> -}
> 
> /* A subroutine of duplicate_decls. Diagnose issues in the redeclaration of
>    guarded functions.  */
> diff --git a/gcc/cp/contracts.h b/gcc/cp/contracts.h
> index 3e5c30db331..c786affd0e3 100644
> --- a/gcc/cp/contracts.h
> +++ b/gcc/cp/contracts.h
> @@ -295,8 +295,9 @@ extern void update_late_contract		(tree, tree, tree);
> extern tree splice_out_contracts		(tree);
> extern bool all_attributes_are_contracts_p	(tree);
> extern void inherit_base_contracts		(tree, tree);
> -extern tree apply_postcondition_to_return	(tree);
> +//extern tree apply_postcondition_to_return	(tree);
> extern void start_function_contracts		(tree);
> +extern void maybe_apply_function_contracts	(tree);
> extern void finish_function_contracts		(tree);
> extern void set_contract_functions		(tree, tree, tree);
> extern tree build_contract_check		(tree);
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index 97bc211ff67..f350fc33e9b 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -4014,6 +4014,8 @@ coro_build_actor_or_destroy_function (tree orig, tree fn_type,
>   DECL_USER_ALIGN (fn) = DECL_USER_ALIGN (orig);
>   /* Apply attributes from the original fn.  */
>   DECL_ATTRIBUTES (fn) = copy_list (DECL_ATTRIBUTES (orig));
> +  /* but we do not want ones for contracts.  */
> +  remove_contract_attributes (fn);
> 
>   /* A void return.  */
>   tree resdecl = build_decl (loc, RESULT_DECL, 0, void_type_node);
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 416c60b7311..505ee5d4dc9 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -451,6 +451,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
>       ATOMIC_CONSTR_MAP_INSTANTIATED_P (in ATOMIC_CONSTR)
>       contract_semantic (in ASSERTION_, PRECONDITION_, POSTCONDITION_STMT)
>       RETURN_EXPR_LOCAL_ADDR_P (in RETURN_EXPR)
> +      CONTRACT_EH_ELSE_P (in EH_ELSE_EXPR)
>    1: IDENTIFIER_KIND_BIT_1 (in IDENTIFIER_NODE)
>       TI_PENDING_TEMPLATE_FLAG.
>       TEMPLATE_PARMS_FOR_INLINE.
> @@ -723,6 +724,8 @@ typedef struct ptrmem_cst * ptrmem_cst_t;
> 
> #define CLEANUP_P(NODE)		TREE_LANG_FLAG_0 (TRY_BLOCK_CHECK (NODE))
> 
> +#define CONTRACT_EH_ELSE_P(NODE) TREE_LANG_FLAG_0 (EH_ELSE_EXPR_CHECK (NODE))
> +
> #define BIND_EXPR_TRY_BLOCK(NODE) \
>   TREE_LANG_FLAG_0 (BIND_EXPR_CHECK (NODE))
> 
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 03deb1493a4..f3e733a29ba 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -18599,16 +18599,19 @@ finish_function (bool inline_p)
>   tree fndecl = current_function_decl;
>   tree fntype, ctype = NULL_TREE;
>   tree resumer = NULL_TREE, destroyer = NULL_TREE;
> -  bool coro_p = flag_coroutines
> -		&& !processing_template_decl
> -		&& DECL_COROUTINE_P (fndecl);
> -  bool coro_emit_helpers = false;
> 
>   /* When we get some parse errors, we can end up without a
>      current_function_decl, so cope.  */
> -  if (fndecl == NULL_TREE)
> +  if (fndecl == NULL_TREE || fndecl == error_mark_node)
>     return error_mark_node;
> 
> +  bool coro_p = flag_coroutines
> +		&& !processing_template_decl
> +		&& DECL_COROUTINE_P (fndecl);
> +  bool coro_emit_helpers = false;
> +  bool do_contracts = DECL_HAS_CONTRACTS_P (fndecl)
> +		      && !processing_template_decl;
> +
>   if (!DECL_OMP_DECLARE_REDUCTION_P (fndecl))
>     finish_lambda_scope ();
> 
> @@ -18644,6 +18647,10 @@ finish_function (bool inline_p)
> 	finish_eh_spec_block (TYPE_RAISES_EXCEPTIONS
> 			      (TREE_TYPE (fndecl)),
> 			      current_eh_spec_block);
> +
> +     /* If outlining succeeded, then add contracts handling if needed.  */
> +     if (coro_emit_helpers && do_contracts)
> +	maybe_apply_function_contracts (fndecl);
>     }
>   else
>   /* For a cloned function, we've already got all the code we need;
> @@ -18659,6 +18666,10 @@ finish_function (bool inline_p)
> 	finish_eh_spec_block (TYPE_RAISES_EXCEPTIONS
> 			      (TREE_TYPE (current_function_decl)),
> 			      current_eh_spec_block);
> +
> +     if (do_contracts)
> +	maybe_apply_function_contracts (current_function_decl);
> +
>     }
> 
>   /* If we're saving up tree structure, tie off the function now.  */
> @@ -18911,7 +18922,7 @@ finish_function (bool inline_p)
>   --function_depth;
> 
>   /* Clean up.  */
> -  current_function_decl = NULL_TREE;
> +  gcc_checking_assert (current_function_decl == NULL_TREE);
> 
>   invoke_plugin_callbacks (PLUGIN_FINISH_PARSE_FUNCTION, fndecl);
> 
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 5970ac3d398..4434ba6e2f4 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -11416,13 +11416,7 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
> 
>   /* Actually copy the value returned into the appropriate location.  */
>   if (retval && retval != result)
> -    {
> -      /* If there's a postcondition for a scalar return value, wrap
> -	 retval in a call to the postcondition function.  */
> -      if (tree post = apply_postcondition_to_return (retval))
> -	retval = post;
> -      retval = cp_build_init_expr (result, retval);
> -    }
> +    retval = cp_build_init_expr (result, retval);
> 
>   if (current_function_return_value == bare_retval)
>     INIT_EXPR_NRV_P (retval) = true;
> @@ -11430,11 +11424,6 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
>   if (tree set = maybe_set_retval_sentinel ())
>     retval = build2 (COMPOUND_EXPR, void_type_node, retval, set);
> 
> -  /* If there's a postcondition for an aggregate return value, call the
> -     postcondition function after the return object is initialized.  */
> -  if (tree post = apply_postcondition_to_return (result))
> -    retval = build2 (COMPOUND_EXPR, void_type_node, retval, post);
> -
>   return retval;
> }
> 
> diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
> index 622c51d5c3f..43bd4c56d7f 100644
> --- a/gcc/gimplify.cc
> +++ b/gcc/gimplify.cc
> @@ -227,6 +227,7 @@ struct gimplify_ctx
>   unsigned keep_stack : 1;
>   unsigned save_stack : 1;
>   unsigned in_switch_expr : 1;
> +  unsigned in_handler_expr : 1;
> };
> 
> enum gimplify_defaultmap_kind
> @@ -18401,10 +18402,12 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
> 	    input_location = UNKNOWN_LOCATION;
> 	    eval = cleanup = NULL;
> 	    gimplify_and_add (TREE_OPERAND (*expr_p, 0), &eval);
> +	    bool save_in_handler_expr = gimplify_ctxp->in_handler_expr;
> 	    if (TREE_CODE (*expr_p) == TRY_FINALLY_EXPR
> 		&& TREE_CODE (TREE_OPERAND (*expr_p, 1)) == EH_ELSE_EXPR)
> 	      {
> 		gimple_seq n = NULL, e = NULL;
> +		gimplify_ctxp->in_handler_expr = true;
> 		gimplify_and_add (TREE_OPERAND (TREE_OPERAND (*expr_p, 1),
> 						0), &n);
> 		gimplify_and_add (TREE_OPERAND (TREE_OPERAND (*expr_p, 1),
> @@ -18416,7 +18419,11 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
> 		  }
> 	      }
> 	    else
> -	      gimplify_and_add (TREE_OPERAND (*expr_p, 1), &cleanup);
> +	      {
> +		gimplify_ctxp->in_handler_expr = true;
> +		gimplify_and_add (TREE_OPERAND (*expr_p, 1), &cleanup);
> +	      }
> +	    gimplify_ctxp->in_handler_expr = save_in_handler_expr;
> 	    /* Don't create bogus GIMPLE_TRY with empty cleanup.  */
> 	    if (gimple_seq_empty_p (cleanup))
> 	      {
> @@ -18516,6 +18523,10 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
> 	  /* When within an OMP context, notice uses of variables.  */
> 	  if (gimplify_omp_ctxp)
> 	    omp_notice_variable (gimplify_omp_ctxp, *expr_p, true);
> +	  /* Handlers can refer to the function result; if that has been
> +	     moved, we need to track it.  */
> +	  if (gimplify_ctxp->in_handler_expr && gimplify_ctxp->return_temp)
> +	    *expr_p = gimplify_ctxp->return_temp;
> 	  ret = GS_ALL_DONE;
> 	  break;
> 
> diff --git a/gcc/testsuite/g++.dg/contracts/pr115434.C b/gcc/testsuite/g++.dg/contracts/pr115434.C
> new file mode 100644
> index 00000000000..e9c847f8969
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/contracts/pr115434.C
> @@ -0,0 +1,16 @@
> +// We were failing to apply post conditions to void functions.
> +
> +// { dg-do run }
> +// { dg-options "-std=c++20 -fcontracts -fcontract-continuation-mode=on" }
> +
> +
> +void foo (const int b)
> +[[ post: b == 9 ]]  // contract not checked
> +{}
> +
> +int main()
> +{
> +   foo(3);
> +}
> +
> +// { dg-output "contract violation in function foo at .*.C:8: b == 9.*(\n|\r\n|\r)" }
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr110871.C b/gcc/testsuite/g++.dg/coroutines/pr110871.C
> new file mode 100644
> index 00000000000..8a667c856ae
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr110871.C
> @@ -0,0 +1,62 @@
> +// { dg-additional-options "-fcontracts -fcontract-continuation-mode=on" }
> +// { dg-do run }
> +#include <iostream>
> +#include <coroutine>
> +
> +// In order to test the contract violation diagnostic, we have to set
> +// -fcontract-continuation-mode=on; this means that the code will emit
> +// the message below - but before that the contract should have been checked.
> +void process(int from, int to)
> +{
> +  if (from > to)
> +    std::cout << "would have been a disaster!" << std::endl;
> +}
> +
> +template <typename T>
> +struct generator
> +{
> +    struct promise_type
> +    {
> +        template <typename... Args>
> +        promise_type(Args&&... args) {
> +            std::cout << "promise init" << std::endl;
> +            process(args...);
> +        }
> +
> +        std::suspend_always yield_value(T) { return {}; }
> +
> +        std::suspend_always initial_suspend() const noexcept { return {}; }
> +        std::suspend_never final_suspend() const noexcept { return {}; }
> +        void unhandled_exception() noexcept {}
> +
> +        generator<T> get_return_object() noexcept { return {}; }
> +    };
> +};
> +
> +namespace std {
> +template <typename T, typename... Args>
> +struct coroutine_traits<generator<T>, Args...>
> +{
> +    using promise_type = typename generator<T>::promise_type;
> +};
> +
> +};
> +
> +generator<int> seq(int from, int to) [[pre: from <= to]]
> +
> +{
> +    std::cout << "coro initial" << std::endl;
> +    for (int i = from; i <= to; ++i) {
> +        co_yield i;
> +        std::cout << "coro resumed" << std::endl;
> +    }
> +}
> +
> +int main() { 
> +    std::cout << "main initial" << std::endl;
> +    generator<int> s = seq(10, 5);
> +    (void)s;
> +    std::cout << "main continues" << std::endl;
> +}
> +
> +// { dg-output "contract violation in function seq at .*.C:45: from \<= to.*(\n|\r\n|\r)" }
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr110872.C b/gcc/testsuite/g++.dg/coroutines/pr110872.C
> new file mode 100644
> index 00000000000..a809986f296
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr110872.C
> @@ -0,0 +1,49 @@
> +// { dg-additional-options "-fcontracts -fcontract-continuation-mode=on" }
> +// { dg-do run }
> +
> +#include <iostream>
> +#include <coroutine>
> +
> +
> +template <typename T>
> +struct generator
> +{
> +    struct promise_type
> +    {
> +        std::suspend_always yield_value(T) { return {}; }
> +
> +        std::suspend_always initial_suspend() const noexcept { return {}; }
> +        std::suspend_never final_suspend() const noexcept { return {}; }
> +        void unhandled_exception() noexcept {}
> +
> +        generator<T> get_return_object() noexcept { return {}; }
> +    };
> +
> +    bool is_valid() { return false; }
> +};
> +
> +namespace std {
> +template <typename T, typename... Args>
> +struct coroutine_traits<generator<T>, Args...>
> +{
> +    using promise_type = typename generator<T>::promise_type;
> +};
> +
> +};
> +
> +generator<int> val(int v) 
> +[[post g: g.is_valid()]]
> +{
> +    std::cout << "coro initial" << std::endl;
> +    co_yield v;
> +    std::cout << "coro resumed" << std::endl;
> +}
> +
> +int main() { 
> +    std::cout << "main initial" << std::endl;
> +    generator<int> s = val(1);
> +    (void)s;
> +    std::cout << "main continues" << std::endl;
> +}
> +
> +// { dg-output "contract violation in function val at .*.C:35: g.is_valid().*(\n|\r\n|\r)" }
> -- 
> 2.39.2 (Apple Git-143)
>

On 6/17/24 8:15 AM, Iain Sandoe wrote:
> This patch came out of a discussion on Mattermost about how to deal
> with contracts/coroutines integration.  Actually, it would also allow
> some semantic checking to be deferred until the same spot - at which
> time there are no dependent types, which can simplify the process.
> 
> NOTE: this is a fix for bugs in the existing '2a' contracts impl. it
> does not attempt to make any of the changes required by P2900 to
> either code-gen or constexpr handling.
> 
> Tested on x86_64-darwin, so far, OK for trunk if testing succeeds on
> x86_64/powerpc64 linux too?
> thanks,
> Iain
> 
> --- 8< ---
> 
> The current implementation of contracts emits the checks into function
> bodies in three places; for pre-conditions at the start of the body,
> for asserts in-line in the function body and for post-conditions as an
> addition to return statements.
> 
> In general (at least with existing "2a" contract semantics) the in-line
> contract asserts behave as expected.
> 
> However, the mechanism is not applicable to:
> 
>   * Handling pre conditions in coroutines since, for those, the standard
>    specifies a wrapping of the original function body by functionality
>    implementing initial and final suspends (along with some housekeeping
>    to route exceptions).  Thus for such transformed function bodies, the
>    preconditions then get actioned after the initial suspend, which does
>    not behave as intended.
> 
>    * Handling post conditions in functions that do not have return
>      statements (which applies to coroutines and void functions).
> 
> In the following, we identify a potentially transformed function body
> (in the case of coroutines, this is usually called the "ramp()" function).
> 
> The patch here re-implements the code insertion in one of the two following
> ways (code for exposition only):
> 
>    * For functions with no post-conditions we wrap the potentially
>      transformed function as follows:

To support caller-side checking, I wonder about (not in this patch) 
doing the wrapping in a separate function, instead of outlining the 
condition evaluation?

>    {
>       handle_pre_condition_checking ();
>       potentially_transformed_function_body ();
>    }
> 
>    This implements the intent that the preconditions are processed after
>    the function parameters are initialised but before any other actions.
> 
>    * For functions with post-conditions:
> 
>    try
>     {
>       if (preconditions_exist)
>         handle_pre_condition_checking ();

Why are the preconditions within the try?

>       potentially_transformed_function_body ();
>     }
>    finally
>     {
>       handle_post_condition_checking ();
>     }
>    else [only if the function is not marked noexcept(true) ]
>     {
>       __rethrow ();

This sounds undesirable, EH normally continues unwinding after running a 
cleanup.  Why would the exception be considered caught in a way that 
needs a rethrow?

>     }
> 
> In this, post-conditions [that might apply to the return value etc.]
> are evaluated on every non-exceptional edge out of the function.
> 
> At present, the model here is that exceptions thrown by the function
> propagate upwards as if there were no contracts present.  If the desired
> semantic becomes that an exception is counted as equivalent to a contract
> violation - then we can add a second handler in place of the rethrow.
> 
> At constexpr time we need to evaluate the contract conditions, but not
> the exceptional path, which is handled by a flag on the EH_ELSE_EXPR that
> indicates it is in use for contract handling.

Why would we ever want to evaluate the exceptional path when we aren't 
throwing an exception?

> This patch specifically does not address changes to code-gen and constexpr
> handling that are contained in P2900.
> 
> 	PR c++/115434
> 	PR c++/110871
> 	PR c++/110872
> 
> gcc/cp/ChangeLog:
> 
> 	* constexpr.cc (cxx_eval_constant_expression): Handle EH_ELSE_EXPR.
> 	* contracts.cc (finish_contract_attribute): Remove excess line.
> 	(build_contract_condition_function): Post condition handlers are
> 	void now.
> 	(emit_postconditions_cleanup): Remove.
> 	(emit_postconditions): New.
> 	(add_pre_condition_fn_call): New.
> 	(add_post_condition_fn_call): New.
> 	(apply_preconditions): New.
> 	(apply_postconditions): New.
> 	(maybe_apply_function_contracts): New.
> 	(apply_postcondition_to_return): Remove.
> 	* contracts.h (apply_postcondition_to_return): Remove.
> 	(maybe_apply_function_contracts): Add.
> 	* coroutines.cc (coro_build_actor_or_destroy_function): Do not
> 	copy contracts to coroutine helpers.
> 	* cp-tree.h (CONTRACT_EH_ELSE_P): New.
> 	* decl.cc (finish_function): Handle wrapping a possibly
> 	transformed function body in contract checks.
> 	* typeck.cc (check_return_expr): Remove handling of post
> 	conditions on return expressions.
> 
> gcc/ChangeLog:
> 
> 	* gimplify.cc (struct gimplify_ctx): Add a flag to show we are
> 	expending a handler.
> 	(gimplify_expr): When we are expanding a handler, and the body
> 	transforms might have re-written DECL_RESULT into a gimple var,
> 	ensure that hander references to DECL_RESULT are also re-written
> 	to refer to the gimple var.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/contracts/pr115434.C: New test.
> 	* g++.dg/coroutines/pr110871.C: New test.
> 	* g++.dg/coroutines/pr110872.C: New test.
> 
> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
> ---
>   gcc/cp/constexpr.cc                        |  16 ++
>   gcc/cp/contracts.cc                        | 249 ++++++++++++---------
>   gcc/cp/contracts.h                         |   3 +-
>   gcc/cp/coroutines.cc                       |   2 +
>   gcc/cp/cp-tree.h                           |   3 +
>   gcc/cp/decl.cc                             |  23 +-
>   gcc/cp/typeck.cc                           |  13 +-
>   gcc/gimplify.cc                            |  13 +-
>   gcc/testsuite/g++.dg/contracts/pr115434.C  |  16 ++
>   gcc/testsuite/g++.dg/coroutines/pr110871.C |  62 +++++
>   gcc/testsuite/g++.dg/coroutines/pr110872.C |  49 ++++
>   11 files changed, 319 insertions(+), 130 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/contracts/pr115434.C
>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr110871.C
>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr110872.C
> 
> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> index bd72533491e..9b66b1753ad 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -7808,6 +7808,22 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
>   				      non_constant_p, overflow_p);
>         break;
>   
> +    case EH_ELSE_EXPR:
> +      /* Evaluate any cleanup that applies to non-EH exits, this only for
> +	 the output of the diagnostics ??? what is really meant to happen
> +	 at constexpr-time?...  */
> +      cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0), vc_discard,
> +				      non_constant_p, overflow_p);
> +
> +      /* The presence of a contract should not affect the constexpr.  */
> +      if (CONTRACT_EH_ELSE_P (t))
> +	break;
> +      if (!*non_constant_p)
> +	/* Also evaluate the EH handler.  */

As mentioned above, this seems undesirable until we get EH in constexpr.

> +	cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 1), vc_discard,
> +				      non_constant_p, overflow_p);
> +      break;
> +
>       case CLEANUP_STMT:
>         r = cxx_eval_constant_expression (ctx, CLEANUP_BODY (t), lval,
>   					non_constant_p, overflow_p,
> diff --git a/gcc/cp/contracts.cc b/gcc/cp/contracts.cc
> index 0822624a910..064f0b6a9d8 100644
> --- a/gcc/cp/contracts.cc
> +++ b/gcc/cp/contracts.cc
> @@ -88,64 +88,7 @@ along with GCC; see the file COPYING3.  If not see
>          return -v;
>        }
>   
> -   The original decl is left alone and instead calls are generated to pre/post
> -   functions within the body:
> -
> -     void fun.pre(int v)
> -     {
> -       [[ assert: v > 0 ]];
> -     }
> -     int fun.post(int v, int __r)
> -     {
> -       [[ assert: __r < 0 ]];
> -       return __r;
> -     }
> -     int fun(int v)
> -     {
> -       fun.pre(v);
> -       return fun.post(v, -v);
> -     }
> -
> -   If fun returns in memory, the return value is not passed through the post
> -   function; instead, the return object is initialized directly and then passed
> -   to the post function by invisible reference.
> -
> -   This sides steps a number of issues with having to rewrite the bodies or
> -   rewrite the parsed conditions as the parameters to the original function
> -   changes (as happens during redeclaration). The ultimate goal is to get
> -   something that optimizes well along the lines of
> -
> -     int fun(int v)
> -     {
> -       [[ assert: v > 0 ]];
> -       auto &&__r = -v;
> -       goto out;
> -     out:
> -       [[ assert: __r < 0 ]];
> -       return __r;
> -     }
> -
> -   With the idea being that multiple return statements could collapse the
> -   function epilogue after inlining the pre/post functions. clang is able
> -   to collapse common function epilogues, while gcc needs -O3 -Os combined.
> -
> -   Directly laying the pre contracts down in the function body doesn't have
> -   many issues. The post contracts may need to be repeated multiple times, once
> -   for each return, or a goto epilogue would need to be generated.
> -   For this initial implementation, generating function calls and letting
> -   later optimizations decide whether to inline and duplicate the actual
> -   checks or whether to collapse the shared epilogue was chosen.
> -
> -   For cdtors a post contract is implemented using a CLEANUP_STMT.
> -
> -   FIXME the compiler already shores cleanup code on multiple exit paths, so
> -   this outlining seems unnecessary if we represent the postcondition as a
> -   cleanup for all functions.
> -
> -   More helpful for optimization might be to make the contracts a wrapper
> -   function (for non-variadic functions), that could be inlined into a
> -   caller while preserving the call to the actual function?  Either that or
> -   mirror a never-continue post contract with an assume in the caller.  */
> +  TODO: reiterate the revised implementation.  */

Please do.  Your patch comment above seems to be pretty close?

>   
>   #include "config.h"
>   #include "system.h"
> @@ -801,7 +744,6 @@ finish_contract_attribute (tree identifier, tree contract)
>     tree attribute = build_tree_list (build_tree_list (NULL_TREE, identifier),
>   				    build_tree_list (NULL_TREE, contract));
>   
> -
>     /* Mark the attribute as dependent if the condition is dependent.
>   
>        TODO: I'm not sure this is strictly necessary. It's going to be marked as
> @@ -1444,10 +1386,8 @@ build_contract_condition_function (tree fndecl, bool pre)
>         *last = build_tree_list (NULL_TREE, value_type);
>         TREE_CHAIN (*last) = void_list_node;
>   
> -      if (aggregate_value_p (value_type, fndecl))
> -	/* If FNDECL returns in memory, don't return the value from the
> -	   postcondition.  */
> -	value_type = void_type_node;
> +      /* The handler is a void return.  */
> +      value_type = void_type_node;
>       }
>   
>     TREE_TYPE (fn) = build_function_type (value_type, arg_types);
> @@ -1882,15 +1822,12 @@ emit_preconditions (tree attr)
>     return emit_contract_conditions (attr, PRECONDITION_STMT);
>   }
>   
> -/* Emit statements for postcondition attributes.  */
> +/* Emit statements for precondition attributes.  */
>   
>   static void
> -emit_postconditions_cleanup (tree contracts)
> +emit_postconditions (tree attr)
>   {
> -  tree stmts = push_stmt_list ();
> -  emit_contract_conditions (contracts, POSTCONDITION_STMT);
> -  stmts = pop_stmt_list (stmts);
> -  push_cleanup (NULL_TREE, stmts, /*eh_only*/false);
> +  return emit_contract_conditions (attr, POSTCONDITION_STMT);
>   }
>   
>   /* We're compiling the pre/postcondition function CONDFN; remap any FN
> @@ -1993,32 +1930,152 @@ start_function_contracts (tree decl1)
>     if (!handle_contracts_p (decl1))
>       return;
>   
> +  /* For cdtors, we evaluate the contracts check inline.  */
>     if (!outline_contracts_p (decl1))
> -    {
> -      emit_preconditions (DECL_CONTRACTS (current_function_decl));
> -      emit_postconditions_cleanup (DECL_CONTRACTS (current_function_decl));
> -      return;
> -    }
> +    return;
>   
>     /* Contracts may have just been added without a chance to parse them, though
>        we still need the PRE_FN available to generate a call to it.  */
>     if (!DECL_PRE_FN (decl1))
>       build_contract_function_decls (decl1);
>   
> +}
> +
> +/* If we have a precondition function and it's valid, call it.  */
> +
> +static void
> +add_pre_condition_fn_call (tree fndecl)
> +{
>     /* If we're starting a guarded function with valid contracts, we need to
>        insert a call to the pre function.  */
> -  if (DECL_PRE_FN (decl1)
> -      && DECL_PRE_FN (decl1) != error_mark_node)
> +  gcc_checking_assert (DECL_PRE_FN (fndecl)
> +      && DECL_PRE_FN (fndecl) != error_mark_node);
> +
> +  releasing_vec args = build_arg_list (fndecl);
> +  tree call = build_call_a (DECL_PRE_FN (fndecl), args->length (),
> +			    args->address ());
> +  CALL_FROM_THUNK_P (call) = true;
> +  finish_expr_stmt (call);
> +}
> +
> +/* Build and add a call to the post-condition checking function, when that
> +   is in use.  */
> +
> +static void
> +add_post_condition_fn_call (tree fndecl)
> +{
> +  gcc_checking_assert (DECL_POST_FN (fndecl)
> +      && DECL_POST_FN (fndecl) != error_mark_node);
> +
> +  releasing_vec args = build_arg_list (fndecl);
> +  if (get_postcondition_result_parameter (fndecl))
> +    vec_safe_push (args, DECL_RESULT (fndecl));
> +  tree call = build_call_a (DECL_POST_FN (fndecl), args->length (),
> +			    args->address ());
> +  CALL_FROM_THUNK_P (call) = true;
> +  finish_expr_stmt (call);
> +}
> +
> +/* Add a call or a direct evaluation of the pre checks.  */
> +
> +static void
> +apply_preconditions (tree fndecl)
> +{
> +  if (outline_contracts_p (fndecl))
> +    add_pre_condition_fn_call (fndecl);
> +  else
> +    emit_preconditions (DECL_CONTRACTS (fndecl));
> +}
> +
> +/* Add a call or a direct evaluation of the post checks.  */
> +
> +static void
> +apply_postconditions (tree fndecl)
> +{
> +  if (outline_contracts_p (fndecl))
> +    add_post_condition_fn_call (fndecl);
> +  else
> +    emit_postconditions (DECL_CONTRACTS (fndecl));
> +}
> +
> +/* Add contract handling to the function in FNDECL.
> +
> +   When we have only pre-conditions, this simply prepends a call (or a direct
> +   evaluation, for cdtors) to the existing function body.
> +
> +   When we have post conditions we build a try-finally block.
> +   If the function might throw then the handler in the try-finally is an
> +   EH_ELSE expression, where the post condition check is applied to the
> +   non-exceptional path, and a rethrow is applied to the EH path.  If the
> +   function has a non-throwing eh spec, then the handler is simply the post
> +   contract checker (either a call or, for cdtors, a direct evaluation).  */
> +
> +void
> +maybe_apply_function_contracts (tree fndecl)
> +{
> +  if (!handle_contracts_p (fndecl))
> +    /* We did nothing and the original function body statement list will be
> +       popped by our caller.  */
> +    return;
> +
> +  bool do_pre = has_active_preconditions (fndecl);
> +  bool do_post = has_active_postconditions (fndecl);
> +  /* We should not have reached here with nothing to do... */
> +  gcc_checking_assert (do_pre || do_post);
> +
> +  /* This copies the approach used for function try blocks.  */
> +  tree fnbody = pop_stmt_list (DECL_SAVED_TREE (fndecl));
> +  DECL_SAVED_TREE (fndecl) = push_stmt_list ();
> +  tree compound_stmt = begin_compound_stmt (0);
> +  current_binding_level->artificial = 1;
> +
> +  /* FIXME : find some better location to use - perhaps the position of the
> +     function opening brace, if that is available.  */
> +  location_t loc = UNKNOWN_LOCATION;

IMO it's actually best to leave it UNKNOWN to avoid problems with the 
debugger jumping back to the beginning of the function when processing 
cleanups.

> +  /* For other cases, we call a function to process the check.  */
> +
> +  /* IF we hsve a pre - but not a post, then just emit that.  */
> +  if (!do_post)
>       {
> -      releasing_vec args = build_arg_list (decl1);
> -      tree call = build_call_a (DECL_PRE_FN (decl1),
> -				args->length (),
> -				args->address ());
> -      CALL_FROM_THUNK_P (call) = true;
> -      finish_expr_stmt (call);
> +      apply_preconditions (fndecl);
> +      add_stmt (fnbody);
> +      finish_compound_stmt (compound_stmt);
> +      return;
>       }
> +
> +  tree try_fin = build_stmt (loc, TRY_FINALLY_EXPR, NULL_TREE, NULL_TREE);
> +  add_stmt (try_fin);
> +  TREE_OPERAND (try_fin, 0) = push_stmt_list ();
> +  if (do_pre)
> +    /* Add a precondition call, if we have one. */
> +    apply_preconditions (fndecl);
> +  add_stmt (fnbody);
> +  TREE_OPERAND (try_fin, 0) = pop_stmt_list (TREE_OPERAND (try_fin, 0));
> +  TREE_OPERAND (try_fin, 1) = push_stmt_list ();
> +  if (!type_noexcept_p (TREE_TYPE (fndecl)))
> +    {
> +      tree eh_else = build_stmt (loc, EH_ELSE_EXPR, NULL_TREE, NULL_TREE);
> +      /* We need to ignore this in constexpr considerations.  */
> +      CONTRACT_EH_ELSE_P (eh_else) = true;
> +      add_stmt (eh_else);
> +      TREE_OPERAND (eh_else, 0) = push_stmt_list ();
> +      apply_postconditions (fndecl);
> +      TREE_OPERAND (eh_else, 0) = pop_stmt_list (TREE_OPERAND (eh_else, 0));
> +      TREE_OPERAND (eh_else, 1) = push_stmt_list ();
> +      tree rethrow = build_throw (input_location, NULL_TREE, tf_warning_or_error);
> +      suppress_warning (rethrow);
> +      finish_expr_stmt (rethrow);
> +      TREE_OPERAND (eh_else, 1) = pop_stmt_list (TREE_OPERAND (eh_else, 1));
> +    }
> +  else
> +    apply_postconditions (fndecl);
> +  TREE_OPERAND (try_fin, 1) = pop_stmt_list (TREE_OPERAND (try_fin, 1));
> +  finish_compound_stmt (compound_stmt);
> +  /* The DECL_SAVED_TREE stmt list will be popped by our caller.  */
>   }
>   
> +

Unnecessary extra line.

>   /* Finish up the pre & post function definitions for a guarded FNDECL,
>      and compile those functions all the way to assembler language output.  */
>   
> @@ -2074,34 +2131,6 @@ finish_function_contracts (tree fndecl)
>       }
>   }
>   
> -/* Rewrite the expression of a returned expression so that it invokes the
> -   postcondition function as needed.  */
> -
> -tree
> -apply_postcondition_to_return (tree expr)
> -{
> -  tree fn = current_function_decl;
> -  tree post = DECL_POST_FN (fn);
> -  if (!post)
> -    return NULL_TREE;
> -
> -  /* If FN returns in memory, POST has a void return type and we call it when
> -     EXPR is DECL_RESULT (fn).  If FN returns a scalar, POST has the same
> -     return type and we call it when EXPR is the value being returned.  */
> -  if (VOID_TYPE_P (TREE_TYPE (TREE_TYPE (post)))
> -      != (expr == DECL_RESULT (fn)))
> -    return NULL_TREE;
> -
> -  releasing_vec args = build_arg_list (fn);
> -  if (get_postcondition_result_parameter (fn))
> -    vec_safe_push (args, expr);
> -  tree call = build_call_a (post,
> -			    args->length (),
> -			    args->address ());
> -  CALL_FROM_THUNK_P (call) = true;
> -
> -  return call;
> -}
>   
>   /* A subroutine of duplicate_decls. Diagnose issues in the redeclaration of
>      guarded functions.  */
> diff --git a/gcc/cp/contracts.h b/gcc/cp/contracts.h
> index 3e5c30db331..c786affd0e3 100644
> --- a/gcc/cp/contracts.h
> +++ b/gcc/cp/contracts.h
> @@ -295,8 +295,9 @@ extern void update_late_contract		(tree, tree, tree);
>   extern tree splice_out_contracts		(tree);
>   extern bool all_attributes_are_contracts_p	(tree);
>   extern void inherit_base_contracts		(tree, tree);
> -extern tree apply_postcondition_to_return	(tree);
> +//extern tree apply_postcondition_to_return	(tree);

This seems like a temporary thing that didn't get removed?

>   extern void start_function_contracts		(tree);
> +extern void maybe_apply_function_contracts	(tree);
>   extern void finish_function_contracts		(tree);
>   extern void set_contract_functions		(tree, tree, tree);
>   extern tree build_contract_check		(tree);
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index 97bc211ff67..f350fc33e9b 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -4014,6 +4014,8 @@ coro_build_actor_or_destroy_function (tree orig, tree fn_type,
>     DECL_USER_ALIGN (fn) = DECL_USER_ALIGN (orig);
>     /* Apply attributes from the original fn.  */
>     DECL_ATTRIBUTES (fn) = copy_list (DECL_ATTRIBUTES (orig));
> +  /* but we do not want ones for contracts.  */
> +  remove_contract_attributes (fn);
>   
>     /* A void return.  */
>     tree resdecl = build_decl (loc, RESULT_DECL, 0, void_type_node);
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 416c60b7311..505ee5d4dc9 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -451,6 +451,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
>         ATOMIC_CONSTR_MAP_INSTANTIATED_P (in ATOMIC_CONSTR)
>         contract_semantic (in ASSERTION_, PRECONDITION_, POSTCONDITION_STMT)
>         RETURN_EXPR_LOCAL_ADDR_P (in RETURN_EXPR)
> +      CONTRACT_EH_ELSE_P (in EH_ELSE_EXPR)
>      1: IDENTIFIER_KIND_BIT_1 (in IDENTIFIER_NODE)
>         TI_PENDING_TEMPLATE_FLAG.
>         TEMPLATE_PARMS_FOR_INLINE.
> @@ -723,6 +724,8 @@ typedef struct ptrmem_cst * ptrmem_cst_t;
>   
>   #define CLEANUP_P(NODE)		TREE_LANG_FLAG_0 (TRY_BLOCK_CHECK (NODE))
>   
> +#define CONTRACT_EH_ELSE_P(NODE) TREE_LANG_FLAG_0 (EH_ELSE_EXPR_CHECK (NODE))

This would need a comment if we kept it.

> +
>   #define BIND_EXPR_TRY_BLOCK(NODE) \
>     TREE_LANG_FLAG_0 (BIND_EXPR_CHECK (NODE))
>   
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 03deb1493a4..f3e733a29ba 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -18599,16 +18599,19 @@ finish_function (bool inline_p)
>     tree fndecl = current_function_decl;
>     tree fntype, ctype = NULL_TREE;
>     tree resumer = NULL_TREE, destroyer = NULL_TREE;
> -  bool coro_p = flag_coroutines
> -		&& !processing_template_decl
> -		&& DECL_COROUTINE_P (fndecl);
> -  bool coro_emit_helpers = false;
>   
>     /* When we get some parse errors, we can end up without a
>        current_function_decl, so cope.  */
> -  if (fndecl == NULL_TREE)
> +  if (fndecl == NULL_TREE || fndecl == error_mark_node)
>       return error_mark_node;
>   
> +  bool coro_p = flag_coroutines
> +		&& !processing_template_decl
> +		&& DECL_COROUTINE_P (fndecl);
> +  bool coro_emit_helpers = false;
> +  bool do_contracts = DECL_HAS_CONTRACTS_P (fndecl)
> +		      && !processing_template_decl;
> +
>     if (!DECL_OMP_DECLARE_REDUCTION_P (fndecl))
>       finish_lambda_scope ();
>   
> @@ -18644,6 +18647,10 @@ finish_function (bool inline_p)
>   	finish_eh_spec_block (TYPE_RAISES_EXCEPTIONS
>   			      (TREE_TYPE (fndecl)),
>   			      current_eh_spec_block);
> +
> +     /* If outlining succeeded, then add contracts handling if needed.  */
> +     if (coro_emit_helpers && do_contracts)
> +	maybe_apply_function_contracts (fndecl);
>       }
>     else
>     /* For a cloned function, we've already got all the code we need;
> @@ -18659,6 +18666,10 @@ finish_function (bool inline_p)
>   	finish_eh_spec_block (TYPE_RAISES_EXCEPTIONS
>   			      (TREE_TYPE (current_function_decl)),
>   			      current_eh_spec_block);
> +
> +     if (do_contracts)
> +	maybe_apply_function_contracts (current_function_decl);
> +
>       }
>   
>     /* If we're saving up tree structure, tie off the function now.  */
> @@ -18911,7 +18922,7 @@ finish_function (bool inline_p)
>     --function_depth;
>   
>     /* Clean up.  */
> -  current_function_decl = NULL_TREE;
> +  gcc_checking_assert (current_function_decl == NULL_TREE);
>   
>     invoke_plugin_callbacks (PLUGIN_FINISH_PARSE_FUNCTION, fndecl);
>   
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 5970ac3d398..4434ba6e2f4 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -11416,13 +11416,7 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
>   
>     /* Actually copy the value returned into the appropriate location.  */
>     if (retval && retval != result)
> -    {
> -      /* If there's a postcondition for a scalar return value, wrap
> -	 retval in a call to the postcondition function.  */
> -      if (tree post = apply_postcondition_to_return (retval))
> -	retval = post;
> -      retval = cp_build_init_expr (result, retval);
> -    }
> +    retval = cp_build_init_expr (result, retval);
>   
>     if (current_function_return_value == bare_retval)
>       INIT_EXPR_NRV_P (retval) = true;
> @@ -11430,11 +11424,6 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
>     if (tree set = maybe_set_retval_sentinel ())
>       retval = build2 (COMPOUND_EXPR, void_type_node, retval, set);
>   
> -  /* If there's a postcondition for an aggregate return value, call the
> -     postcondition function after the return object is initialized.  */
> -  if (tree post = apply_postcondition_to_return (result))
> -    retval = build2 (COMPOUND_EXPR, void_type_node, retval, post);
> -
>     return retval;
>   }
>   
> diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
> index 622c51d5c3f..43bd4c56d7f 100644
> --- a/gcc/gimplify.cc
> +++ b/gcc/gimplify.cc
> @@ -227,6 +227,7 @@ struct gimplify_ctx
>     unsigned keep_stack : 1;
>     unsigned save_stack : 1;
>     unsigned in_switch_expr : 1;
> +  unsigned in_handler_expr : 1;
>   };
>   
>   enum gimplify_defaultmap_kind
> @@ -18401,10 +18402,12 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
>   	    input_location = UNKNOWN_LOCATION;
>   	    eval = cleanup = NULL;
>   	    gimplify_and_add (TREE_OPERAND (*expr_p, 0), &eval);
> +	    bool save_in_handler_expr = gimplify_ctxp->in_handler_expr;
>   	    if (TREE_CODE (*expr_p) == TRY_FINALLY_EXPR
>   		&& TREE_CODE (TREE_OPERAND (*expr_p, 1)) == EH_ELSE_EXPR)
>   	      {
>   		gimple_seq n = NULL, e = NULL;
> +		gimplify_ctxp->in_handler_expr = true;
>   		gimplify_and_add (TREE_OPERAND (TREE_OPERAND (*expr_p, 1),
>   						0), &n);
>   		gimplify_and_add (TREE_OPERAND (TREE_OPERAND (*expr_p, 1),
> @@ -18416,7 +18419,11 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
>   		  }
>   	      }
>   	    else
> -	      gimplify_and_add (TREE_OPERAND (*expr_p, 1), &cleanup);
> +	      {
> +		gimplify_ctxp->in_handler_expr = true;
> +		gimplify_and_add (TREE_OPERAND (*expr_p, 1), &cleanup);
> +	      }
> +	    gimplify_ctxp->in_handler_expr = save_in_handler_expr;
>   	    /* Don't create bogus GIMPLE_TRY with empty cleanup.  */
>   	    if (gimple_seq_empty_p (cleanup))
>   	      {
> @@ -18516,6 +18523,10 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
>   	  /* When within an OMP context, notice uses of variables.  */
>   	  if (gimplify_omp_ctxp)
>   	    omp_notice_variable (gimplify_omp_ctxp, *expr_p, true);
> +	  /* Handlers can refer to the function result; if that has been
> +	     moved, we need to track it.  */
> +	  if (gimplify_ctxp->in_handler_expr && gimplify_ctxp->return_temp)
> +	    *expr_p = gimplify_ctxp->return_temp;
>   	  ret = GS_ALL_DONE;
>   	  break;
>   
> diff --git a/gcc/testsuite/g++.dg/contracts/pr115434.C b/gcc/testsuite/g++.dg/contracts/pr115434.C
> new file mode 100644
> index 00000000000..e9c847f8969
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/contracts/pr115434.C
> @@ -0,0 +1,16 @@
> +// We were failing to apply post conditions to void functions.
> +
> +// { dg-do run }
> +// { dg-options "-std=c++20 -fcontracts -fcontract-continuation-mode=on" }
> +
> +
> +void foo (const int b)
> +[[ post: b == 9 ]]  // contract not checked
> +{}
> +
> +int main()
> +{
> +   foo(3);
> +}
> +
> +// { dg-output "contract violation in function foo at .*.C:8: b == 9.*(\n|\r\n|\r)" }
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr110871.C b/gcc/testsuite/g++.dg/coroutines/pr110871.C
> new file mode 100644
> index 00000000000..8a667c856ae
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr110871.C
> @@ -0,0 +1,62 @@
> +// { dg-additional-options "-fcontracts -fcontract-continuation-mode=on" }
> +// { dg-do run }
> +#include <iostream>
> +#include <coroutine>
> +
> +// In order to test the contract violation diagnostic, we have to set
> +// -fcontract-continuation-mode=on; this means that the code will emit
> +// the message below - but before that the contract should have been checked.
> +void process(int from, int to)
> +{
> +  if (from > to)
> +    std::cout << "would have been a disaster!" << std::endl;
> +}
> +
> +template <typename T>
> +struct generator
> +{
> +    struct promise_type
> +    {
> +        template <typename... Args>
> +        promise_type(Args&&... args) {
> +            std::cout << "promise init" << std::endl;
> +            process(args...);
> +        }
> +
> +        std::suspend_always yield_value(T) { return {}; }
> +
> +        std::suspend_always initial_suspend() const noexcept { return {}; }
> +        std::suspend_never final_suspend() const noexcept { return {}; }
> +        void unhandled_exception() noexcept {}
> +
> +        generator<T> get_return_object() noexcept { return {}; }
> +    };
> +};
> +
> +namespace std {
> +template <typename T, typename... Args>
> +struct coroutine_traits<generator<T>, Args...>
> +{
> +    using promise_type = typename generator<T>::promise_type;
> +};
> +
> +};
> +
> +generator<int> seq(int from, int to) [[pre: from <= to]]
> +
> +{
> +    std::cout << "coro initial" << std::endl;
> +    for (int i = from; i <= to; ++i) {
> +        co_yield i;
> +        std::cout << "coro resumed" << std::endl;
> +    }
> +}
> +
> +int main() {
> +    std::cout << "main initial" << std::endl;
> +    generator<int> s = seq(10, 5);
> +    (void)s;
> +    std::cout << "main continues" << std::endl;
> +}
> +
> +// { dg-output "contract violation in function seq at .*.C:45: from \<= to.*(\n|\r\n|\r)" }
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr110872.C b/gcc/testsuite/g++.dg/coroutines/pr110872.C
> new file mode 100644
> index 00000000000..a809986f296
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr110872.C
> @@ -0,0 +1,49 @@
> +// { dg-additional-options "-fcontracts -fcontract-continuation-mode=on" }
> +// { dg-do run }
> +
> +#include <iostream>
> +#include <coroutine>
> +
> +
> +template <typename T>
> +struct generator
> +{
> +    struct promise_type
> +    {
> +        std::suspend_always yield_value(T) { return {}; }
> +
> +        std::suspend_always initial_suspend() const noexcept { return {}; }
> +        std::suspend_never final_suspend() const noexcept { return {}; }
> +        void unhandled_exception() noexcept {}
> +
> +        generator<T> get_return_object() noexcept { return {}; }
> +    };
> +
> +    bool is_valid() { return false; }
> +};
> +
> +namespace std {
> +template <typename T, typename... Args>
> +struct coroutine_traits<generator<T>, Args...>
> +{
> +    using promise_type = typename generator<T>::promise_type;
> +};
> +
> +};
> +
> +generator<int> val(int v)
> +[[post g: g.is_valid()]]
> +{
> +    std::cout << "coro initial" << std::endl;
> +    co_yield v;
> +    std::cout << "coro resumed" << std::endl;
> +}
> +
> +int main() {
> +    std::cout << "main initial" << std::endl;
> +    generator<int> s = val(1);
> +    (void)s;
> +    std::cout << "main continues" << std::endl;
> +}
> +
> +// { dg-output "contract violation in function val at .*.C:35: g.is_valid().*(\n|\r\n|\r)" }

Hello Jason,

before re-working, I think I need some guidance.

> On 8 Jul 2024, at 20:19, Jason Merrill <jason@redhat.com> wrote:
> 
> On 6/17/24 8:15 AM, Iain Sandoe wrote:
>> This patch came out of a discussion on Mattermost about how to deal
>> with contracts/coroutines integration.  Actually, it would also allow
>> some semantic checking to be deferred until the same spot - at which
>> time there are no dependent types, which can simplify the process.
>> NOTE: this is a fix for bugs in the existing '2a' contracts impl. it
>> does not attempt to make any of the changes required by P2900 to
>> either code-gen or constexpr handling.
>> Tested on x86_64-darwin, so far, OK for trunk if testing succeeds on
>> x86_64/powerpc64 linux too?
>> thanks,
>> Iain
>> --- 8< ---
>> The current implementation of contracts emits the checks into function
>> bodies in three places; for pre-conditions at the start of the body,
>> for asserts in-line in the function body and for post-conditions as an
>> addition to return statements.
>> In general (at least with existing "2a" contract semantics) the in-line
>> contract asserts behave as expected.
>> However, the mechanism is not applicable to:
>>  * Handling pre conditions in coroutines since, for those, the standard
>>   specifies a wrapping of the original function body by functionality
>>   implementing initial and final suspends (along with some housekeeping
>>   to route exceptions).  Thus for such transformed function bodies, the
>>   preconditions then get actioned after the initial suspend, which does
>>   not behave as intended.
>>   * Handling post conditions in functions that do not have return
>>     statements (which applies to coroutines and void functions).
>> In the following, we identify a potentially transformed function body
>> (in the case of coroutines, this is usually called the "ramp()" function).
>> The patch here re-implements the code insertion in one of the two following
>> ways (code for exposition only):
>>   * For functions with no post-conditions we wrap the potentially
>>     transformed function as follows:
> 
> To support caller-side checking, I wonder about (not in this patch) doing the wrapping in a separate function, instead of outlining the condition evaluation?

ack,
note that, following Josh’s presentation to EWG, we  are [well Nina is] going to be implementing caller-side contracts in support of the new virtual function model.  I think we need to consider the two things together.

>>   {
>>      handle_pre_condition_checking ();
>>      potentially_transformed_function_body ();
>>   }
>>   This implements the intent that the preconditions are processed after
>>   the function parameters are initialised but before any other actions.
>>   * For functions with post-conditions:
>>   try
>>    {
>>      if (preconditions_exist)
>>        handle_pre_condition_checking ();
> 
> Why are the preconditions within the try?

It was the intention that if any EH actions were needed to implement the semantics of P2900 that would be handled.
They do not need to be given your comments below.

>>      potentially_transformed_function_body ();
>>    }
>>   finally
>>    {
>>      handle_post_condition_checking ();
>>    }
>>   else [only if the function is not marked noexcept(true) ]
>>    {
>>      __rethrow ();
> 
> This sounds undesirable, EH normally continues unwinding after running a cleanup.  Why would the exception be considered caught in a way that needs a rethrow?

We need to implement that the no-EH cleanup [i.e. the postcondition] does _not_ run when an exception is thrown.  Right now there’s no HANDLER type that does that - we have EH_ONLY but no NON_EH one (otherwise I'd have built a C++ try).

however, I could not figure out how to make this arm of the EH_ELSE_EXPR work as an empty construct (the gimplifier does not seem to expect this).

I suppose another possibility is to keep the contracts trees until gimplification time and lower them directly to the gimple equivalent - but I’m not sure it would solve the underlying issue - we need a “non-eh-only edges” cleanup.

>>    }
>> In this, post-conditions [that might apply to the return value etc.]
>> are evaluated on every non-exceptional edge out of the function.


>> At present, the model here is that exceptions thrown by the function
>> propagate upwards as if there were no contracts present.  If the desired
>> semantic becomes that an exception is counted as equivalent to a contract
>> violation - then we can add a second handler in place of the rethrow.
>> At constexpr time we need to evaluate the contract conditions, but not
>> the exceptional path, which is handled by a flag on the EH_ELSE_EXPR that
>> indicates it is in use for contract handling.
> 
> Why would we ever want to evaluate the exceptional path when we aren't throwing an exception?

We wouldn’t; my explanation is not adequate - the constexpr recursion fails if it reaches (the likely unneccessary) rethrow - we need to prevent that.

the remainder of your comments below I can handle with the revised edition, but I’d welcome comments on the non-eh-only cleanup.

thanks for the review
Iain


>> This patch specifically does not address changes to code-gen and constexpr
>> handling that are contained in P2900.
>> 	PR c++/115434
>> 	PR c++/110871
>> 	PR c++/110872
>> gcc/cp/ChangeLog:
>> 	* constexpr.cc (cxx_eval_constant_expression): Handle EH_ELSE_EXPR.
>> 	* contracts.cc (finish_contract_attribute): Remove excess line.
>> 	(build_contract_condition_function): Post condition handlers are
>> 	void now.
>> 	(emit_postconditions_cleanup): Remove.
>> 	(emit_postconditions): New.
>> 	(add_pre_condition_fn_call): New.
>> 	(add_post_condition_fn_call): New.
>> 	(apply_preconditions): New.
>> 	(apply_postconditions): New.
>> 	(maybe_apply_function_contracts): New.
>> 	(apply_postcondition_to_return): Remove.
>> 	* contracts.h (apply_postcondition_to_return): Remove.
>> 	(maybe_apply_function_contracts): Add.
>> 	* coroutines.cc (coro_build_actor_or_destroy_function): Do not
>> 	copy contracts to coroutine helpers.
>> 	* cp-tree.h (CONTRACT_EH_ELSE_P): New.
>> 	* decl.cc (finish_function): Handle wrapping a possibly
>> 	transformed function body in contract checks.
>> 	* typeck.cc (check_return_expr): Remove handling of post
>> 	conditions on return expressions.
>> gcc/ChangeLog:
>> 	* gimplify.cc (struct gimplify_ctx): Add a flag to show we are
>> 	expending a handler.
>> 	(gimplify_expr): When we are expanding a handler, and the body
>> 	transforms might have re-written DECL_RESULT into a gimple var,
>> 	ensure that hander references to DECL_RESULT are also re-written
>> 	to refer to the gimple var.
>> gcc/testsuite/ChangeLog:
>> 	* g++.dg/contracts/pr115434.C: New test.
>> 	* g++.dg/coroutines/pr110871.C: New test.
>> 	* g++.dg/coroutines/pr110872.C: New test.
>> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
>> ---
>>  gcc/cp/constexpr.cc                        |  16 ++
>>  gcc/cp/contracts.cc                        | 249 ++++++++++++---------
>>  gcc/cp/contracts.h                         |   3 +-
>>  gcc/cp/coroutines.cc                       |   2 +
>>  gcc/cp/cp-tree.h                           |   3 +
>>  gcc/cp/decl.cc                             |  23 +-
>>  gcc/cp/typeck.cc                           |  13 +-
>>  gcc/gimplify.cc                            |  13 +-
>>  gcc/testsuite/g++.dg/contracts/pr115434.C  |  16 ++
>>  gcc/testsuite/g++.dg/coroutines/pr110871.C |  62 +++++
>>  gcc/testsuite/g++.dg/coroutines/pr110872.C |  49 ++++
>>  11 files changed, 319 insertions(+), 130 deletions(-)
>>  create mode 100644 gcc/testsuite/g++.dg/contracts/pr115434.C
>>  create mode 100644 gcc/testsuite/g++.dg/coroutines/pr110871.C
>>  create mode 100644 gcc/testsuite/g++.dg/coroutines/pr110872.C
>> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
>> index bd72533491e..9b66b1753ad 100644
>> --- a/gcc/cp/constexpr.cc
>> +++ b/gcc/cp/constexpr.cc
>> @@ -7808,6 +7808,22 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
>>  				      non_constant_p, overflow_p);
>>        break;
>>  +    case EH_ELSE_EXPR:
>> +      /* Evaluate any cleanup that applies to non-EH exits, this only for
>> +	 the output of the diagnostics ??? what is really meant to happen
>> +	 at constexpr-time?...  */
>> +      cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0), vc_discard,
>> +				      non_constant_p, overflow_p);
>> +
>> +      /* The presence of a contract should not affect the constexpr.  */
>> +      if (CONTRACT_EH_ELSE_P (t))
>> +	break;
>> +      if (!*non_constant_p)
>> +	/* Also evaluate the EH handler.  */
> 
> As mentioned above, this seems undesirable until we get EH in constexpr.
> 
>> +	cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 1), vc_discard,
>> +				      non_constant_p, overflow_p);
>> +      break;
>> +
>>      case CLEANUP_STMT:
>>        r = cxx_eval_constant_expression (ctx, CLEANUP_BODY (t), lval,
>>  					non_constant_p, overflow_p,
>> diff --git a/gcc/cp/contracts.cc b/gcc/cp/contracts.cc
>> index 0822624a910..064f0b6a9d8 100644
>> --- a/gcc/cp/contracts.cc
>> +++ b/gcc/cp/contracts.cc
>> @@ -88,64 +88,7 @@ along with GCC; see the file COPYING3.  If not see
>>         return -v;
>>       }
>>  -   The original decl is left alone and instead calls are generated to pre/post
>> -   functions within the body:
>> -
>> -     void fun.pre(int v)
>> -     {
>> -       [[ assert: v > 0 ]];
>> -     }
>> -     int fun.post(int v, int __r)
>> -     {
>> -       [[ assert: __r < 0 ]];
>> -       return __r;
>> -     }
>> -     int fun(int v)
>> -     {
>> -       fun.pre(v);
>> -       return fun.post(v, -v);
>> -     }
>> -
>> -   If fun returns in memory, the return value is not passed through the post
>> -   function; instead, the return object is initialized directly and then passed
>> -   to the post function by invisible reference.
>> -
>> -   This sides steps a number of issues with having to rewrite the bodies or
>> -   rewrite the parsed conditions as the parameters to the original function
>> -   changes (as happens during redeclaration). The ultimate goal is to get
>> -   something that optimizes well along the lines of
>> -
>> -     int fun(int v)
>> -     {
>> -       [[ assert: v > 0 ]];
>> -       auto &&__r = -v;
>> -       goto out;
>> -     out:
>> -       [[ assert: __r < 0 ]];
>> -       return __r;
>> -     }
>> -
>> -   With the idea being that multiple return statements could collapse the
>> -   function epilogue after inlining the pre/post functions. clang is able
>> -   to collapse common function epilogues, while gcc needs -O3 -Os combined.
>> -
>> -   Directly laying the pre contracts down in the function body doesn't have
>> -   many issues. The post contracts may need to be repeated multiple times, once
>> -   for each return, or a goto epilogue would need to be generated.
>> -   For this initial implementation, generating function calls and letting
>> -   later optimizations decide whether to inline and duplicate the actual
>> -   checks or whether to collapse the shared epilogue was chosen.
>> -
>> -   For cdtors a post contract is implemented using a CLEANUP_STMT.
>> -
>> -   FIXME the compiler already shores cleanup code on multiple exit paths, so
>> -   this outlining seems unnecessary if we represent the postcondition as a
>> -   cleanup for all functions.
>> -
>> -   More helpful for optimization might be to make the contracts a wrapper
>> -   function (for non-variadic functions), that could be inlined into a
>> -   caller while preserving the call to the actual function?  Either that or
>> -   mirror a never-continue post contract with an assume in the caller.  */
>> +  TODO: reiterate the revised implementation.  */
> 
> Please do.  Your patch comment above seems to be pretty close?
> 
>>    #include "config.h"
>>  #include "system.h"
>> @@ -801,7 +744,6 @@ finish_contract_attribute (tree identifier, tree contract)
>>    tree attribute = build_tree_list (build_tree_list (NULL_TREE, identifier),
>>  				    build_tree_list (NULL_TREE, contract));
>>  -
>>    /* Mark the attribute as dependent if the condition is dependent.
>>         TODO: I'm not sure this is strictly necessary. It's going to be marked as
>> @@ -1444,10 +1386,8 @@ build_contract_condition_function (tree fndecl, bool pre)
>>        *last = build_tree_list (NULL_TREE, value_type);
>>        TREE_CHAIN (*last) = void_list_node;
>>  -      if (aggregate_value_p (value_type, fndecl))
>> -	/* If FNDECL returns in memory, don't return the value from the
>> -	   postcondition.  */
>> -	value_type = void_type_node;
>> +      /* The handler is a void return.  */
>> +      value_type = void_type_node;
>>      }
>>      TREE_TYPE (fn) = build_function_type (value_type, arg_types);
>> @@ -1882,15 +1822,12 @@ emit_preconditions (tree attr)
>>    return emit_contract_conditions (attr, PRECONDITION_STMT);
>>  }
>>  -/* Emit statements for postcondition attributes.  */
>> +/* Emit statements for precondition attributes.  */
>>    static void
>> -emit_postconditions_cleanup (tree contracts)
>> +emit_postconditions (tree attr)
>>  {
>> -  tree stmts = push_stmt_list ();
>> -  emit_contract_conditions (contracts, POSTCONDITION_STMT);
>> -  stmts = pop_stmt_list (stmts);
>> -  push_cleanup (NULL_TREE, stmts, /*eh_only*/false);
>> +  return emit_contract_conditions (attr, POSTCONDITION_STMT);
>>  }
>>    /* We're compiling the pre/postcondition function CONDFN; remap any FN
>> @@ -1993,32 +1930,152 @@ start_function_contracts (tree decl1)
>>    if (!handle_contracts_p (decl1))
>>      return;
>>  +  /* For cdtors, we evaluate the contracts check inline.  */
>>    if (!outline_contracts_p (decl1))
>> -    {
>> -      emit_preconditions (DECL_CONTRACTS (current_function_decl));
>> -      emit_postconditions_cleanup (DECL_CONTRACTS (current_function_decl));
>> -      return;
>> -    }
>> +    return;
>>      /* Contracts may have just been added without a chance to parse them, though
>>       we still need the PRE_FN available to generate a call to it.  */
>>    if (!DECL_PRE_FN (decl1))
>>      build_contract_function_decls (decl1);
>>  +}
>> +
>> +/* If we have a precondition function and it's valid, call it.  */
>> +
>> +static void
>> +add_pre_condition_fn_call (tree fndecl)
>> +{
>>    /* If we're starting a guarded function with valid contracts, we need to
>>       insert a call to the pre function.  */
>> -  if (DECL_PRE_FN (decl1)
>> -      && DECL_PRE_FN (decl1) != error_mark_node)
>> +  gcc_checking_assert (DECL_PRE_FN (fndecl)
>> +      && DECL_PRE_FN (fndecl) != error_mark_node);
>> +
>> +  releasing_vec args = build_arg_list (fndecl);
>> +  tree call = build_call_a (DECL_PRE_FN (fndecl), args->length (),
>> +			    args->address ());
>> +  CALL_FROM_THUNK_P (call) = true;
>> +  finish_expr_stmt (call);
>> +}
>> +
>> +/* Build and add a call to the post-condition checking function, when that
>> +   is in use.  */
>> +
>> +static void
>> +add_post_condition_fn_call (tree fndecl)
>> +{
>> +  gcc_checking_assert (DECL_POST_FN (fndecl)
>> +      && DECL_POST_FN (fndecl) != error_mark_node);
>> +
>> +  releasing_vec args = build_arg_list (fndecl);
>> +  if (get_postcondition_result_parameter (fndecl))
>> +    vec_safe_push (args, DECL_RESULT (fndecl));
>> +  tree call = build_call_a (DECL_POST_FN (fndecl), args->length (),
>> +			    args->address ());
>> +  CALL_FROM_THUNK_P (call) = true;
>> +  finish_expr_stmt (call);
>> +}
>> +
>> +/* Add a call or a direct evaluation of the pre checks.  */
>> +
>> +static void
>> +apply_preconditions (tree fndecl)
>> +{
>> +  if (outline_contracts_p (fndecl))
>> +    add_pre_condition_fn_call (fndecl);
>> +  else
>> +    emit_preconditions (DECL_CONTRACTS (fndecl));
>> +}
>> +
>> +/* Add a call or a direct evaluation of the post checks.  */
>> +
>> +static void
>> +apply_postconditions (tree fndecl)
>> +{
>> +  if (outline_contracts_p (fndecl))
>> +    add_post_condition_fn_call (fndecl);
>> +  else
>> +    emit_postconditions (DECL_CONTRACTS (fndecl));
>> +}
>> +
>> +/* Add contract handling to the function in FNDECL.
>> +
>> +   When we have only pre-conditions, this simply prepends a call (or a direct
>> +   evaluation, for cdtors) to the existing function body.
>> +
>> +   When we have post conditions we build a try-finally block.
>> +   If the function might throw then the handler in the try-finally is an
>> +   EH_ELSE expression, where the post condition check is applied to the
>> +   non-exceptional path, and a rethrow is applied to the EH path.  If the
>> +   function has a non-throwing eh spec, then the handler is simply the post
>> +   contract checker (either a call or, for cdtors, a direct evaluation).  */
>> +
>> +void
>> +maybe_apply_function_contracts (tree fndecl)
>> +{
>> +  if (!handle_contracts_p (fndecl))
>> +    /* We did nothing and the original function body statement list will be
>> +       popped by our caller.  */
>> +    return;
>> +
>> +  bool do_pre = has_active_preconditions (fndecl);
>> +  bool do_post = has_active_postconditions (fndecl);
>> +  /* We should not have reached here with nothing to do... */
>> +  gcc_checking_assert (do_pre || do_post);
>> +
>> +  /* This copies the approach used for function try blocks.  */
>> +  tree fnbody = pop_stmt_list (DECL_SAVED_TREE (fndecl));
>> +  DECL_SAVED_TREE (fndecl) = push_stmt_list ();
>> +  tree compound_stmt = begin_compound_stmt (0);
>> +  current_binding_level->artificial = 1;
>> +
>> +  /* FIXME : find some better location to use - perhaps the position of the
>> +     function opening brace, if that is available.  */
>> +  location_t loc = UNKNOWN_LOCATION;
> 
> IMO it's actually best to leave it UNKNOWN to avoid problems with the debugger jumping back to the beginning of the function when processing cleanups.
> 
>> +  /* For other cases, we call a function to process the check.  */
>> +
>> +  /* IF we hsve a pre - but not a post, then just emit that.  */
>> +  if (!do_post)
>>      {
>> -      releasing_vec args = build_arg_list (decl1);
>> -      tree call = build_call_a (DECL_PRE_FN (decl1),
>> -				args->length (),
>> -				args->address ());
>> -      CALL_FROM_THUNK_P (call) = true;
>> -      finish_expr_stmt (call);
>> +      apply_preconditions (fndecl);
>> +      add_stmt (fnbody);
>> +      finish_compound_stmt (compound_stmt);
>> +      return;
>>      }
>> +
>> +  tree try_fin = build_stmt (loc, TRY_FINALLY_EXPR, NULL_TREE, NULL_TREE);
>> +  add_stmt (try_fin);
>> +  TREE_OPERAND (try_fin, 0) = push_stmt_list ();
>> +  if (do_pre)
>> +    /* Add a precondition call, if we have one. */
>> +    apply_preconditions (fndecl);
>> +  add_stmt (fnbody);
>> +  TREE_OPERAND (try_fin, 0) = pop_stmt_list (TREE_OPERAND (try_fin, 0));
>> +  TREE_OPERAND (try_fin, 1) = push_stmt_list ();
>> +  if (!type_noexcept_p (TREE_TYPE (fndecl)))
>> +    {
>> +      tree eh_else = build_stmt (loc, EH_ELSE_EXPR, NULL_TREE, NULL_TREE);
>> +      /* We need to ignore this in constexpr considerations.  */
>> +      CONTRACT_EH_ELSE_P (eh_else) = true;
>> +      add_stmt (eh_else);
>> +      TREE_OPERAND (eh_else, 0) = push_stmt_list ();
>> +      apply_postconditions (fndecl);
>> +      TREE_OPERAND (eh_else, 0) = pop_stmt_list (TREE_OPERAND (eh_else, 0));
>> +      TREE_OPERAND (eh_else, 1) = push_stmt_list ();
>> +      tree rethrow = build_throw (input_location, NULL_TREE, tf_warning_or_error);
>> +      suppress_warning (rethrow);
>> +      finish_expr_stmt (rethrow);
>> +      TREE_OPERAND (eh_else, 1) = pop_stmt_list (TREE_OPERAND (eh_else, 1));
>> +    }
>> +  else
>> +    apply_postconditions (fndecl);
>> +  TREE_OPERAND (try_fin, 1) = pop_stmt_list (TREE_OPERAND (try_fin, 1));
>> +  finish_compound_stmt (compound_stmt);
>> +  /* The DECL_SAVED_TREE stmt list will be popped by our caller.  */
>>  }
>>  +
> 
> Unnecessary extra line.
> 
>>  /* Finish up the pre & post function definitions for a guarded FNDECL,
>>     and compile those functions all the way to assembler language output.  */
>>  @@ -2074,34 +2131,6 @@ finish_function_contracts (tree fndecl)
>>      }
>>  }
>>  -/* Rewrite the expression of a returned expression so that it invokes the
>> -   postcondition function as needed.  */
>> -
>> -tree
>> -apply_postcondition_to_return (tree expr)
>> -{
>> -  tree fn = current_function_decl;
>> -  tree post = DECL_POST_FN (fn);
>> -  if (!post)
>> -    return NULL_TREE;
>> -
>> -  /* If FN returns in memory, POST has a void return type and we call it when
>> -     EXPR is DECL_RESULT (fn).  If FN returns a scalar, POST has the same
>> -     return type and we call it when EXPR is the value being returned.  */
>> -  if (VOID_TYPE_P (TREE_TYPE (TREE_TYPE (post)))
>> -      != (expr == DECL_RESULT (fn)))
>> -    return NULL_TREE;
>> -
>> -  releasing_vec args = build_arg_list (fn);
>> -  if (get_postcondition_result_parameter (fn))
>> -    vec_safe_push (args, expr);
>> -  tree call = build_call_a (post,
>> -			    args->length (),
>> -			    args->address ());
>> -  CALL_FROM_THUNK_P (call) = true;
>> -
>> -  return call;
>> -}
>>    /* A subroutine of duplicate_decls. Diagnose issues in the redeclaration of
>>     guarded functions.  */
>> diff --git a/gcc/cp/contracts.h b/gcc/cp/contracts.h
>> index 3e5c30db331..c786affd0e3 100644
>> --- a/gcc/cp/contracts.h
>> +++ b/gcc/cp/contracts.h
>> @@ -295,8 +295,9 @@ extern void update_late_contract		(tree, tree, tree);
>>  extern tree splice_out_contracts		(tree);
>>  extern bool all_attributes_are_contracts_p	(tree);
>>  extern void inherit_base_contracts		(tree, tree);
>> -extern tree apply_postcondition_to_return	(tree);
>> +//extern tree apply_postcondition_to_return	(tree);
> 
> This seems like a temporary thing that didn't get removed?
> 
>>  extern void start_function_contracts		(tree);
>> +extern void maybe_apply_function_contracts	(tree);
>>  extern void finish_function_contracts		(tree);
>>  extern void set_contract_functions		(tree, tree, tree);
>>  extern tree build_contract_check		(tree);
>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
>> index 97bc211ff67..f350fc33e9b 100644
>> --- a/gcc/cp/coroutines.cc
>> +++ b/gcc/cp/coroutines.cc
>> @@ -4014,6 +4014,8 @@ coro_build_actor_or_destroy_function (tree orig, tree fn_type,
>>    DECL_USER_ALIGN (fn) = DECL_USER_ALIGN (orig);
>>    /* Apply attributes from the original fn.  */
>>    DECL_ATTRIBUTES (fn) = copy_list (DECL_ATTRIBUTES (orig));
>> +  /* but we do not want ones for contracts.  */
>> +  remove_contract_attributes (fn);
>>      /* A void return.  */
>>    tree resdecl = build_decl (loc, RESULT_DECL, 0, void_type_node);
>> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
>> index 416c60b7311..505ee5d4dc9 100644
>> --- a/gcc/cp/cp-tree.h
>> +++ b/gcc/cp/cp-tree.h
>> @@ -451,6 +451,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
>>        ATOMIC_CONSTR_MAP_INSTANTIATED_P (in ATOMIC_CONSTR)
>>        contract_semantic (in ASSERTION_, PRECONDITION_, POSTCONDITION_STMT)
>>        RETURN_EXPR_LOCAL_ADDR_P (in RETURN_EXPR)
>> +      CONTRACT_EH_ELSE_P (in EH_ELSE_EXPR)
>>     1: IDENTIFIER_KIND_BIT_1 (in IDENTIFIER_NODE)
>>        TI_PENDING_TEMPLATE_FLAG.
>>        TEMPLATE_PARMS_FOR_INLINE.
>> @@ -723,6 +724,8 @@ typedef struct ptrmem_cst * ptrmem_cst_t;
>>    #define CLEANUP_P(NODE)		TREE_LANG_FLAG_0 (TRY_BLOCK_CHECK (NODE))
>>  +#define CONTRACT_EH_ELSE_P(NODE) TREE_LANG_FLAG_0 (EH_ELSE_EXPR_CHECK (NODE))
> 
> This would need a comment if we kept it.
> 
>> +
>>  #define BIND_EXPR_TRY_BLOCK(NODE) \
>>    TREE_LANG_FLAG_0 (BIND_EXPR_CHECK (NODE))
>>  diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
>> index 03deb1493a4..f3e733a29ba 100644
>> --- a/gcc/cp/decl.cc
>> +++ b/gcc/cp/decl.cc
>> @@ -18599,16 +18599,19 @@ finish_function (bool inline_p)
>>    tree fndecl = current_function_decl;
>>    tree fntype, ctype = NULL_TREE;
>>    tree resumer = NULL_TREE, destroyer = NULL_TREE;
>> -  bool coro_p = flag_coroutines
>> -		&& !processing_template_decl
>> -		&& DECL_COROUTINE_P (fndecl);
>> -  bool coro_emit_helpers = false;
>>      /* When we get some parse errors, we can end up without a
>>       current_function_decl, so cope.  */
>> -  if (fndecl == NULL_TREE)
>> +  if (fndecl == NULL_TREE || fndecl == error_mark_node)
>>      return error_mark_node;
>>  +  bool coro_p = flag_coroutines
>> +		&& !processing_template_decl
>> +		&& DECL_COROUTINE_P (fndecl);
>> +  bool coro_emit_helpers = false;
>> +  bool do_contracts = DECL_HAS_CONTRACTS_P (fndecl)
>> +		      && !processing_template_decl;
>> +
>>    if (!DECL_OMP_DECLARE_REDUCTION_P (fndecl))
>>      finish_lambda_scope ();
>>  @@ -18644,6 +18647,10 @@ finish_function (bool inline_p)
>>  	finish_eh_spec_block (TYPE_RAISES_EXCEPTIONS
>>  			      (TREE_TYPE (fndecl)),
>>  			      current_eh_spec_block);
>> +
>> +     /* If outlining succeeded, then add contracts handling if needed.  */
>> +     if (coro_emit_helpers && do_contracts)
>> +	maybe_apply_function_contracts (fndecl);
>>      }
>>    else
>>    /* For a cloned function, we've already got all the code we need;
>> @@ -18659,6 +18666,10 @@ finish_function (bool inline_p)
>>  	finish_eh_spec_block (TYPE_RAISES_EXCEPTIONS
>>  			      (TREE_TYPE (current_function_decl)),
>>  			      current_eh_spec_block);
>> +
>> +     if (do_contracts)
>> +	maybe_apply_function_contracts (current_function_decl);
>> +
>>      }
>>      /* If we're saving up tree structure, tie off the function now.  */
>> @@ -18911,7 +18922,7 @@ finish_function (bool inline_p)
>>    --function_depth;
>>      /* Clean up.  */
>> -  current_function_decl = NULL_TREE;
>> +  gcc_checking_assert (current_function_decl == NULL_TREE);
>>      invoke_plugin_callbacks (PLUGIN_FINISH_PARSE_FUNCTION, fndecl);
>>  diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
>> index 5970ac3d398..4434ba6e2f4 100644
>> --- a/gcc/cp/typeck.cc
>> +++ b/gcc/cp/typeck.cc
>> @@ -11416,13 +11416,7 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
>>      /* Actually copy the value returned into the appropriate location.  */
>>    if (retval && retval != result)
>> -    {
>> -      /* If there's a postcondition for a scalar return value, wrap
>> -	 retval in a call to the postcondition function.  */
>> -      if (tree post = apply_postcondition_to_return (retval))
>> -	retval = post;
>> -      retval = cp_build_init_expr (result, retval);
>> -    }
>> +    retval = cp_build_init_expr (result, retval);
>>      if (current_function_return_value == bare_retval)
>>      INIT_EXPR_NRV_P (retval) = true;
>> @@ -11430,11 +11424,6 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
>>    if (tree set = maybe_set_retval_sentinel ())
>>      retval = build2 (COMPOUND_EXPR, void_type_node, retval, set);
>>  -  /* If there's a postcondition for an aggregate return value, call the
>> -     postcondition function after the return object is initialized.  */
>> -  if (tree post = apply_postcondition_to_return (result))
>> -    retval = build2 (COMPOUND_EXPR, void_type_node, retval, post);
>> -
>>    return retval;
>>  }
>>  diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
>> index 622c51d5c3f..43bd4c56d7f 100644
>> --- a/gcc/gimplify.cc
>> +++ b/gcc/gimplify.cc
>> @@ -227,6 +227,7 @@ struct gimplify_ctx
>>    unsigned keep_stack : 1;
>>    unsigned save_stack : 1;
>>    unsigned in_switch_expr : 1;
>> +  unsigned in_handler_expr : 1;
>>  };
>>    enum gimplify_defaultmap_kind
>> @@ -18401,10 +18402,12 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
>>  	    input_location = UNKNOWN_LOCATION;
>>  	    eval = cleanup = NULL;
>>  	    gimplify_and_add (TREE_OPERAND (*expr_p, 0), &eval);
>> +	    bool save_in_handler_expr = gimplify_ctxp->in_handler_expr;
>>  	    if (TREE_CODE (*expr_p) == TRY_FINALLY_EXPR
>>  		&& TREE_CODE (TREE_OPERAND (*expr_p, 1)) == EH_ELSE_EXPR)
>>  	      {
>>  		gimple_seq n = NULL, e = NULL;
>> +		gimplify_ctxp->in_handler_expr = true;
>>  		gimplify_and_add (TREE_OPERAND (TREE_OPERAND (*expr_p, 1),
>>  						0), &n);
>>  		gimplify_and_add (TREE_OPERAND (TREE_OPERAND (*expr_p, 1),
>> @@ -18416,7 +18419,11 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
>>  		  }
>>  	      }
>>  	    else
>> -	      gimplify_and_add (TREE_OPERAND (*expr_p, 1), &cleanup);
>> +	      {
>> +		gimplify_ctxp->in_handler_expr = true;
>> +		gimplify_and_add (TREE_OPERAND (*expr_p, 1), &cleanup);
>> +	      }
>> +	    gimplify_ctxp->in_handler_expr = save_in_handler_expr;
>>  	    /* Don't create bogus GIMPLE_TRY with empty cleanup.  */
>>  	    if (gimple_seq_empty_p (cleanup))
>>  	      {
>> @@ -18516,6 +18523,10 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
>>  	  /* When within an OMP context, notice uses of variables.  */
>>  	  if (gimplify_omp_ctxp)
>>  	    omp_notice_variable (gimplify_omp_ctxp, *expr_p, true);
>> +	  /* Handlers can refer to the function result; if that has been
>> +	     moved, we need to track it.  */
>> +	  if (gimplify_ctxp->in_handler_expr && gimplify_ctxp->return_temp)
>> +	    *expr_p = gimplify_ctxp->return_temp;
>>  	  ret = GS_ALL_DONE;
>>  	  break;
>>  diff --git a/gcc/testsuite/g++.dg/contracts/pr115434.C b/gcc/testsuite/g++.dg/contracts/pr115434.C
>> new file mode 100644
>> index 00000000000..e9c847f8969
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/contracts/pr115434.C
>> @@ -0,0 +1,16 @@
>> +// We were failing to apply post conditions to void functions.
>> +
>> +// { dg-do run }
>> +// { dg-options "-std=c++20 -fcontracts -fcontract-continuation-mode=on" }
>> +
>> +
>> +void foo (const int b)
>> +[[ post: b == 9 ]]  // contract not checked
>> +{}
>> +
>> +int main()
>> +{
>> +   foo(3);
>> +}
>> +
>> +// { dg-output "contract violation in function foo at .*.C:8: b == 9.*(\n|\r\n|\r)" }
>> diff --git a/gcc/testsuite/g++.dg/coroutines/pr110871.C b/gcc/testsuite/g++.dg/coroutines/pr110871.C
>> new file mode 100644
>> index 00000000000..8a667c856ae
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/coroutines/pr110871.C
>> @@ -0,0 +1,62 @@
>> +// { dg-additional-options "-fcontracts -fcontract-continuation-mode=on" }
>> +// { dg-do run }
>> +#include <iostream>
>> +#include <coroutine>
>> +
>> +// In order to test the contract violation diagnostic, we have to set
>> +// -fcontract-continuation-mode=on; this means that the code will emit
>> +// the message below - but before that the contract should have been checked.
>> +void process(int from, int to)
>> +{
>> +  if (from > to)
>> +    std::cout << "would have been a disaster!" << std::endl;
>> +}
>> +
>> +template <typename T>
>> +struct generator
>> +{
>> +    struct promise_type
>> +    {
>> +        template <typename... Args>
>> +        promise_type(Args&&... args) {
>> +            std::cout << "promise init" << std::endl;
>> +            process(args...);
>> +        }
>> +
>> +        std::suspend_always yield_value(T) { return {}; }
>> +
>> +        std::suspend_always initial_suspend() const noexcept { return {}; }
>> +        std::suspend_never final_suspend() const noexcept { return {}; }
>> +        void unhandled_exception() noexcept {}
>> +
>> +        generator<T> get_return_object() noexcept { return {}; }
>> +    };
>> +};
>> +
>> +namespace std {
>> +template <typename T, typename... Args>
>> +struct coroutine_traits<generator<T>, Args...>
>> +{
>> +    using promise_type = typename generator<T>::promise_type;
>> +};
>> +
>> +};
>> +
>> +generator<int> seq(int from, int to) [[pre: from <= to]]
>> +
>> +{
>> +    std::cout << "coro initial" << std::endl;
>> +    for (int i = from; i <= to; ++i) {
>> +        co_yield i;
>> +        std::cout << "coro resumed" << std::endl;
>> +    }
>> +}
>> +
>> +int main() {
>> +    std::cout << "main initial" << std::endl;
>> +    generator<int> s = seq(10, 5);
>> +    (void)s;
>> +    std::cout << "main continues" << std::endl;
>> +}
>> +
>> +// { dg-output "contract violation in function seq at .*.C:45: from \<= to.*(\n|\r\n|\r)" }
>> diff --git a/gcc/testsuite/g++.dg/coroutines/pr110872.C b/gcc/testsuite/g++.dg/coroutines/pr110872.C
>> new file mode 100644
>> index 00000000000..a809986f296
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/coroutines/pr110872.C
>> @@ -0,0 +1,49 @@
>> +// { dg-additional-options "-fcontracts -fcontract-continuation-mode=on" }
>> +// { dg-do run }
>> +
>> +#include <iostream>
>> +#include <coroutine>
>> +
>> +
>> +template <typename T>
>> +struct generator
>> +{
>> +    struct promise_type
>> +    {
>> +        std::suspend_always yield_value(T) { return {}; }
>> +
>> +        std::suspend_always initial_suspend() const noexcept { return {}; }
>> +        std::suspend_never final_suspend() const noexcept { return {}; }
>> +        void unhandled_exception() noexcept {}
>> +
>> +        generator<T> get_return_object() noexcept { return {}; }
>> +    };
>> +
>> +    bool is_valid() { return false; }
>> +};
>> +
>> +namespace std {
>> +template <typename T, typename... Args>
>> +struct coroutine_traits<generator<T>, Args...>
>> +{
>> +    using promise_type = typename generator<T>::promise_type;
>> +};
>> +
>> +};
>> +
>> +generator<int> val(int v)
>> +[[post g: g.is_valid()]]
>> +{
>> +    std::cout << "coro initial" << std::endl;
>> +    co_yield v;
>> +    std::cout << "coro resumed" << std::endl;
>> +}
>> +
>> +int main() {
>> +    std::cout << "main initial" << std::endl;
>> +    generator<int> s = val(1);
>> +    (void)s;
>> +    std::cout << "main continues" << std::endl;
>> +}
>> +
>> +// { dg-output "contract violation in function val at .*.C:35: g.is_valid().*(\n|\r\n|\r)" }

On 7/8/24 3:37 PM, Iain Sandoe wrote:
> Hello Jason,
> 
> before re-working, I think I need some guidance.
> 
>> On 8 Jul 2024, at 20:19, Jason Merrill <jason@redhat.com> wrote:
>>
>> On 6/17/24 8:15 AM, Iain Sandoe wrote:
>>> This patch came out of a discussion on Mattermost about how to deal
>>> with contracts/coroutines integration.  Actually, it would also allow
>>> some semantic checking to be deferred until the same spot - at which
>>> time there are no dependent types, which can simplify the process.
>>> NOTE: this is a fix for bugs in the existing '2a' contracts impl. it
>>> does not attempt to make any of the changes required by P2900 to
>>> either code-gen or constexpr handling.
>>> Tested on x86_64-darwin, so far, OK for trunk if testing succeeds on
>>> x86_64/powerpc64 linux too?
>>> thanks,
>>> Iain
>>> --- 8< ---
>>> The current implementation of contracts emits the checks into function
>>> bodies in three places; for pre-conditions at the start of the body,
>>> for asserts in-line in the function body and for post-conditions as an
>>> addition to return statements.
>>> In general (at least with existing "2a" contract semantics) the in-line
>>> contract asserts behave as expected.
>>> However, the mechanism is not applicable to:
>>>   * Handling pre conditions in coroutines since, for those, the standard
>>>    specifies a wrapping of the original function body by functionality
>>>    implementing initial and final suspends (along with some housekeeping
>>>    to route exceptions).  Thus for such transformed function bodies, the
>>>    preconditions then get actioned after the initial suspend, which does
>>>    not behave as intended.
>>>    * Handling post conditions in functions that do not have return
>>>      statements (which applies to coroutines and void functions).
>>> In the following, we identify a potentially transformed function body
>>> (in the case of coroutines, this is usually called the "ramp()" function).
>>> The patch here re-implements the code insertion in one of the two following
>>> ways (code for exposition only):
>>>    * For functions with no post-conditions we wrap the potentially
>>>      transformed function as follows:
>>
>> To support caller-side checking, I wonder about (not in this patch) doing the wrapping in a separate function, instead of outlining the condition evaluation?
> 
> ack,
> note that, following Josh’s presentation to EWG, we  are [well Nina is] going to be implementing caller-side contracts in support of the new virtual function model.  I think we need to consider the two things together.

Makes sense.

>>>    {
>>>       handle_pre_condition_checking ();
>>>       potentially_transformed_function_body ();
>>>    }
>>>    This implements the intent that the preconditions are processed after
>>>    the function parameters are initialised but before any other actions.
>>>    * For functions with post-conditions:
>>>    try
>>>     {
>>>       if (preconditions_exist)
>>>         handle_pre_condition_checking ();
>>
>> Why are the preconditions within the try?
> 
> It was the intention that if any EH actions were needed to implement the semantics of P2900 that would be handled.
> They do not need to be given your comments below.
> 
>>>       potentially_transformed_function_body ();
>>>     }
>>>    finally
>>>     {
>>>       handle_post_condition_checking ();
>>>     }
>>>    else [only if the function is not marked noexcept(true) ]
>>>     {
>>>       __rethrow ();
>>
>> This sounds undesirable, EH normally continues unwinding after running a cleanup.  Why would the exception be considered caught in a way that needs a rethrow?
> 
> We need to implement that the no-EH cleanup [i.e. the postcondition] does _not_ run when an exception is thrown.  Right now there’s no HANDLER type that does that - we have EH_ONLY but no NON_EH one (otherwise I'd have built a C++ try).
> 
> however, I could not figure out how to make this arm of the EH_ELSE_EXPR work as an empty construct (the gimplifier does not seem to expect this).

You can't just put void_node in the else arm?

> I suppose another possibility is to keep the contracts trees until gimplification time and lower them directly to the gimple equivalent - but I’m not sure it would solve the underlying issue - we need a “non-eh-only edges” cleanup.

We could add a CLEANUP_NON_EH_ONLY flag, which cp-gimplify would then 
lower to this same TRY_FINALLY_EXPR+EH_ELSE_EXPR structure, but that 
seems unnecessary until we have another use for it.

>>>     }
>>> In this, post-conditions [that might apply to the return value etc.]
>>> are evaluated on every non-exceptional edge out of the function.
> 
> 
>>> At present, the model here is that exceptions thrown by the function
>>> propagate upwards as if there were no contracts present.  If the desired
>>> semantic becomes that an exception is counted as equivalent to a contract
>>> violation - then we can add a second handler in place of the rethrow.
>>> At constexpr time we need to evaluate the contract conditions, but not
>>> the exceptional path, which is handled by a flag on the EH_ELSE_EXPR that
>>> indicates it is in use for contract handling.
>>
>> Why would we ever want to evaluate the exceptional path when we aren't throwing an exception?
> 
> We wouldn’t; my explanation is not adequate - the constexpr recursion fails if it reaches (the likely unneccessary) rethrow - we need to prevent that.

Yes, constexpr should unconditionally ignore operand 1 of an 
EH_ELSE_EXPR, rather than ignoring it only if CONTRACT_EH_ELSE_P is set.

> the remainder of your comments below I can handle with the revised edition, but I’d welcome comments on the non-eh-only cleanup.
> 
> thanks for the review
> Iain
> 
> 
>>> This patch specifically does not address changes to code-gen and constexpr
>>> handling that are contained in P2900.
>>> 	PR c++/115434
>>> 	PR c++/110871
>>> 	PR c++/110872
>>> gcc/cp/ChangeLog:
>>> 	* constexpr.cc (cxx_eval_constant_expression): Handle EH_ELSE_EXPR.
>>> 	* contracts.cc (finish_contract_attribute): Remove excess line.
>>> 	(build_contract_condition_function): Post condition handlers are
>>> 	void now.
>>> 	(emit_postconditions_cleanup): Remove.
>>> 	(emit_postconditions): New.
>>> 	(add_pre_condition_fn_call): New.
>>> 	(add_post_condition_fn_call): New.
>>> 	(apply_preconditions): New.
>>> 	(apply_postconditions): New.
>>> 	(maybe_apply_function_contracts): New.
>>> 	(apply_postcondition_to_return): Remove.
>>> 	* contracts.h (apply_postcondition_to_return): Remove.
>>> 	(maybe_apply_function_contracts): Add.
>>> 	* coroutines.cc (coro_build_actor_or_destroy_function): Do not
>>> 	copy contracts to coroutine helpers.
>>> 	* cp-tree.h (CONTRACT_EH_ELSE_P): New.
>>> 	* decl.cc (finish_function): Handle wrapping a possibly
>>> 	transformed function body in contract checks.
>>> 	* typeck.cc (check_return_expr): Remove handling of post
>>> 	conditions on return expressions.
>>> gcc/ChangeLog:
>>> 	* gimplify.cc (struct gimplify_ctx): Add a flag to show we are
>>> 	expending a handler.
>>> 	(gimplify_expr): When we are expanding a handler, and the body
>>> 	transforms might have re-written DECL_RESULT into a gimple var,
>>> 	ensure that hander references to DECL_RESULT are also re-written
>>> 	to refer to the gimple var.
>>> gcc/testsuite/ChangeLog:
>>> 	* g++.dg/contracts/pr115434.C: New test.
>>> 	* g++.dg/coroutines/pr110871.C: New test.
>>> 	* g++.dg/coroutines/pr110872.C: New test.
>>> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
>>> ---
>>>   gcc/cp/constexpr.cc                        |  16 ++
>>>   gcc/cp/contracts.cc                        | 249 ++++++++++++---------
>>>   gcc/cp/contracts.h                         |   3 +-
>>>   gcc/cp/coroutines.cc                       |   2 +
>>>   gcc/cp/cp-tree.h                           |   3 +
>>>   gcc/cp/decl.cc                             |  23 +-
>>>   gcc/cp/typeck.cc                           |  13 +-
>>>   gcc/gimplify.cc                            |  13 +-
>>>   gcc/testsuite/g++.dg/contracts/pr115434.C  |  16 ++
>>>   gcc/testsuite/g++.dg/coroutines/pr110871.C |  62 +++++
>>>   gcc/testsuite/g++.dg/coroutines/pr110872.C |  49 ++++
>>>   11 files changed, 319 insertions(+), 130 deletions(-)
>>>   create mode 100644 gcc/testsuite/g++.dg/contracts/pr115434.C
>>>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr110871.C
>>>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr110872.C
>>> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
>>> index bd72533491e..9b66b1753ad 100644
>>> --- a/gcc/cp/constexpr.cc
>>> +++ b/gcc/cp/constexpr.cc
>>> @@ -7808,6 +7808,22 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
>>>   				      non_constant_p, overflow_p);
>>>         break;
>>>   +    case EH_ELSE_EXPR:
>>> +      /* Evaluate any cleanup that applies to non-EH exits, this only for
>>> +	 the output of the diagnostics ??? what is really meant to happen
>>> +	 at constexpr-time?...  */
>>> +      cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0), vc_discard,
>>> +				      non_constant_p, overflow_p);
>>> +
>>> +      /* The presence of a contract should not affect the constexpr.  */
>>> +      if (CONTRACT_EH_ELSE_P (t))
>>> +	break;
>>> +      if (!*non_constant_p)
>>> +	/* Also evaluate the EH handler.  */
>>
>> As mentioned above, this seems undesirable until we get EH in constexpr.
>>
>>> +	cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 1), vc_discard,
>>> +				      non_constant_p, overflow_p);
>>> +      break;
>>> +
>>>       case CLEANUP_STMT:
>>>         r = cxx_eval_constant_expression (ctx, CLEANUP_BODY (t), lval,
>>>   					non_constant_p, overflow_p,
>>> diff --git a/gcc/cp/contracts.cc b/gcc/cp/contracts.cc
>>> index 0822624a910..064f0b6a9d8 100644
>>> --- a/gcc/cp/contracts.cc
>>> +++ b/gcc/cp/contracts.cc
>>> @@ -88,64 +88,7 @@ along with GCC; see the file COPYING3.  If not see
>>>          return -v;
>>>        }
>>>   -   The original decl is left alone and instead calls are generated to pre/post
>>> -   functions within the body:
>>> -
>>> -     void fun.pre(int v)
>>> -     {
>>> -       [[ assert: v > 0 ]];
>>> -     }
>>> -     int fun.post(int v, int __r)
>>> -     {
>>> -       [[ assert: __r < 0 ]];
>>> -       return __r;
>>> -     }
>>> -     int fun(int v)
>>> -     {
>>> -       fun.pre(v);
>>> -       return fun.post(v, -v);
>>> -     }
>>> -
>>> -   If fun returns in memory, the return value is not passed through the post
>>> -   function; instead, the return object is initialized directly and then passed
>>> -   to the post function by invisible reference.
>>> -
>>> -   This sides steps a number of issues with having to rewrite the bodies or
>>> -   rewrite the parsed conditions as the parameters to the original function
>>> -   changes (as happens during redeclaration). The ultimate goal is to get
>>> -   something that optimizes well along the lines of
>>> -
>>> -     int fun(int v)
>>> -     {
>>> -       [[ assert: v > 0 ]];
>>> -       auto &&__r = -v;
>>> -       goto out;
>>> -     out:
>>> -       [[ assert: __r < 0 ]];
>>> -       return __r;
>>> -     }
>>> -
>>> -   With the idea being that multiple return statements could collapse the
>>> -   function epilogue after inlining the pre/post functions. clang is able
>>> -   to collapse common function epilogues, while gcc needs -O3 -Os combined.
>>> -
>>> -   Directly laying the pre contracts down in the function body doesn't have
>>> -   many issues. The post contracts may need to be repeated multiple times, once
>>> -   for each return, or a goto epilogue would need to be generated.
>>> -   For this initial implementation, generating function calls and letting
>>> -   later optimizations decide whether to inline and duplicate the actual
>>> -   checks or whether to collapse the shared epilogue was chosen.
>>> -
>>> -   For cdtors a post contract is implemented using a CLEANUP_STMT.
>>> -
>>> -   FIXME the compiler already shores cleanup code on multiple exit paths, so
>>> -   this outlining seems unnecessary if we represent the postcondition as a
>>> -   cleanup for all functions.
>>> -
>>> -   More helpful for optimization might be to make the contracts a wrapper
>>> -   function (for non-variadic functions), that could be inlined into a
>>> -   caller while preserving the call to the actual function?  Either that or
>>> -   mirror a never-continue post contract with an assume in the caller.  */
>>> +  TODO: reiterate the revised implementation.  */
>>
>> Please do.  Your patch comment above seems to be pretty close?
>>
>>>     #include "config.h"
>>>   #include "system.h"
>>> @@ -801,7 +744,6 @@ finish_contract_attribute (tree identifier, tree contract)
>>>     tree attribute = build_tree_list (build_tree_list (NULL_TREE, identifier),
>>>   				    build_tree_list (NULL_TREE, contract));
>>>   -
>>>     /* Mark the attribute as dependent if the condition is dependent.
>>>          TODO: I'm not sure this is strictly necessary. It's going to be marked as
>>> @@ -1444,10 +1386,8 @@ build_contract_condition_function (tree fndecl, bool pre)
>>>         *last = build_tree_list (NULL_TREE, value_type);
>>>         TREE_CHAIN (*last) = void_list_node;
>>>   -      if (aggregate_value_p (value_type, fndecl))
>>> -	/* If FNDECL returns in memory, don't return the value from the
>>> -	   postcondition.  */
>>> -	value_type = void_type_node;
>>> +      /* The handler is a void return.  */
>>> +      value_type = void_type_node;
>>>       }
>>>       TREE_TYPE (fn) = build_function_type (value_type, arg_types);
>>> @@ -1882,15 +1822,12 @@ emit_preconditions (tree attr)
>>>     return emit_contract_conditions (attr, PRECONDITION_STMT);
>>>   }
>>>   -/* Emit statements for postcondition attributes.  */
>>> +/* Emit statements for precondition attributes.  */
>>>     static void
>>> -emit_postconditions_cleanup (tree contracts)
>>> +emit_postconditions (tree attr)
>>>   {
>>> -  tree stmts = push_stmt_list ();
>>> -  emit_contract_conditions (contracts, POSTCONDITION_STMT);
>>> -  stmts = pop_stmt_list (stmts);
>>> -  push_cleanup (NULL_TREE, stmts, /*eh_only*/false);
>>> +  return emit_contract_conditions (attr, POSTCONDITION_STMT);
>>>   }
>>>     /* We're compiling the pre/postcondition function CONDFN; remap any FN
>>> @@ -1993,32 +1930,152 @@ start_function_contracts (tree decl1)
>>>     if (!handle_contracts_p (decl1))
>>>       return;
>>>   +  /* For cdtors, we evaluate the contracts check inline.  */
>>>     if (!outline_contracts_p (decl1))
>>> -    {
>>> -      emit_preconditions (DECL_CONTRACTS (current_function_decl));
>>> -      emit_postconditions_cleanup (DECL_CONTRACTS (current_function_decl));
>>> -      return;
>>> -    }
>>> +    return;
>>>       /* Contracts may have just been added without a chance to parse them, though
>>>        we still need the PRE_FN available to generate a call to it.  */
>>>     if (!DECL_PRE_FN (decl1))
>>>       build_contract_function_decls (decl1);
>>>   +}
>>> +
>>> +/* If we have a precondition function and it's valid, call it.  */
>>> +
>>> +static void
>>> +add_pre_condition_fn_call (tree fndecl)
>>> +{
>>>     /* If we're starting a guarded function with valid contracts, we need to
>>>        insert a call to the pre function.  */
>>> -  if (DECL_PRE_FN (decl1)
>>> -      && DECL_PRE_FN (decl1) != error_mark_node)
>>> +  gcc_checking_assert (DECL_PRE_FN (fndecl)
>>> +      && DECL_PRE_FN (fndecl) != error_mark_node);
>>> +
>>> +  releasing_vec args = build_arg_list (fndecl);
>>> +  tree call = build_call_a (DECL_PRE_FN (fndecl), args->length (),
>>> +			    args->address ());
>>> +  CALL_FROM_THUNK_P (call) = true;
>>> +  finish_expr_stmt (call);
>>> +}
>>> +
>>> +/* Build and add a call to the post-condition checking function, when that
>>> +   is in use.  */
>>> +
>>> +static void
>>> +add_post_condition_fn_call (tree fndecl)
>>> +{
>>> +  gcc_checking_assert (DECL_POST_FN (fndecl)
>>> +      && DECL_POST_FN (fndecl) != error_mark_node);
>>> +
>>> +  releasing_vec args = build_arg_list (fndecl);
>>> +  if (get_postcondition_result_parameter (fndecl))
>>> +    vec_safe_push (args, DECL_RESULT (fndecl));
>>> +  tree call = build_call_a (DECL_POST_FN (fndecl), args->length (),
>>> +			    args->address ());
>>> +  CALL_FROM_THUNK_P (call) = true;
>>> +  finish_expr_stmt (call);
>>> +}
>>> +
>>> +/* Add a call or a direct evaluation of the pre checks.  */
>>> +
>>> +static void
>>> +apply_preconditions (tree fndecl)
>>> +{
>>> +  if (outline_contracts_p (fndecl))
>>> +    add_pre_condition_fn_call (fndecl);
>>> +  else
>>> +    emit_preconditions (DECL_CONTRACTS (fndecl));
>>> +}
>>> +
>>> +/* Add a call or a direct evaluation of the post checks.  */
>>> +
>>> +static void
>>> +apply_postconditions (tree fndecl)
>>> +{
>>> +  if (outline_contracts_p (fndecl))
>>> +    add_post_condition_fn_call (fndecl);
>>> +  else
>>> +    emit_postconditions (DECL_CONTRACTS (fndecl));
>>> +}
>>> +
>>> +/* Add contract handling to the function in FNDECL.
>>> +
>>> +   When we have only pre-conditions, this simply prepends a call (or a direct
>>> +   evaluation, for cdtors) to the existing function body.
>>> +
>>> +   When we have post conditions we build a try-finally block.
>>> +   If the function might throw then the handler in the try-finally is an
>>> +   EH_ELSE expression, where the post condition check is applied to the
>>> +   non-exceptional path, and a rethrow is applied to the EH path.  If the
>>> +   function has a non-throwing eh spec, then the handler is simply the post
>>> +   contract checker (either a call or, for cdtors, a direct evaluation).  */
>>> +
>>> +void
>>> +maybe_apply_function_contracts (tree fndecl)
>>> +{
>>> +  if (!handle_contracts_p (fndecl))
>>> +    /* We did nothing and the original function body statement list will be
>>> +       popped by our caller.  */
>>> +    return;
>>> +
>>> +  bool do_pre = has_active_preconditions (fndecl);
>>> +  bool do_post = has_active_postconditions (fndecl);
>>> +  /* We should not have reached here with nothing to do... */
>>> +  gcc_checking_assert (do_pre || do_post);
>>> +
>>> +  /* This copies the approach used for function try blocks.  */
>>> +  tree fnbody = pop_stmt_list (DECL_SAVED_TREE (fndecl));
>>> +  DECL_SAVED_TREE (fndecl) = push_stmt_list ();
>>> +  tree compound_stmt = begin_compound_stmt (0);
>>> +  current_binding_level->artificial = 1;
>>> +
>>> +  /* FIXME : find some better location to use - perhaps the position of the
>>> +     function opening brace, if that is available.  */
>>> +  location_t loc = UNKNOWN_LOCATION;
>>
>> IMO it's actually best to leave it UNKNOWN to avoid problems with the debugger jumping back to the beginning of the function when processing cleanups.
>>
>>> +  /* For other cases, we call a function to process the check.  */
>>> +
>>> +  /* IF we hsve a pre - but not a post, then just emit that.  */
>>> +  if (!do_post)
>>>       {
>>> -      releasing_vec args = build_arg_list (decl1);
>>> -      tree call = build_call_a (DECL_PRE_FN (decl1),
>>> -				args->length (),
>>> -				args->address ());
>>> -      CALL_FROM_THUNK_P (call) = true;
>>> -      finish_expr_stmt (call);
>>> +      apply_preconditions (fndecl);
>>> +      add_stmt (fnbody);
>>> +      finish_compound_stmt (compound_stmt);
>>> +      return;
>>>       }
>>> +
>>> +  tree try_fin = build_stmt (loc, TRY_FINALLY_EXPR, NULL_TREE, NULL_TREE);
>>> +  add_stmt (try_fin);
>>> +  TREE_OPERAND (try_fin, 0) = push_stmt_list ();
>>> +  if (do_pre)
>>> +    /* Add a precondition call, if we have one. */
>>> +    apply_preconditions (fndecl);
>>> +  add_stmt (fnbody);
>>> +  TREE_OPERAND (try_fin, 0) = pop_stmt_list (TREE_OPERAND (try_fin, 0));
>>> +  TREE_OPERAND (try_fin, 1) = push_stmt_list ();
>>> +  if (!type_noexcept_p (TREE_TYPE (fndecl)))
>>> +    {
>>> +      tree eh_else = build_stmt (loc, EH_ELSE_EXPR, NULL_TREE, NULL_TREE);
>>> +      /* We need to ignore this in constexpr considerations.  */
>>> +      CONTRACT_EH_ELSE_P (eh_else) = true;
>>> +      add_stmt (eh_else);
>>> +      TREE_OPERAND (eh_else, 0) = push_stmt_list ();
>>> +      apply_postconditions (fndecl);
>>> +      TREE_OPERAND (eh_else, 0) = pop_stmt_list (TREE_OPERAND (eh_else, 0));
>>> +      TREE_OPERAND (eh_else, 1) = push_stmt_list ();
>>> +      tree rethrow = build_throw (input_location, NULL_TREE, tf_warning_or_error);
>>> +      suppress_warning (rethrow);
>>> +      finish_expr_stmt (rethrow);
>>> +      TREE_OPERAND (eh_else, 1) = pop_stmt_list (TREE_OPERAND (eh_else, 1));
>>> +    }
>>> +  else
>>> +    apply_postconditions (fndecl);
>>> +  TREE_OPERAND (try_fin, 1) = pop_stmt_list (TREE_OPERAND (try_fin, 1));
>>> +  finish_compound_stmt (compound_stmt);
>>> +  /* The DECL_SAVED_TREE stmt list will be popped by our caller.  */
>>>   }
>>>   +
>>
>> Unnecessary extra line.
>>
>>>   /* Finish up the pre & post function definitions for a guarded FNDECL,
>>>      and compile those functions all the way to assembler language output.  */
>>>   @@ -2074,34 +2131,6 @@ finish_function_contracts (tree fndecl)
>>>       }
>>>   }
>>>   -/* Rewrite the expression of a returned expression so that it invokes the
>>> -   postcondition function as needed.  */
>>> -
>>> -tree
>>> -apply_postcondition_to_return (tree expr)
>>> -{
>>> -  tree fn = current_function_decl;
>>> -  tree post = DECL_POST_FN (fn);
>>> -  if (!post)
>>> -    return NULL_TREE;
>>> -
>>> -  /* If FN returns in memory, POST has a void return type and we call it when
>>> -     EXPR is DECL_RESULT (fn).  If FN returns a scalar, POST has the same
>>> -     return type and we call it when EXPR is the value being returned.  */
>>> -  if (VOID_TYPE_P (TREE_TYPE (TREE_TYPE (post)))
>>> -      != (expr == DECL_RESULT (fn)))
>>> -    return NULL_TREE;
>>> -
>>> -  releasing_vec args = build_arg_list (fn);
>>> -  if (get_postcondition_result_parameter (fn))
>>> -    vec_safe_push (args, expr);
>>> -  tree call = build_call_a (post,
>>> -			    args->length (),
>>> -			    args->address ());
>>> -  CALL_FROM_THUNK_P (call) = true;
>>> -
>>> -  return call;
>>> -}
>>>     /* A subroutine of duplicate_decls. Diagnose issues in the redeclaration of
>>>      guarded functions.  */
>>> diff --git a/gcc/cp/contracts.h b/gcc/cp/contracts.h
>>> index 3e5c30db331..c786affd0e3 100644
>>> --- a/gcc/cp/contracts.h
>>> +++ b/gcc/cp/contracts.h
>>> @@ -295,8 +295,9 @@ extern void update_late_contract		(tree, tree, tree);
>>>   extern tree splice_out_contracts		(tree);
>>>   extern bool all_attributes_are_contracts_p	(tree);
>>>   extern void inherit_base_contracts		(tree, tree);
>>> -extern tree apply_postcondition_to_return	(tree);
>>> +//extern tree apply_postcondition_to_return	(tree);
>>
>> This seems like a temporary thing that didn't get removed?
>>
>>>   extern void start_function_contracts		(tree);
>>> +extern void maybe_apply_function_contracts	(tree);
>>>   extern void finish_function_contracts		(tree);
>>>   extern void set_contract_functions		(tree, tree, tree);
>>>   extern tree build_contract_check		(tree);
>>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
>>> index 97bc211ff67..f350fc33e9b 100644
>>> --- a/gcc/cp/coroutines.cc
>>> +++ b/gcc/cp/coroutines.cc
>>> @@ -4014,6 +4014,8 @@ coro_build_actor_or_destroy_function (tree orig, tree fn_type,
>>>     DECL_USER_ALIGN (fn) = DECL_USER_ALIGN (orig);
>>>     /* Apply attributes from the original fn.  */
>>>     DECL_ATTRIBUTES (fn) = copy_list (DECL_ATTRIBUTES (orig));
>>> +  /* but we do not want ones for contracts.  */
>>> +  remove_contract_attributes (fn);
>>>       /* A void return.  */
>>>     tree resdecl = build_decl (loc, RESULT_DECL, 0, void_type_node);
>>> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
>>> index 416c60b7311..505ee5d4dc9 100644
>>> --- a/gcc/cp/cp-tree.h
>>> +++ b/gcc/cp/cp-tree.h
>>> @@ -451,6 +451,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
>>>         ATOMIC_CONSTR_MAP_INSTANTIATED_P (in ATOMIC_CONSTR)
>>>         contract_semantic (in ASSERTION_, PRECONDITION_, POSTCONDITION_STMT)
>>>         RETURN_EXPR_LOCAL_ADDR_P (in RETURN_EXPR)
>>> +      CONTRACT_EH_ELSE_P (in EH_ELSE_EXPR)
>>>      1: IDENTIFIER_KIND_BIT_1 (in IDENTIFIER_NODE)
>>>         TI_PENDING_TEMPLATE_FLAG.
>>>         TEMPLATE_PARMS_FOR_INLINE.
>>> @@ -723,6 +724,8 @@ typedef struct ptrmem_cst * ptrmem_cst_t;
>>>     #define CLEANUP_P(NODE)		TREE_LANG_FLAG_0 (TRY_BLOCK_CHECK (NODE))
>>>   +#define CONTRACT_EH_ELSE_P(NODE) TREE_LANG_FLAG_0 (EH_ELSE_EXPR_CHECK (NODE))
>>
>> This would need a comment if we kept it.
>>
>>> +
>>>   #define BIND_EXPR_TRY_BLOCK(NODE) \
>>>     TREE_LANG_FLAG_0 (BIND_EXPR_CHECK (NODE))
>>>   diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
>>> index 03deb1493a4..f3e733a29ba 100644
>>> --- a/gcc/cp/decl.cc
>>> +++ b/gcc/cp/decl.cc
>>> @@ -18599,16 +18599,19 @@ finish_function (bool inline_p)
>>>     tree fndecl = current_function_decl;
>>>     tree fntype, ctype = NULL_TREE;
>>>     tree resumer = NULL_TREE, destroyer = NULL_TREE;
>>> -  bool coro_p = flag_coroutines
>>> -		&& !processing_template_decl
>>> -		&& DECL_COROUTINE_P (fndecl);
>>> -  bool coro_emit_helpers = false;
>>>       /* When we get some parse errors, we can end up without a
>>>        current_function_decl, so cope.  */
>>> -  if (fndecl == NULL_TREE)
>>> +  if (fndecl == NULL_TREE || fndecl == error_mark_node)
>>>       return error_mark_node;
>>>   +  bool coro_p = flag_coroutines
>>> +		&& !processing_template_decl
>>> +		&& DECL_COROUTINE_P (fndecl);
>>> +  bool coro_emit_helpers = false;
>>> +  bool do_contracts = DECL_HAS_CONTRACTS_P (fndecl)
>>> +		      && !processing_template_decl;
>>> +
>>>     if (!DECL_OMP_DECLARE_REDUCTION_P (fndecl))
>>>       finish_lambda_scope ();
>>>   @@ -18644,6 +18647,10 @@ finish_function (bool inline_p)
>>>   	finish_eh_spec_block (TYPE_RAISES_EXCEPTIONS
>>>   			      (TREE_TYPE (fndecl)),
>>>   			      current_eh_spec_block);
>>> +
>>> +     /* If outlining succeeded, then add contracts handling if needed.  */
>>> +     if (coro_emit_helpers && do_contracts)
>>> +	maybe_apply_function_contracts (fndecl);
>>>       }
>>>     else
>>>     /* For a cloned function, we've already got all the code we need;
>>> @@ -18659,6 +18666,10 @@ finish_function (bool inline_p)
>>>   	finish_eh_spec_block (TYPE_RAISES_EXCEPTIONS
>>>   			      (TREE_TYPE (current_function_decl)),
>>>   			      current_eh_spec_block);
>>> +
>>> +     if (do_contracts)
>>> +	maybe_apply_function_contracts (current_function_decl);
>>> +
>>>       }
>>>       /* If we're saving up tree structure, tie off the function now.  */
>>> @@ -18911,7 +18922,7 @@ finish_function (bool inline_p)
>>>     --function_depth;
>>>       /* Clean up.  */
>>> -  current_function_decl = NULL_TREE;
>>> +  gcc_checking_assert (current_function_decl == NULL_TREE);
>>>       invoke_plugin_callbacks (PLUGIN_FINISH_PARSE_FUNCTION, fndecl);
>>>   diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
>>> index 5970ac3d398..4434ba6e2f4 100644
>>> --- a/gcc/cp/typeck.cc
>>> +++ b/gcc/cp/typeck.cc
>>> @@ -11416,13 +11416,7 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
>>>       /* Actually copy the value returned into the appropriate location.  */
>>>     if (retval && retval != result)
>>> -    {
>>> -      /* If there's a postcondition for a scalar return value, wrap
>>> -	 retval in a call to the postcondition function.  */
>>> -      if (tree post = apply_postcondition_to_return (retval))
>>> -	retval = post;
>>> -      retval = cp_build_init_expr (result, retval);
>>> -    }
>>> +    retval = cp_build_init_expr (result, retval);
>>>       if (current_function_return_value == bare_retval)
>>>       INIT_EXPR_NRV_P (retval) = true;
>>> @@ -11430,11 +11424,6 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
>>>     if (tree set = maybe_set_retval_sentinel ())
>>>       retval = build2 (COMPOUND_EXPR, void_type_node, retval, set);
>>>   -  /* If there's a postcondition for an aggregate return value, call the
>>> -     postcondition function after the return object is initialized.  */
>>> -  if (tree post = apply_postcondition_to_return (result))
>>> -    retval = build2 (COMPOUND_EXPR, void_type_node, retval, post);
>>> -
>>>     return retval;
>>>   }
>>>   diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
>>> index 622c51d5c3f..43bd4c56d7f 100644
>>> --- a/gcc/gimplify.cc
>>> +++ b/gcc/gimplify.cc
>>> @@ -227,6 +227,7 @@ struct gimplify_ctx
>>>     unsigned keep_stack : 1;
>>>     unsigned save_stack : 1;
>>>     unsigned in_switch_expr : 1;
>>> +  unsigned in_handler_expr : 1;
>>>   };
>>>     enum gimplify_defaultmap_kind
>>> @@ -18401,10 +18402,12 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
>>>   	    input_location = UNKNOWN_LOCATION;
>>>   	    eval = cleanup = NULL;
>>>   	    gimplify_and_add (TREE_OPERAND (*expr_p, 0), &eval);
>>> +	    bool save_in_handler_expr = gimplify_ctxp->in_handler_expr;
>>>   	    if (TREE_CODE (*expr_p) == TRY_FINALLY_EXPR
>>>   		&& TREE_CODE (TREE_OPERAND (*expr_p, 1)) == EH_ELSE_EXPR)
>>>   	      {
>>>   		gimple_seq n = NULL, e = NULL;
>>> +		gimplify_ctxp->in_handler_expr = true;
>>>   		gimplify_and_add (TREE_OPERAND (TREE_OPERAND (*expr_p, 1),
>>>   						0), &n);
>>>   		gimplify_and_add (TREE_OPERAND (TREE_OPERAND (*expr_p, 1),
>>> @@ -18416,7 +18419,11 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
>>>   		  }
>>>   	      }
>>>   	    else
>>> -	      gimplify_and_add (TREE_OPERAND (*expr_p, 1), &cleanup);
>>> +	      {
>>> +		gimplify_ctxp->in_handler_expr = true;
>>> +		gimplify_and_add (TREE_OPERAND (*expr_p, 1), &cleanup);
>>> +	      }
>>> +	    gimplify_ctxp->in_handler_expr = save_in_handler_expr;
>>>   	    /* Don't create bogus GIMPLE_TRY with empty cleanup.  */
>>>   	    if (gimple_seq_empty_p (cleanup))
>>>   	      {
>>> @@ -18516,6 +18523,10 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
>>>   	  /* When within an OMP context, notice uses of variables.  */
>>>   	  if (gimplify_omp_ctxp)
>>>   	    omp_notice_variable (gimplify_omp_ctxp, *expr_p, true);
>>> +	  /* Handlers can refer to the function result; if that has been
>>> +	     moved, we need to track it.  */
>>> +	  if (gimplify_ctxp->in_handler_expr && gimplify_ctxp->return_temp)
>>> +	    *expr_p = gimplify_ctxp->return_temp;
>>>   	  ret = GS_ALL_DONE;
>>>   	  break;
>>>   diff --git a/gcc/testsuite/g++.dg/contracts/pr115434.C b/gcc/testsuite/g++.dg/contracts/pr115434.C
>>> new file mode 100644
>>> index 00000000000..e9c847f8969
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/contracts/pr115434.C
>>> @@ -0,0 +1,16 @@
>>> +// We were failing to apply post conditions to void functions.
>>> +
>>> +// { dg-do run }
>>> +// { dg-options "-std=c++20 -fcontracts -fcontract-continuation-mode=on" }
>>> +
>>> +
>>> +void foo (const int b)
>>> +[[ post: b == 9 ]]  // contract not checked
>>> +{}
>>> +
>>> +int main()
>>> +{
>>> +   foo(3);
>>> +}
>>> +
>>> +// { dg-output "contract violation in function foo at .*.C:8: b == 9.*(\n|\r\n|\r)" }
>>> diff --git a/gcc/testsuite/g++.dg/coroutines/pr110871.C b/gcc/testsuite/g++.dg/coroutines/pr110871.C
>>> new file mode 100644
>>> index 00000000000..8a667c856ae
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/coroutines/pr110871.C
>>> @@ -0,0 +1,62 @@
>>> +// { dg-additional-options "-fcontracts -fcontract-continuation-mode=on" }
>>> +// { dg-do run }
>>> +#include <iostream>
>>> +#include <coroutine>
>>> +
>>> +// In order to test the contract violation diagnostic, we have to set
>>> +// -fcontract-continuation-mode=on; this means that the code will emit
>>> +// the message below - but before that the contract should have been checked.
>>> +void process(int from, int to)
>>> +{
>>> +  if (from > to)
>>> +    std::cout << "would have been a disaster!" << std::endl;
>>> +}
>>> +
>>> +template <typename T>
>>> +struct generator
>>> +{
>>> +    struct promise_type
>>> +    {
>>> +        template <typename... Args>
>>> +        promise_type(Args&&... args) {
>>> +            std::cout << "promise init" << std::endl;
>>> +            process(args...);
>>> +        }
>>> +
>>> +        std::suspend_always yield_value(T) { return {}; }
>>> +
>>> +        std::suspend_always initial_suspend() const noexcept { return {}; }
>>> +        std::suspend_never final_suspend() const noexcept { return {}; }
>>> +        void unhandled_exception() noexcept {}
>>> +
>>> +        generator<T> get_return_object() noexcept { return {}; }
>>> +    };
>>> +};
>>> +
>>> +namespace std {
>>> +template <typename T, typename... Args>
>>> +struct coroutine_traits<generator<T>, Args...>
>>> +{
>>> +    using promise_type = typename generator<T>::promise_type;
>>> +};
>>> +
>>> +};
>>> +
>>> +generator<int> seq(int from, int to) [[pre: from <= to]]
>>> +
>>> +{
>>> +    std::cout << "coro initial" << std::endl;
>>> +    for (int i = from; i <= to; ++i) {
>>> +        co_yield i;
>>> +        std::cout << "coro resumed" << std::endl;
>>> +    }
>>> +}
>>> +
>>> +int main() {
>>> +    std::cout << "main initial" << std::endl;
>>> +    generator<int> s = seq(10, 5);
>>> +    (void)s;
>>> +    std::cout << "main continues" << std::endl;
>>> +}
>>> +
>>> +// { dg-output "contract violation in function seq at .*.C:45: from \<= to.*(\n|\r\n|\r)" }
>>> diff --git a/gcc/testsuite/g++.dg/coroutines/pr110872.C b/gcc/testsuite/g++.dg/coroutines/pr110872.C
>>> new file mode 100644
>>> index 00000000000..a809986f296
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/coroutines/pr110872.C
>>> @@ -0,0 +1,49 @@
>>> +// { dg-additional-options "-fcontracts -fcontract-continuation-mode=on" }
>>> +// { dg-do run }
>>> +
>>> +#include <iostream>
>>> +#include <coroutine>
>>> +
>>> +
>>> +template <typename T>
>>> +struct generator
>>> +{
>>> +    struct promise_type
>>> +    {
>>> +        std::suspend_always yield_value(T) { return {}; }
>>> +
>>> +        std::suspend_always initial_suspend() const noexcept { return {}; }
>>> +        std::suspend_never final_suspend() const noexcept { return {}; }
>>> +        void unhandled_exception() noexcept {}
>>> +
>>> +        generator<T> get_return_object() noexcept { return {}; }
>>> +    };
>>> +
>>> +    bool is_valid() { return false; }
>>> +};
>>> +
>>> +namespace std {
>>> +template <typename T, typename... Args>
>>> +struct coroutine_traits<generator<T>, Args...>
>>> +{
>>> +    using promise_type = typename generator<T>::promise_type;
>>> +};
>>> +
>>> +};
>>> +
>>> +generator<int> val(int v)
>>> +[[post g: g.is_valid()]]
>>> +{
>>> +    std::cout << "coro initial" << std::endl;
>>> +    co_yield v;
>>> +    std::cout << "coro resumed" << std::endl;
>>> +}
>>> +
>>> +int main() {
>>> +    std::cout << "main initial" << std::endl;
>>> +    generator<int> s = val(1);
>>> +    (void)s;
>>> +    std::cout << "main continues" << std::endl;
>>> +}
>>> +
>>> +// { dg-output "contract violation in function val at .*.C:35: g.is_valid().*(\n|\r\n|\r)" }
>

Hi Folks

> On 8 Jul 2024, at 20:57, Jason Merrill <jason@redhat.com> wrote:
> 
> On 7/8/24 3:37 PM, Iain Sandoe wrote:

>>> On 8 Jul 2024, at 20:19, Jason Merrill <jason@redhat.com> wrote:
>>> 
>>> On 6/17/24 8:15 AM, Iain Sandoe wrote:

>>>>      potentially_transformed_function_body ();
>>>>    }
>>>>   finally
>>>>    {
>>>>      handle_post_condition_checking ();
>>>>    }
>>>>   else [only if the function is not marked noexcept(true) ]
>>>>    {
>>>>      __rethrow ();
>>> 
>>> This sounds undesirable, EH normally continues unwinding after running a cleanup.  Why would the exception be considered caught in a way that needs a rethrow?
>> We need to implement that the no-EH cleanup [i.e. the postcondition] does _not_ run when an exception is thrown.  Right now there’s no HANDLER type that does that - we have EH_ONLY but no NON_EH one (otherwise I'd have built a C++ try).
>> however, I could not figure out how to make this arm of the EH_ELSE_EXPR work as an empty construct (the gimplifier does not seem to expect this).
> 
> You can't just put void_node in the else arm?

I had tried a number of permutations (void_node, empty_stmt, empty statement list etc).  Unfortunately, at present that causes the EH_ELSE expression to be silently dropped in gimplication.  The fact that you think it should work (as did i) makes me think either we have a gimplifier bug or a misunderstanding:

@Alex;

we have (gcc/gimplify.cc:18423):

   if (!gimple_seq_empty_p (n) && !gimple_seq_empty_p (e))
   {
    geh_else *stmt = gimple_build_eh_else (n, e);
    gimple_seq_add_stmt (&cleanup, stmt);
  }

Which essentially says “if either of the sub-expressions to this are empty, then do not build it”
Was there a reason for this, or is it a typo?

If I replace ‘&&' by ‘||' (i.e. “if either of the sub-expressions is present, then build it” then things behave as I expect.

IMO it the current action _is_ intended
 (a) it should at least emit a checking diagnostic
 (b) we need to figure out how to extend it so that we can implement what’s needed (non-EH only cleanups)

So I patched the gimplifier as above and initial testing says that this change does not cause any C++ regressions - but I need to do Ada, Objective-C etc too.

thoughts?
thanks
Iain

On 7/9/24 11:52 AM, Iain Sandoe wrote:
> Hi Folks
> 
>> On 8 Jul 2024, at 20:57, Jason Merrill <jason@redhat.com> wrote:
>>
>> On 7/8/24 3:37 PM, Iain Sandoe wrote:
> 
>>>> On 8 Jul 2024, at 20:19, Jason Merrill <jason@redhat.com> wrote:
>>>>
>>>> On 6/17/24 8:15 AM, Iain Sandoe wrote:
> 
>>>>>       potentially_transformed_function_body ();
>>>>>     }
>>>>>    finally
>>>>>     {
>>>>>       handle_post_condition_checking ();
>>>>>     }
>>>>>    else [only if the function is not marked noexcept(true) ]
>>>>>     {
>>>>>       __rethrow ();
>>>>
>>>> This sounds undesirable, EH normally continues unwinding after running a cleanup.  Why would the exception be considered caught in a way that needs a rethrow?
>>> We need to implement that the no-EH cleanup [i.e. the postcondition] does _not_ run when an exception is thrown.  Right now there’s no HANDLER type that does that - we have EH_ONLY but no NON_EH one (otherwise I'd have built a C++ try).
>>> however, I could not figure out how to make this arm of the EH_ELSE_EXPR work as an empty construct (the gimplifier does not seem to expect this).
>>
>> You can't just put void_node in the else arm?
> 
> I had tried a number of permutations (void_node, empty_stmt, empty statement list etc).  Unfortunately, at present that causes the EH_ELSE expression to be silently dropped in gimplication.  The fact that you think it should work (as did i) makes me think either we have a gimplifier bug or a misunderstanding:
> 
> @Alex;
> 
> we have (gcc/gimplify.cc:18423):
> 
>     if (!gimple_seq_empty_p (n) && !gimple_seq_empty_p (e))
>     {
>      geh_else *stmt = gimple_build_eh_else (n, e);
>      gimple_seq_add_stmt (&cleanup, stmt);
>    }
> 
> Which essentially says “if either of the sub-expressions to this are empty, then do not build it”
> Was there a reason for this, or is it a typo?
> 
> If I replace ‘&&' by ‘||' (i.e. “if either of the sub-expressions is present, then build it” then things behave as I expect.
> 
> IMO it the current action _is_ intended
>   (a) it should at least emit a checking diagnostic
>   (b) we need to figure out how to extend it so that we can implement what’s needed (non-EH only cleanups)
> 
> So I patched the gimplifier as above and initial testing says that this change does not cause any C++ regressions - but I need to do Ada, Objective-C etc too.
> 
> thoughts?

That sounds good to me, it does seem like a typo.

Jason

On Jul  9, 2024, Iain Sandoe <iain@sandoe.co.uk> wrote:

>    if (!gimple_seq_empty_p (n) && !gimple_seq_empty_p (e))
>    {
>     geh_else *stmt = gimple_build_eh_else (n, e);
>     gimple_seq_add_stmt (&cleanup, stmt);
>   }

> Which essentially says “if either of the sub-expressions to this are empty, then do not build it”
> Was there a reason for this, or is it a typo?

Most certainly a thinko :-(

Thanks for identifying it and for proposing a fix for it!

HI Jason,

> On 9 Jul 2024, at 22:55, Jason Merrill <jason@redhat.com> wrote:
> 
> On 7/9/24 11:52 AM, Iain Sandoe wrote:
>> Hi Folks
>>> On 8 Jul 2024, at 20:57, Jason Merrill <jason@redhat.com> wrote:
>>> 
>>> On 7/8/24 3:37 PM, Iain Sandoe wrote:
>>>>> On 8 Jul 2024, at 20:19, Jason Merrill <jason@redhat.com> wrote:
>>>>> 
>>>>> On 6/17/24 8:15 AM, Iain Sandoe wrote:
>>>>>>      potentially_transformed_function_body ();
>>>>>>    }
>>>>>>   finally
>>>>>>    {
>>>>>>      handle_post_condition_checking ();
>>>>>>    }
>>>>>>   else [only if the function is not marked noexcept(true) ]
>>>>>>    {
>>>>>>      __rethrow ();
>>>>> 
>>>>> This sounds undesirable, EH normally continues unwinding after running a cleanup.  Why would the exception be considered caught in a way that needs a rethrow?
>>>> We need to implement that the no-EH cleanup [i.e. the postcondition] does _not_ run when an exception is thrown.  Right now there’s no HANDLER type that does that - we have EH_ONLY but no NON_EH one (otherwise I'd have built a C++ try).
>>>> however, I could not figure out how to make this arm of the EH_ELSE_EXPR work as an empty construct (the gimplifier does not seem to expect this).
>>> 
>>> You can't just put void_node in the else arm?
>> I had tried a number of permutations (void_node, empty_stmt, empty statement list etc).  Unfortunately, at present that causes the EH_ELSE expression to be silently dropped in gimplication.  The fact that you think it should work (as did i) makes me think either we have a gimplifier bug or a misunderstanding:
>> @Alex;
>> we have (gcc/gimplify.cc:18423):
>>    if (!gimple_seq_empty_p (n) && !gimple_seq_empty_p (e))
>>    {
>>     geh_else *stmt = gimple_build_eh_else (n, e);
>>     gimple_seq_add_stmt (&cleanup, stmt);
>>   }
>> Which essentially says “if either of the sub-expressions to this are empty, then do not build it”
>> Was there a reason for this, or is it a typo?
>> If I replace ‘&&' by ‘||' (i.e. “if either of the sub-expressions is present, then build it” then things behave as I expect.
>> IMO it the current action _is_ intended
>>  (a) it should at least emit a checking diagnostic
>>  (b) we need to figure out how to extend it so that we can implement what’s needed (non-EH only cleanups)
>> So I patched the gimplifier as above and initial testing says that this change does not cause any C++ regressions - but I need to do Ada, Objective-C etc too.
>> thoughts?
> 
> That sounds good to me, it does seem like a typo.

Here is the revised patch - which implements the gimplifier change and thus simplifes the rest.

OK for trunk?
thanks
Iain

P.S. I did not figure out how to find the right message-id to reply to from git send-email, so attaching.



> 
> Jason
>

On 7/12/24 1:03 PM, Iain Sandoe wrote:
> HI Jason,
> 
>> On 9 Jul 2024, at 22:55, Jason Merrill <jason@redhat.com> wrote:
>>
>> On 7/9/24 11:52 AM, Iain Sandoe wrote:
>>> Hi Folks
>>>> On 8 Jul 2024, at 20:57, Jason Merrill <jason@redhat.com> wrote:
>>>>
>>>> On 7/8/24 3:37 PM, Iain Sandoe wrote:
>>>>>> On 8 Jul 2024, at 20:19, Jason Merrill <jason@redhat.com> wrote:
>>>>>>
>>>>>> On 6/17/24 8:15 AM, Iain Sandoe wrote:
>>>>>>>       potentially_transformed_function_body ();
>>>>>>>     }
>>>>>>>    finally
>>>>>>>     {
>>>>>>>       handle_post_condition_checking ();
>>>>>>>     }
>>>>>>>    else [only if the function is not marked noexcept(true) ]
>>>>>>>     {
>>>>>>>       __rethrow ();
>>>>>>
>>>>>> This sounds undesirable, EH normally continues unwinding after running a cleanup.  Why would the exception be considered caught in a way that needs a rethrow?
>>>>> We need to implement that the no-EH cleanup [i.e. the postcondition] does _not_ run when an exception is thrown.  Right now there’s no HANDLER type that does that - we have EH_ONLY but no NON_EH one (otherwise I'd have built a C++ try).
>>>>> however, I could not figure out how to make this arm of the EH_ELSE_EXPR work as an empty construct (the gimplifier does not seem to expect this).
>>>>
>>>> You can't just put void_node in the else arm?
>>> I had tried a number of permutations (void_node, empty_stmt, empty statement list etc).  Unfortunately, at present that causes the EH_ELSE expression to be silently dropped in gimplication.  The fact that you think it should work (as did i) makes me think either we have a gimplifier bug or a misunderstanding:
>>> @Alex;
>>> we have (gcc/gimplify.cc:18423):
>>>     if (!gimple_seq_empty_p (n) && !gimple_seq_empty_p (e))
>>>     {
>>>      geh_else *stmt = gimple_build_eh_else (n, e);
>>>      gimple_seq_add_stmt (&cleanup, stmt);
>>>    }
>>> Which essentially says “if either of the sub-expressions to this are empty, then do not build it”
>>> Was there a reason for this, or is it a typo?
>>> If I replace ‘&&' by ‘||' (i.e. “if either of the sub-expressions is present, then build it” then things behave as I expect.
>>> IMO it the current action _is_ intended
>>>   (a) it should at least emit a checking diagnostic
>>>   (b) we need to figure out how to extend it so that we can implement what’s needed (non-EH only cleanups)
>>> So I patched the gimplifier as above and initial testing says that this change does not cause any C++ regressions - but I need to do Ada, Objective-C etc too.
>>> thoughts?
>>
>> That sounds good to me, it does seem like a typo.
> 
> Here is the revised patch - which implements the gimplifier change and thus simplifes the rest.
> 
> OK for trunk?
> thanks
> Iain
> 
> P.S. I did not figure out how to find the right message-id to reply to from git send-email, so attaching.

I tend to need to "view source" for that, which varies with mail readers.

> -   The original decl is left alone and instead calls are generated to pre/post
> -   functions within the body:
>  
> -     void fun.pre(int v)
> -     {
> -       [[ assert: v > 0 ]];
> -     }
> -     int fun.post(int v, int __r)
>       {
> -       [[ assert: __r < 0 ]];
> -       return __r;
>       }
> -     int fun(int v)
>       {
> -       fun.pre(v);
> -       return fun.post(v, -v);
>       }
> -
> -   If fun returns in memory, the return value is not passed through the post
> -   function; instead, the return object is initialized directly and then passed
> -   to the post function by invisible reference.
> -
> -   This sides steps a number of issues with having to rewrite the bodies or
> -   rewrite the parsed conditions as the parameters to the original function
> -   changes (as happens during redeclaration). The ultimate goal is to get
> -   something that optimizes well along the lines of
> -
> -     int fun(int v)
>       {
> -       [[ assert: v > 0 ]];
> -       auto &&__r = -v;
> -       goto out;
> -     out:
> -       [[ assert: __r < 0 ]];
> -       return __r;
>       }
>  
> -   With the idea being that multiple return statements could collapse the
> -   function epilogue after inlining the pre/post functions. clang is able
> -   to collapse common function epilogues, while gcc needs -O3 -Os combined.
> -
> -   Directly laying the pre contracts down in the function body doesn't have
> -   many issues. The post contracts may need to be repeated multiple times, once
> -   for each return, or a goto epilogue would need to be generated.
> -   For this initial implementation, generating function calls and letting
> -   later optimizations decide whether to inline and duplicate the actual
> -   checks or whether to collapse the shared epilogue was chosen.
> -
> -   For cdtors a post contract is implemented using a CLEANUP_STMT.
> -
> -   FIXME the compiler already shores cleanup code on multiple exit paths, so
> -   this outlining seems unnecessary if we represent the postcondition as a
> -   cleanup for all functions.
> -
> -   More helpful for optimization might be to make the contracts a wrapper
> -   function (for non-variadic functions), that could be inlined into a
> -   caller while preserving the call to the actual function?  Either that or
> -   mirror a never-continue post contract with an assume in the caller.  */

Much the old documentation about outlining conditions still seems 
relevant.  In particular I'd keep something like

"FIXME outlining contract checks into separate functions was motivated 
partly by wanting to call the postcondition function at each return 
statement, which we no longer do; at this point outlining doesn't seem 
to have any advantage over emitting the contracts directly in the 
function body.

More helpful for optimization might be to make the contracts a wrapper 
function that could be inlined into the caller, the callee, or both."

> +  gcc_checking_assert (DECL_PRE_FN (fndecl)
> +      && DECL_PRE_FN (fndecl) != error_mark_node);
...
> +  gcc_checking_assert (DECL_POST_FN (fndecl)
> +      && DECL_POST_FN (fndecl) != error_mark_node);

The && should line up past the ( on the previous line.

> -/* Emit statements for postcondition attributes.  */
> +/* Emit statements for precondition attributes.  */
>  
>  static void
> -emit_postconditions_cleanup (tree contracts)
> +emit_postconditions (tree attr)

You changed the comment from post to pre, but the function is still post.

> +  bool coro_p = flag_coroutines
> +		&& !processing_template_decl
> +		&& DECL_COROUTINE_P (fndecl);
> +  bool coro_emit_helpers = false;
> +  bool do_contracts = DECL_HAS_CONTRACTS_P (fndecl)
> +		      && !processing_template_decl;

Let's add parens to help preserve the indentation.

> -  current_function_decl = NULL_TREE;
> +  gcc_checking_assert (current_function_decl == NULL_TREE);

I think we can just drop this line, it's redundant with the clearing a 
few lines above.

OK with these tweaks.

Jason

Hi Jason,

> On 15 Jul 2024, at 23:29, Jason Merrill <jason@redhat.com> wrote:
> 
> On 7/12/24 1:03 PM, Iain Sandoe wrote:
>> 

>> -   More helpful for optimization might be to make the contracts a wrapper
>> -   function (for non-variadic functions), that could be inlined into a
>> -   caller while preserving the call to the actual function?  Either that or
>> -   mirror a never-continue post contract with an assume in the caller.  */
> 
> Much the old documentation about outlining conditions still seems relevant.  In particular I'd keep something like
> 
> "FIXME outlining contract checks into separate functions was motivated partly by wanting to call the postcondition function at each return statement, which we no longer do; at this point outlining doesn't seem to have any advantage over emitting the contracts directly in the function body.
> 
> More helpful for optimization might be to make the contracts a wrapper function that could be inlined into the caller, the callee, or both."

Nina is about to start looking into the implementation of caller-side contracts (at least in so far as it relates to the current virtual functions proposal) .. so I will work with her to implement this.

My expectation is that the callee side can be updated to do the work inline, we can then drop the special handling for cdtors.

Caller-side seems likey to need a wrapper thunk that then calls the original fn.

(but we did not get into the details yet, those are just initial thoughts).
> 
> 
> OK with these tweaks.

thanks for the review,
what was applied is attached,
thanks
Iain

On 7/16/24 12:05 PM, Iain Sandoe wrote:
> Hi Jason,
> 
>> On 15 Jul 2024, at 23:29, Jason Merrill <jason@redhat.com> wrote:
>> On 7/12/24 1:03 PM, Iain Sandoe wrote:
> 
>>> -   More helpful for optimization might be to make the contracts a wrapper
>>> -   function (for non-variadic functions), that could be inlined into a
>>> -   caller while preserving the call to the actual function?  Either that or
>>> -   mirror a never-continue post contract with an assume in the caller.  */
>>
>> Much the old documentation about outlining conditions still seems relevant.  In particular I'd keep something like
>>
>> "FIXME outlining contract checks into separate functions was motivated partly by wanting to call the postcondition function at each return statement, which we no longer do; at this point outlining doesn't seem to have any advantage over emitting the contracts directly in the function body.
>>
>> More helpful for optimization might be to make the contracts a wrapper function that could be inlined into the caller, the callee, or both."
> 
> Nina is about to start looking into the implementation of caller-side contracts (at least in so far as it relates to the current virtual functions proposal) .. so I will work with her to implement this.
> 
> My expectation is that the callee side can be updated to do the work inline, we can then drop the special handling for cdtors.
> 
> Caller-side seems likely to need a wrapper thunk that then calls the original fn.
> 
> (but we did not get into the details yet, those are just initial thoughts).

I had been thinking that for non-virtual functions it could be two of 
the same wrapper, but as discussed on the list that would involve ABI 
complexities, so indeed probably simpler for callee to handle them directly.

Jason