public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jason Merrill <jason@redhat.com>
To: iain@sandoe.co.uk, gcc-patches@gcc.gnu.org
Cc: arsen@aarsen.me
Subject: Re: [PATCH] c++, coroutines: Fix block nests when the function has no top-level bind.
Date: Sat, 8 Apr 2023 10:10:25 -0700	[thread overview]
Message-ID: <3377ff7c-3778-c29c-c5a3-19903507f975@redhat.com> (raw)
In-Reply-To: <20230406062737.79723-1-iain@sandoe.co.uk>

On 4/6/23 02:27, Iain Sandoe wrote:
> I spotted this problem while trying to collate my WIP patches (sorry
> for not spotting it when the original patch was discussed).
> 
> Although we do not yet have any regression filed against this (AFAIK),
> it's only a matter of time - so I'd suggest that we should apply to
> 13.
> 
>   Tested on x86_64-darwin21 and x86_64_linux-gnu.
>   OK for trunk? (when?)
>   Iain
>   
>   --- 8< ---
> 
> When the function contains no local vars and also no nested scopes, there
> is no top-level bind expression.  Because the rewritten coroutine body will
> require both local vars and contain nested scopes, we add a bind expression
> to such functions.  When this was done the necessary scope blocks were
> omitted which leads to disconnected function content.
> 
> Fixed by adding a new block to the added bind expression.
> 
> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
> 
> gcc/cp/ChangeLog:
> 
> 	* coroutines.cc (coro_rewrite_function_body): Ensure that added
> 	bind expressions have scope blocks.
> ---
>   gcc/cp/coroutines.cc | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index b307c8ca8b6..59a240ebd40 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -4113,6 +4113,10 @@ coro_rewrite_function_body (location_t fn_start, tree fnbody, tree orig,
>         tree bind_wrap = build3_loc (fn_start, BIND_EXPR, void_type_node,
>   				   NULL, NULL, NULL);
>         BIND_EXPR_BODY (bind_wrap) = fnbody;
> +      /* Ensure we have a block to connect up the scopes.  */
> +      tree new_blk = make_node (BLOCK);
> +      BIND_EXPR_BLOCK (bind_wrap) = new_blk;
> +      BLOCK_SUBBLOCKS (top_block) = new_blk;
>         fnbody = bind_wrap;
>       }

I wonder if you want to use c_build_bind_expr for various BIND_EXPR 
creation?  You'll still need to explicitly build a BLOCK in this 
particular case, as in poplevel with nonzero functionbody, but that 
might reduce the boilerplate in general.

The patch is OK.

Jason


      reply	other threads:[~2023-04-08 17:10 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-06  6:27 Iain Sandoe
2023-04-08 17:10 ` Jason Merrill [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3377ff7c-3778-c29c-c5a3-19903507f975@redhat.com \
    --to=jason@redhat.com \
    --cc=arsen@aarsen.me \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=iain@sandoe.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).