diff mbox series

[coroutines] Do not strip cleanup_point when promote temporaries out of current stmt

Message ID b68838b2-bed8-f998-2bb3-bf2771ef0be3@linux.alibaba.com
State New
Headers show
Series [coroutines] Do not strip cleanup_point when promote temporaries out of current stmt | expand

Commit Message

JunMa Feb. 11, 2020, 2:14 a.m. UTC
Hi
In maybe_promote_captured_temps, the cleanup_point_stmt has been
stripped when handle temporaries captured by reference. However, maybe
there are non-reference temporaries in current stmt which cause ice in
gimpilify pass.

This patch fix this. The testcase comes from cppcoro and is reduced by
creduce.

Bootstrap and test on X86_64, is it OK?

Regards
JunMa

gcc/cp
2020-02-11  Jun Ma <JunMa@linux.alibaba.com>

         * coroutines.cc (maybe_promote_captured_temps): Do not strip
         cleanup_point_stmt.

gcc/testsuite
2020-02-11  Jun Ma <JunMa@linux.alibaba.com>

         * g++.dg/coroutines/torture/lambda-09-capture-object.C: New test.
---
 gcc/cp/coroutines.cc                          |   8 +-
 .../torture/lambda-09-capture-object.C        | 132 ++++++++++++++++++
 2 files changed, 135 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/lambda-09-capture-object.C

Comments

JunMa Feb. 27, 2020, 2:18 a.m. UTC | #1
在 2020/2/11 上午10:14, JunMa 写道:
Kindly ping

Regards
JunMa
> Hi
> In maybe_promote_captured_temps, the cleanup_point_stmt has been
> stripped when handle temporaries captured by reference. However, maybe
> there are non-reference temporaries in current stmt which cause ice in
> gimpilify pass.
>
> This patch fix this. The testcase comes from cppcoro and is reduced by
> creduce.
>
> Bootstrap and test on X86_64, is it OK?
>
> Regards
> JunMa
>
> gcc/cp
> 2020-02-11  Jun Ma <JunMa@linux.alibaba.com>
>
>         * coroutines.cc (maybe_promote_captured_temps): Do not strip
>         cleanup_point_stmt.
>
> gcc/testsuite
> 2020-02-11  Jun Ma <JunMa@linux.alibaba.com>
>
>         * g++.dg/coroutines/torture/lambda-09-capture-object.C: New test.
JunMa March 5, 2020, 11:55 a.m. UTC | #2
Ping

Regards
JunMa
在 2020/2/27 上午10:18, JunMa 写道:
> 在 2020/2/11 上午10:14, JunMa 写道:
> Kindly ping
>
> Regards
> JunMa
>> Hi
>> In maybe_promote_captured_temps, the cleanup_point_stmt has been
>> stripped when handle temporaries captured by reference. However, maybe
>> there are non-reference temporaries in current stmt which cause ice in
>> gimpilify pass.
>>
>> This patch fix this. The testcase comes from cppcoro and is reduced by
>> creduce.
>>
>> Bootstrap and test on X86_64, is it OK?
>>
>> Regards
>> JunMa
>>
>> gcc/cp
>> 2020-02-11  Jun Ma <JunMa@linux.alibaba.com>
>>
>>         * coroutines.cc (maybe_promote_captured_temps): Do not strip
>>         cleanup_point_stmt.
>>
>> gcc/testsuite
>> 2020-02-11  Jun Ma <JunMa@linux.alibaba.com>
>>
>>         * g++.dg/coroutines/torture/lambda-09-capture-object.C: New 
>> test.
>
>
Iain Sandoe March 5, 2020, 2:18 p.m. UTC | #3
Hello JunMa,

JunMa <JunMa@linux.alibaba.com> wrote:

> Ping

Once again, sorry for taking time to review this.

> 在 2020/2/27 上午10:18, JunMa 写道:
>> 在 2020/2/11 上午10:14, JunMa 写道:
>> Kindly ping
>>
>> Regards
>> JunMa
>>> Hi
>>> In maybe_promote_captured_temps, the cleanup_point_stmt has been
>>> stripped when handle temporaries captured by reference. However, maybe
>>> there are non-reference temporaries in current stmt which cause ice in
>>> gimpilify pass.
>>>
>>> This patch fix this. The testcase comes from cppcoro and is reduced by
>>> creduce.

With current trunk + Bin’s two approved patches.

I see no change in the testcase (lambda-09-capture-object.C) before / after  
the patch
  (it fails for me at -O0 only - in both cases).

please could you check?
thanks
Iain
JunMa March 6, 2020, 5:54 a.m. UTC | #4
在 2020/3/5 下午10:18, Iain Sandoe 写道:
> Hello JunMa,
>
> JunMa <JunMa@linux.alibaba.com> wrote:
>
>> Ping
>
> Once again, sorry for taking time to review this.
>
>> 在 2020/2/27 上午10:18, JunMa 写道:
>>> 在 2020/2/11 上午10:14, JunMa 写道:
>>> Kindly ping
>>>
>>> Regards
>>> JunMa
>>>> Hi
>>>> In maybe_promote_captured_temps, the cleanup_point_stmt has been
>>>> stripped when handle temporaries captured by reference. However, maybe
>>>> there are non-reference temporaries in current stmt which cause ice in
>>>> gimpilify pass.
>>>>
>>>> This patch fix this. The testcase comes from cppcoro and is reduced by
>>>> creduce.
>
> With current trunk + Bin’s two approved patches.
>
> I see no change in the testcase (lambda-09-capture-object.C) before / 
> after the patch
>  (it fails for me at -O0 only - in both cases).
>
> please could you check?
As I said at previous mail, this patch fix the ICE in gimpilify pass.

I test with current trunk + Bin's two patches, the testcase passes with 
the patch and fails
without the patch. It also fix co-await-syntax-11.C which caused by same 
ICE.

could you do double check?

Regards
JunMa
> thanks
> Iain
Bin.Cheng March 10, 2020, 10:04 a.m. UTC | #5
On Thu, Mar 5, 2020 at 10:18 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>
> Hello JunMa,
>
> JunMa <JunMa@linux.alibaba.com> wrote:
>
> > Ping
>
> Once again, sorry for taking time to review this.
>
> > 在 2020/2/27 上午10:18, JunMa 写道:
> >> 在 2020/2/11 上午10:14, JunMa 写道:
> >> Kindly ping
> >>
> >> Regards
> >> JunMa
> >>> Hi
> >>> In maybe_promote_captured_temps, the cleanup_point_stmt has been
> >>> stripped when handle temporaries captured by reference. However, maybe
> >>> there are non-reference temporaries in current stmt which cause ice in
> >>> gimpilify pass.
> >>>
> >>> This patch fix this. The testcase comes from cppcoro and is reduced by
> >>> creduce.
>
> With current trunk + Bin’s two approved patches.
>
> I see no change in the testcase (lambda-09-capture-object.C) before / after
> the patch
>   (it fails for me at -O0 only - in both cases).

Hi Iain,
I tried exactly what you did, however, the result is different.
With current trunk(cb2c60206f4f2218f84ccde21663b00de068d8c7) with my
approved patch, the case(lambda-09-capture-object.C) still causes ICE.
Actually, the same ICE happens in testcase(co-await-syntax-11.C) added
by my patch.
So this one is a prerequisite for my approved patch.

Thanks,
bin
>
> please could you check?
> thanks
> Iain
>
Iain Sandoe March 11, 2020, 9:07 a.m. UTC | #6
Bin.Cheng <amker.cheng@gmail.com> wrote:

> On Thu, Mar 5, 2020 at 10:18 PM Iain Sandoe <iain@sandoe.co.uk> wrote:

>> With current trunk + Bin’s two approved patches.
>> 
>> I see no change in the testcase (lambda-09-capture-object.C) before / after
>> the patch
>>  (it fails for me at -O0 only - in both cases).
> 

> I tried exactly what you did, however, the result is different.

I am a bit concerned that we get different results - yesterday I retested this on:
  Linux : x86_64-linux (cfarm gcc122) 
  Darwin (x86_64-darwin16), 
 with the same results on both platforms.

1) I applied the two testcases (note I have renamed lambda-09-capture-object.C => lambda-11-capture-object.C so that the test numbers are sequential).  However, I have not changed the test in any other way.

Native configuration is x86_64-pc-linux-gnu

		=== g++ tests ===

Schedule of variations:
    unix/-m32
    unix/-m64

Running target unix/-m32
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
FAIL: g++.dg/coroutines/co-await-syntax-10.C (internal compiler error)
FAIL: g++.dg/coroutines/co-await-syntax-10.C (test for excess errors)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)

Running target unix/-m64
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
FAIL: g++.dg/coroutines/co-await-syntax-10.C (internal compiler error)
FAIL: g++.dg/coroutines/co-await-syntax-10.C (test for excess errors)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)

^^ so, this shows that both tests fail (co-await-syntax-10.C is expected)

2) I apply Bin’s patch (Pickup more CO_AWAIT_EXPR expanding cases) (which is approved)

Native configuration is x86_64-pc-linux-gnu

		=== g++ tests ===

Schedule of variations:
    unix/-m32
    unix/-m64

Running target unix/-m32
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)

Running target unix/-m64
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)

^^^ this shows that co-await-syntax-10.C is now OK (which is why I was happy to have this patch applied).

3) I apply JunMa’s patch (Do not strip cleanup_point when promote temporaries out of current stmt)

Native configuration is x86_64-pc-linux-gnu

		=== g++ tests ===

Schedule of variations:
    unix/-m32
    unix/-m64

Running target unix/-m32
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)

Running target unix/-m64
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)

^^^ and there is no change to the result.

====

If it helps, I could push a branch to users/iains/ on the FSF git server with this sequence.

thanks for your patience,
Iain
Li, Pan2 via Gcc-patches March 16, 2020, 9:34 a.m. UTC | #7
On Wed, Mar 11, 2020 at 5:07 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>
> Bin.Cheng <amker.cheng@gmail.com> wrote:
>
> > On Thu, Mar 5, 2020 at 10:18 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>
> >> With current trunk + Bin’s two approved patches.
> >>
> >> I see no change in the testcase (lambda-09-capture-object.C) before / after
> >> the patch
> >>  (it fails for me at -O0 only - in both cases).
> >
>
> > I tried exactly what you did, however, the result is different.
>
> I am a bit concerned that we get different results - yesterday I retested this on:
>   Linux : x86_64-linux (cfarm gcc122)
>   Darwin (x86_64-darwin16),
>  with the same results on both platforms.
>
> 1) I applied the two testcases (note I have renamed lambda-09-capture-object.C => lambda-11-capture-object.C so that the test numbers are sequential).  However, I have not changed the test in any other way.
>
> Native configuration is x86_64-pc-linux-gnu
>
>                 === g++ tests ===
>
> Schedule of variations:
>     unix/-m32
>     unix/-m64
>
> Running target unix/-m32
> Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
> Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
> Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
> Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
> FAIL: g++.dg/coroutines/co-await-syntax-10.C (internal compiler error)
> FAIL: g++.dg/coroutines/co-await-syntax-10.C (test for excess errors)
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
> Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)
>
> Running target unix/-m64
> Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
> Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
> Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
> Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
> FAIL: g++.dg/coroutines/co-await-syntax-10.C (internal compiler error)
> FAIL: g++.dg/coroutines/co-await-syntax-10.C (test for excess errors)
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
> Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)
>
> ^^ so, this shows that both tests fail (co-await-syntax-10.C is expected)
>
> 2) I apply Bin’s patch (Pickup more CO_AWAIT_EXPR expanding cases) (which is approved)
>
> Native configuration is x86_64-pc-linux-gnu
>
>                 === g++ tests ===
>
> Schedule of variations:
>     unix/-m32
>     unix/-m64
>
> Running target unix/-m32
> Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
> Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
> Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
> Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
> Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)
>
> Running target unix/-m64
> Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
> Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
> Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
> Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
> Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)
>
> ^^^ this shows that co-await-syntax-10.C is now OK (which is why I was happy to have this patch applied).
>
> 3) I apply JunMa’s patch (Do not strip cleanup_point when promote temporaries out of current stmt)
>
> Native configuration is x86_64-pc-linux-gnu
>
>                 === g++ tests ===
>
> Schedule of variations:
>     unix/-m32
>     unix/-m64
>
> Running target unix/-m32
> Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
> Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
> Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
> Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
> Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)
>
> Running target unix/-m64
> Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
> Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
> Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
> Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
> Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
> FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)
>
> ^^^ and there is no change to the result.
>
> ====
>
> If it helps, I could push a branch to users/iains/ on the FSF git server with this sequence.
Sorry for being slow replying.  This is weird, were you testing against trunk?

Let's put other patches aside and consider only "Do not strip
cleanup_point when promote temporaries out of current stmt".

Here is what I got:

master: 6d44c881286762628afce5169d921a388ae6a1ff

Applying patch from:
"Do not strip cleanup_point when promote temporaries out of current stmt".

Configuring GCC:
$ ../gcc/configure --prefix=... --disable-bootstrap --disable-multilib
--disable-libasan --disable-libusan --with-isl=...
--enable-languages=c,c++ --disable-libsanitizer

Run make check:
$ make check-gcc RUNTESTFLAGS="coroutines.exp=lambda-09-capture-object.C"

The result:
Running target unix
Running /.../gcc/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
PASS: g++.dg/coroutines/torture/lambda-09-capture-object.C (test for
excess errors)

Note I can't run following check:
$ make check-gcc RUNTESTFLAGS="coro-torture.exp=lambda-09-capture-object.C"
because it errors like:
Running /.../gcc/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
ERROR: tcl error sourcing
/.../gcc/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp.
ERROR: can't read "DEFAULT_CXXFLAGS": no such variable
    while executing
"set DEFAULT_COROFLAGS $DEFAULT_CXXFLAGS"
    (file "/.../gcc/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp"
line 11)
    invoked from within
"source /.../gcc/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp"
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 source
/.../gcc/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp"
    invoked from within
"catch "uplevel #0 source $test_file_name""

So I suspect you were testing with local changes?  If so, please push
the branch.

Thanks,
bin



>
> thanks for your patience,
> Iain
>
>
Li, Pan2 via Gcc-patches March 26, 2020, 6:04 a.m. UTC | #8
On Mon, Mar 16, 2020 at 5:34 PM Bin.Cheng <amker.cheng@gmail.com> wrote:
>
> On Wed, Mar 11, 2020 at 5:07 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
> >
> > Bin.Cheng <amker.cheng@gmail.com> wrote:
> >
> > > On Thu, Mar 5, 2020 at 10:18 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
> >
> > >> With current trunk + Bin’s two approved patches.
> > >>
> > >> I see no change in the testcase (lambda-09-capture-object.C) before / after
> > >> the patch
> > >>  (it fails for me at -O0 only - in both cases).
> > >
> >
> > > I tried exactly what you did, however, the result is different.
> >
> > I am a bit concerned that we get different results - yesterday I retested this on:
> >   Linux : x86_64-linux (cfarm gcc122)
> >   Darwin (x86_64-darwin16),
> >  with the same results on both platforms.
> >
> > 1) I applied the two testcases (note I have renamed lambda-09-capture-object.C => lambda-11-capture-object.C so that the test numbers are sequential).  However, I have not changed the test in any other way.
> >
> > Native configuration is x86_64-pc-linux-gnu
> >
> >                 === g++ tests ===
> >
> > Schedule of variations:
> >     unix/-m32
> >     unix/-m64
> >
> > Running target unix/-m32
> > Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
> > Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
> > Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
> > Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
> > FAIL: g++.dg/coroutines/co-await-syntax-10.C (internal compiler error)
> > FAIL: g++.dg/coroutines/co-await-syntax-10.C (test for excess errors)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
> > Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)
> >
> > Running target unix/-m64
> > Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
> > Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
> > Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
> > Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
> > FAIL: g++.dg/coroutines/co-await-syntax-10.C (internal compiler error)
> > FAIL: g++.dg/coroutines/co-await-syntax-10.C (test for excess errors)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
> > Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)
> >
> > ^^ so, this shows that both tests fail (co-await-syntax-10.C is expected)
> >
> > 2) I apply Bin’s patch (Pickup more CO_AWAIT_EXPR expanding cases) (which is approved)
> >
> > Native configuration is x86_64-pc-linux-gnu
> >
> >                 === g++ tests ===
> >
> > Schedule of variations:
> >     unix/-m32
> >     unix/-m64
> >
> > Running target unix/-m32
> > Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
> > Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
> > Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
> > Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
> > Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)
> >
> > Running target unix/-m64
> > Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
> > Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
> > Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
> > Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
> > Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)
> >
> > ^^^ this shows that co-await-syntax-10.C is now OK (which is why I was happy to have this patch applied).
> >
> > 3) I apply JunMa’s patch (Do not strip cleanup_point when promote temporaries out of current stmt)
> >
> > Native configuration is x86_64-pc-linux-gnu
> >
> >                 === g++ tests ===
> >
> > Schedule of variations:
> >     unix/-m32
> >     unix/-m64
> >
> > Running target unix/-m32
> > Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
> > Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
> > Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
> > Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
> > Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)
> >
> > Running target unix/-m64
> > Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
> > Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
> > Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
> > Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess errors)
> > Running /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for excess errors)
> >
> > ^^^ and there is no change to the result.
> >
> > ====
> >
> > If it helps, I could push a branch to users/iains/ on the FSF git server with this sequence.
> Sorry for being slow replying.  This is weird, were you testing against trunk?
>
> Let's put other patches aside and consider only "Do not strip
> cleanup_point when promote temporaries out of current stmt".
>
> Here is what I got:
>
> master: 6d44c881286762628afce5169d921a388ae6a1ff
>
> Applying patch from:
> "Do not strip cleanup_point when promote temporaries out of current stmt".
>
> Configuring GCC:
> $ ../gcc/configure --prefix=... --disable-bootstrap --disable-multilib
> --disable-libasan --disable-libusan --with-isl=...
> --enable-languages=c,c++ --disable-libsanitizer
>
> Run make check:
> $ make check-gcc RUNTESTFLAGS="coroutines.exp=lambda-09-capture-object.C"
>
> The result:
> Running target unix
> Running /.../gcc/gcc/testsuite/g++.dg/coroutines/coroutines.exp ...
> PASS: g++.dg/coroutines/torture/lambda-09-capture-object.C (test for
> excess errors)
>
> Note I can't run following check:
> $ make check-gcc RUNTESTFLAGS="coro-torture.exp=lambda-09-capture-object.C"
> because it errors like:
> Running /.../gcc/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp ...
> ERROR: tcl error sourcing
> /.../gcc/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp.
> ERROR: can't read "DEFAULT_CXXFLAGS": no such variable
>     while executing
> "set DEFAULT_COROFLAGS $DEFAULT_CXXFLAGS"
>     (file "/.../gcc/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp"
> line 11)
>     invoked from within
> "source /.../gcc/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp"
>     ("uplevel" body line 1)
>     invoked from within
> "uplevel #0 source
> /.../gcc/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp"
>     invoked from within
> "catch "uplevel #0 source $test_file_name""
>
> So I suspect you were testing with local changes?  If so, please push
> the branch.
Ping.  Any updates?

Hi Iains,
I noticed you committed fix to coro-torture.exp, and it's still a
testcase pass for me.

Thanks,
bin
>
> Thanks,
> bin
>
>
>
> >
> > thanks for your patience,
> > Iain
> >
> >
Iain Sandoe March 26, 2020, 9:15 a.m. UTC | #9
Hi Bin,

Bin.Cheng via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:

> On Mon, Mar 16, 2020 at 5:34 PM Bin.Cheng <amker.cheng@gmail.com> wrote:
>> On Wed, Mar 11, 2020 at 5:07 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>>> Bin.Cheng <amker.cheng@gmail.com> wrote:
>>>
>>>> On Thu, Mar 5, 2020 at 10:18 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>>>
>>>>> With current trunk + Bin’s two approved patches.
>>>>>
>>>>> I see no change in the testcase (lambda-09-capture-object.C) before /  
>>>>> after
>>>>> the patch
>>>>> (it fails for me at -O0 only - in both cases).

>>> If it helps, I could push a branch to users/iains/ on the FSF git  
>>> server with this sequence.
>> Sorry for being slow replying.  This is weird, were you testing against  
>> trunk?

Yes.

>> So I suspect you were testing with local changes?

No local changes.

(and two different platforms - x86_64-darwin16, and x86_64-linux-gnu  
[cfarm122]).

* At least on Darwin (I have a trunk build from overnight) I see no change  
at r10-7390-g27f8c8c4c923.

>>  If so, please push the branch.
> Ping.  Any updates?

* I am currently working first on trying to complete the n4849 update  
changes.

* This remains on my TODO.
   - I am trying to figure out how you can consistently get different results from me.

> I noticed you committed fix to coro-torture.exp,

Hopefully, at least, the testsuite problem is fixed for you now?

thanks
Iain
Iain Sandoe April 7, 2020, 3:44 p.m. UTC | #10
Bin.Cheng <amker.cheng@gmail.com> wrote:

> On Mon, Mar 16, 2020 at 5:34 PM Bin.Cheng <amker.cheng@gmail.com> wrote:
>> On Wed, Mar 11, 2020 at 5:07 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>>> Bin.Cheng <amker.cheng@gmail.com> wrote:
>>> 
>>>> On Thu, Mar 5, 2020 at 10:18 PM Iain Sandoe <iain@sandoe.co.uk> wrote:

>>> If it helps, I could push a branch to users/iains/ on the FSF git server with this sequence.
>> Sorry for being slow replying.  This is weird, were you testing against trunk?

1/ @Jun Ma

  - I think your observation is correct, we should enusre that cleanups are applied where needed
   to any temps that remain after we’ve those captured by reference.

  However, I would prefer that we do this as required, rather than assuming it will be needed so
  I have an updated patch below.

2/ @ Bin / Jun Ma

  - The problem is that the testcase seems to be fragile, perhaps it was a c-reduced one?

So… I do not propose to add this test-case at this point (perhaps we could construct one that actually
tests the required behaviour - that cleanups are still  run correctly for temps that are not promoted by
capture)?

Anyway to avoid further delay, I think we should apply the patch below (I have other fixes on top of this
for open PRs)

OK for master?
thanks
Iain

    coroutines: Add cleanups, where required, to statements with captured references.
    
    When we promote captured temporaries to local variables, we also
    remove their initializers from the relevant call expression.  This
    means that we should recompute the need for a cleanup expression
    once the set of temporaries that remains becomes known.
    
    gcc/cp/ChangeLog:
    
    2020-04-07  Iain Sandoe  <iain@sandoe.co.uk>
                Jun Ma  <JunMa@linux.alibaba.com>
    
            * coroutines.cc (maybe_promote_captured_temps): Add a
            cleanup expression, if needed, to any call from which
            we promoted temporaries captured by reference.

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 983fa650b55..936be06c336 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -2798,11 +2798,13 @@ maybe_promote_captured_temps (tree *stmt, void *d)
       location_t sloc = EXPR_LOCATION (*stmt);
       tree aw_bind
 	= build3_loc (sloc, BIND_EXPR, void_type_node, NULL, NULL, NULL);
-      tree aw_statement_current;
-      if (TREE_CODE (*stmt) == CLEANUP_POINT_EXPR)
-	aw_statement_current = TREE_OPERAND (*stmt, 0);
-      else
-	aw_statement_current = *stmt;
+
+      /* Any cleanup point expression might no longer be necessary, since we
+	 are removing one or more temporaries.  */
+      tree aw_statement_current = *stmt;
+      if (TREE_CODE (aw_statement_current) == CLEANUP_POINT_EXPR)
+	aw_statement_current = TREE_OPERAND (aw_statement_current, 0);
+
       /* Collected the scope vars we need move the temps to regular. */
       tree aw_bind_body = push_stmt_list ();
       tree varlist = NULL_TREE;
@@ -2843,8 +2845,12 @@ maybe_promote_captured_temps (tree *stmt, void *d)
 	  /* Replace all instances of that temp in the original expr.  */
 	  cp_walk_tree (&aw_statement_current, replace_proxy, &pr, NULL);
 	}
-      /* What's left should be the original statement with any temporaries
-	 broken out.  */
+
+      /* What's left should be the original statement with any co_await
+	 captured temporaries broken out.  Other temporaries might remain
+	 so see if we need to wrap the revised statement in a cleanup.  */
+      aw_statement_current =
+	maybe_cleanup_point_expr_void (aw_statement_current);
       add_stmt (aw_statement_current);
       BIND_EXPR_BODY (aw_bind) = pop_stmt_list (aw_bind_body);
       awpts->captured_temps.empty ();
Li, Pan2 via Gcc-patches April 8, 2020, 7:41 a.m. UTC | #11
On Tue, Apr 7, 2020 at 11:45 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>
> Bin.Cheng <amker.cheng@gmail.com> wrote:
>
> > On Mon, Mar 16, 2020 at 5:34 PM Bin.Cheng <amker.cheng@gmail.com> wrote:
> >> On Wed, Mar 11, 2020 at 5:07 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
> >>> Bin.Cheng <amker.cheng@gmail.com> wrote:
> >>>
> >>>> On Thu, Mar 5, 2020 at 10:18 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>
> >>> If it helps, I could push a branch to users/iains/ on the FSF git server with this sequence.
> >> Sorry for being slow replying.  This is weird, were you testing against trunk?
>
> 1/ @Jun Ma
>
>   - I think your observation is correct, we should enusre that cleanups are applied where needed
>    to any temps that remain after we’ve those captured by reference.
>
>   However, I would prefer that we do this as required, rather than assuming it will be needed so
>   I have an updated patch below.
Sorry it's been quite long, I don't remember the detail of this
change.  Since this new version fixes the ICE too, please go ahead
commit it.
>
> 2/ @ Bin / Jun Ma
>
>   - The problem is that the testcase seems to be fragile, perhaps it was a c-reduced one?
It's strange, however, I don't see being c-reduced has impact here in
this scenario.
>
> So… I do not propose to add this test-case at this point (perhaps we could construct one that actually
> tests the required behaviour - that cleanups are still  run correctly for temps that are not promoted by
> capture)?
>
> Anyway to avoid further delay, I think we should apply the patch below (I have other fixes on top of this
> for open PRs)
>
> OK for master?
Yes, please.  Also one of approved patch is waiting for this one.

Thanks,
bin
> thanks
> Iain
>
>     coroutines: Add cleanups, where required, to statements with captured references.
>
>     When we promote captured temporaries to local variables, we also
>     remove their initializers from the relevant call expression.  This
>     means that we should recompute the need for a cleanup expression
>     once the set of temporaries that remains becomes known.
>
>     gcc/cp/ChangeLog:
>
>     2020-04-07  Iain Sandoe  <iain@sandoe.co.uk>
>                 Jun Ma  <JunMa@linux.alibaba.com>
>
>             * coroutines.cc (maybe_promote_captured_temps): Add a
>             cleanup expression, if needed, to any call from which
>             we promoted temporaries captured by reference.
>
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index 983fa650b55..936be06c336 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -2798,11 +2798,13 @@ maybe_promote_captured_temps (tree *stmt, void *d)
>        location_t sloc = EXPR_LOCATION (*stmt);
>        tree aw_bind
>         = build3_loc (sloc, BIND_EXPR, void_type_node, NULL, NULL, NULL);
> -      tree aw_statement_current;
> -      if (TREE_CODE (*stmt) == CLEANUP_POINT_EXPR)
> -       aw_statement_current = TREE_OPERAND (*stmt, 0);
> -      else
> -       aw_statement_current = *stmt;
> +
> +      /* Any cleanup point expression might no longer be necessary, since we
> +        are removing one or more temporaries.  */
> +      tree aw_statement_current = *stmt;
> +      if (TREE_CODE (aw_statement_current) == CLEANUP_POINT_EXPR)
> +       aw_statement_current = TREE_OPERAND (aw_statement_current, 0);
> +
>        /* Collected the scope vars we need move the temps to regular. */
>        tree aw_bind_body = push_stmt_list ();
>        tree varlist = NULL_TREE;
> @@ -2843,8 +2845,12 @@ maybe_promote_captured_temps (tree *stmt, void *d)
>           /* Replace all instances of that temp in the original expr.  */
>           cp_walk_tree (&aw_statement_current, replace_proxy, &pr, NULL);
>         }
> -      /* What's left should be the original statement with any temporaries
> -        broken out.  */
> +
> +      /* What's left should be the original statement with any co_await
> +        captured temporaries broken out.  Other temporaries might remain
> +        so see if we need to wrap the revised statement in a cleanup.  */
> +      aw_statement_current =
> +       maybe_cleanup_point_expr_void (aw_statement_current);
>        add_stmt (aw_statement_current);
>        BIND_EXPR_BODY (aw_bind) = pop_stmt_list (aw_bind_body);
>        awpts->captured_temps.empty ();
>
Nathan Sidwell April 8, 2020, 7:20 p.m. UTC | #12
On 4/7/20 11:44 AM, Iain Sandoe wrote:
> Bin.Cheng <amker.cheng@gmail.com> wrote:
> 
>> On Mon, Mar 16, 2020 at 5:34 PM Bin.Cheng <amker.cheng@gmail.com> wrote:
>>> On Wed, Mar 11, 2020 at 5:07 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>>>> Bin.Cheng <amker.cheng@gmail.com> wrote:
>>>>
>>>>> On Thu, Mar 5, 2020 at 10:18 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
> 
>>>> If it helps, I could push a branch to users/iains/ on the FSF git server with this sequence.
>>> Sorry for being slow replying.  This is weird, were you testing against trunk?
> 
> 1/ @Jun Ma
> 
>    - I think your observation is correct, we should enusre that cleanups are applied where needed
>     to any temps that remain after we’ve those captured by reference.
> 
>    However, I would prefer that we do this as required, rather than assuming it will be needed so
>    I have an updated patch below.
> 
> 2/ @ Bin / Jun Ma
> 
>    - The problem is that the testcase seems to be fragile, perhaps it was a c-reduced one?
> 
> So… I do not propose to add this test-case at this point (perhaps we could construct one that actually
> tests the required behaviour - that cleanups are still  run correctly for temps that are not promoted by
> capture)?
> 
> Anyway to avoid further delay, I think we should apply the patch below (I have other fixes on top of this
> for open PRs)
> 
> OK for master?
> thanks
> Iain
> 
>      coroutines: Add cleanups, where required, to statements with captured references.
>      
>      When we promote captured temporaries to local variables, we also
>      remove their initializers from the relevant call expression.  This
>      means that we should recompute the need for a cleanup expression
>      once the set of temporaries that remains becomes known.
>      
>      gcc/cp/ChangeLog:
>      
>      2020-04-07  Iain Sandoe  <iain@sandoe.co.uk>
>                  Jun Ma  <JunMa@linux.alibaba.com>
>      
>              * coroutines.cc (maybe_promote_captured_temps): Add a
>              cleanup expression, if needed, to any call from which
>              we promoted temporaries captured by reference.
> 
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index 983fa650b55..936be06c336 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -2798,11 +2798,13 @@ maybe_promote_captured_temps (tree *stmt, void *d)
>         location_t sloc = EXPR_LOCATION (*stmt);
>         tree aw_bind
>   	= build3_loc (sloc, BIND_EXPR, void_type_node, NULL, NULL, NULL);
> -      tree aw_statement_current;
> -      if (TREE_CODE (*stmt) == CLEANUP_POINT_EXPR)
> -	aw_statement_current = TREE_OPERAND (*stmt, 0);
> -      else
> -	aw_statement_current = *stmt;
> +
> +      /* Any cleanup point expression might no longer be necessary, since we
> +	 are removing one or more temporaries.  */
> +      tree aw_statement_current = *stmt;
> +      if (TREE_CODE (aw_statement_current) == CLEANUP_POINT_EXPR)
> +	aw_statement_current = TREE_OPERAND (aw_statement_current, 0);
> +
>         /* Collected the scope vars we need move the temps to regular. */
>         tree aw_bind_body = push_stmt_list ();
>         tree varlist = NULL_TREE;
> @@ -2843,8 +2845,12 @@ maybe_promote_captured_temps (tree *stmt, void *d)
>   	  /* Replace all instances of that temp in the original expr.  */
>   	  cp_walk_tree (&aw_statement_current, replace_proxy, &pr, NULL);
>   	}
> -      /* What's left should be the original statement with any temporaries
> -	 broken out.  */
> +
> +      /* What's left should be the original statement with any co_await
> +	 captured temporaries broken out.  Other temporaries might remain
> +	 so see if we need to wrap the revised statement in a cleanup.  */
> +      aw_statement_current =
> +	maybe_cleanup_point_expr_void (aw_statement_current);
>         add_stmt (aw_statement_current);
>         BIND_EXPR_BODY (aw_bind) = pop_stmt_list (aw_bind_body);
>         awpts->captured_temps.empty ();
> 

ok
diff mbox series

Patch

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 9fcbb647201..3095b46ecb1 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -2681,11 +2681,9 @@  maybe_promote_captured_temps (tree *stmt, void *d)
       location_t sloc = EXPR_LOCATION (*stmt);
       tree aw_bind
 	= build3_loc (sloc, BIND_EXPR, void_type_node, NULL, NULL, NULL);
-      tree aw_statement_current;
-      if (TREE_CODE (*stmt) == CLEANUP_POINT_EXPR)
-	aw_statement_current = TREE_OPERAND (*stmt, 0);
-      else
-	aw_statement_current = *stmt;
+      /* Do not skip cleanup_point since maybe there are other temporaries
+         need cleanup.  */
+      tree aw_statement_current = *stmt;
       /* Collected the scope vars we need move the temps to regular. */
       tree aw_bind_body = push_stmt_list ();
       tree varlist = NULL_TREE;
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/lambda-09-capture-object.C b/gcc/testsuite/g++.dg/coroutines/torture/lambda-09-capture-object.C
new file mode 100644
index 00000000000..1297b72cc8f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/lambda-09-capture-object.C
@@ -0,0 +1,132 @@ 
+//  { dg-do compile }
+//  { dg-additional-options "-Wno-return-type" }
+
+#include "../coro.h"
+#include <tuple>
+#include <functional>
+
+template <typename, typename = std::void_t<>> struct a;
+template <typename b>
+struct a<b, std::void_t<decltype(std::declval<b>().await_resume())>>
+    : std::conjunction<> {};
+template <typename b>
+auto c(b value, int) -> decltype(value.operator co_await());
+template <typename b, std::enable_if_t<a<b>::value, int> = 0> b c(b, int);
+template <typename b> auto d(b value) -> decltype(c(value, 3)) {}
+template <typename b> struct f {
+  using e = decltype(d(std::declval<b>()));
+  using h = decltype(std::declval<e>().await_resume());
+};
+template <typename> class s;
+template <typename... g> class s<std::tuple<g...>> {
+public:
+  s(std::tuple<g...>);
+  auto operator co_await() {
+    struct aa {
+      std::tuple<g...> await_resume() {};
+      s i;
+    };
+    return aa{*this};
+  }
+};
+template <typename j> class k {
+public:
+  using l = std::coroutine_handle<k>;
+  j m();
+};
+template <typename j> class ab {
+public:
+  using promise_type = k<j>;
+  using l = typename promise_type::l;
+  auto m() { return n.promise().m(); }
+  auto ac() { return m(); }
+  l n;
+};
+template <typename o, typename j = typename f<o>::h,
+          std::enable_if_t<!std::is_void_v<j>, int> = 0>
+ab<j> p(o);
+template <typename b> struct u { using ad = b; };
+template <typename b> using t = typename u<b>::ad;
+template <typename... ae, std::enable_if_t<std::conjunction_v<>, int> = 0>
+auto af(ae... ag) {
+  return s<std::tuple<ab<typename f<t<std::remove_reference_t<ae>>>::h>...>>(
+      std::make_tuple(p(ag)...));
+}
+template <typename q, typename o> class ah {
+  using e = typename f<o>::e;
+
+public:
+  ah(q ai, o aj) : r(ai), ak(d(aj)) {}
+  auto await_ready() { return 0; }
+  template <typename v> auto await_suspend(std::coroutine_handle<v>) {}
+  template <typename w = decltype(0),
+            std::enable_if_t<!std::is_void_v<w>, int> = 0>
+  auto await_resume() {
+    return invoke(r, ak.await_resume());
+  }
+  q r;
+  e ak;
+};
+template <typename q, typename o> class x {
+public:
+  template <typename y, typename al,
+            std::enable_if_t<std::is_constructible_v<o, al>, int> = 0>
+  x(y ai, al aj) : r(ai), i(aj) {}
+  auto operator co_await() { return ah(r, i); }
+  q r;
+  o i;
+};
+template <typename q, typename o> auto am(q ai, o aj) {
+  return x<std::remove_cv_t<std::remove_reference_t<q>>,
+           std::remove_cv_t<std::remove_reference_t<o>>>(ai, aj);
+}
+template <typename... ae, std::enable_if_t<std::conjunction_v<>, int> = 0>
+auto an(ae... ag) {
+  return am(
+      [](auto ao) {
+        auto ap =
+            apply([](auto... aq) { return std::make_tuple(aq.ac()...); }, ao);
+        return ap;
+      },
+      af(ag...));
+}
+class ar;
+class z {
+public:
+  ar as();
+};
+class at {
+public:
+  ~at();
+};
+class ar {
+public:
+  at await_resume();
+};
+class au;
+class av {
+  struct aw {
+    bool await_ready();
+    template <typename v> void await_suspend(std::coroutine_handle<v>);
+    void await_resume();
+  };
+
+public:
+  auto initial_suspend() { return std::suspend_always{}; }
+  auto final_suspend() { return aw{}; }
+};
+class ax : public av {
+public:
+  au get_return_object();
+  void unhandled_exception();
+};
+class au {
+public:
+  using promise_type = ax;
+};
+void d() {
+  []() -> au {
+    z ay;
+    co_await an(ay.as());
+  };
+}