public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Iain Sandoe <iain@sandoe.co.uk>
To: Adrian Perl <adrian.perl@web.de>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>, Jason Merrill <jason@redhat.com>
Subject: Re: [PATCH] coroutines: Fix promotion of class members in co_await statements [PR99576]
Date: Wed, 30 Nov 2022 08:51:03 +0000	[thread overview]
Message-ID: <F9F61BF9-5BF4-4F6A-9100-2633F0AE4265@sandoe.co.uk> (raw)
In-Reply-To: <B675756E-D394-4B24-B921-19F9F3AACDFB@sandoe.co.uk>

Hi Adrian,

> On 28 Nov 2022, at 20:44, Iain Sandoe <iain@sandoe.co.uk> wrote:

>> Bootstrapping and running the testsuite on x86_64 was successfull. No regression occured.
> 
> This looks resonable to me, as said in the PR.  I’d like to test a little wider with some larger
> codebases, if you could bear with me for a few days.

So wider testing (in this case folly) reveals that, although the analysis seems reasonable, this is not quite the right patch to fix the issue.  It can be that CONSTRUCTORS contain nested await expressions, so we cannot simply punt on seeing one.

My hunch is that the real solution lies in (correctly) deciding whether to promote the temporary or not.  Jason recently made a change that identifies whether a target expression is expected to be elided (i.e. it is a direct intializer for another object) - I think this might help in this case.  My concern is whether I should read “expected to be elided” to be a guarantee (“expected” to me could also be read “but it might not be”).

As is, the patch is not OK since it regresses cases with nested await expressions in CONSTRUCTORS.
sorry for not spotting that sooner,

Iain


  reply	other threads:[~2022-11-30  8:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-28 11:55 Adrian Perl
2022-11-28 20:44 ` Iain Sandoe
2022-11-30  8:51   ` Iain Sandoe [this message]
2022-11-30 18:58     ` Jason Merrill

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=F9F61BF9-5BF4-4F6A-9100-2633F0AE4265@sandoe.co.uk \
    --to=iain@sandoe.co.uk \
    --cc=adrian.perl@web.de \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    /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).