[PATCH] D145639: [Coroutines] Fix premature conversion of return object

2023-03-21 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG43f5085fa80f: [Coroutines] Fix premature conversion of return object (authored by bruno). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145639/new/

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

2023-03-21 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno updated this revision to Diff 507084. bruno added a comment. Put dependency in place for D145641 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145639/new/ https://reviews.llvm.org/D145639 Files:

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

2023-03-17 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added a comment. > @bruno, @ChuanqiXu please let us know if you have any objections, otherwise > we will land the revert in ~2 hours. No sweat, I didn't see this in time. Thanks for the reduced testcase. > What's the resolution here? Can we revert this and continue the discussions >

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

2023-03-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. I suspect D145641 won't land today in the EU working hours. My suggestion would be to revert this patch and reland together with D145641 . This should be in line with @ChuanqiXu's suggestions to

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

2023-03-17 Thread Bogdan Graur via Phabricator via cfe-commits
bgraur added a comment. What's the resolution here? Can we revert this and continue the discussions independently? We can always re-land this change if the conclusion is that the approach here is the one that we want. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

2023-03-16 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D145639#4198837 , @ilya-biryukov wrote: > In D145639#4198794 , @ChuanqiXu > wrote: > >>> ! In D145639#4198815 , @ChuanqiXu >>> wrote: >>>

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

2023-03-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In D145639#4198794 , @ChuanqiXu wrote: > Reverting is common in Clang/LLVM. But let's wait for the response from > @bruno temporarily. I think it makes sense to revert this one if @bruno don't > response tomorrow. And we

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

2023-03-16 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > The RVO seems to be mandated by the standard. Yeah, I thought so... But WG21 decided to not make it in a recent meeting... So we can only make it as a non-mandated compiler optimization. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

2023-03-16 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. The reproducer seems like what we're searching for tests in https://reviews.llvm.org/D145641. Reverting is common in Clang/LLVM. But let's wait for the response from @bruno temporarily. I think it makes sense to revert this one if @bruno don't response tomorrow. And

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

2023-03-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Reverting the RVO breaks some coroutine code we have. The short repro is https://gcc.godbolt.org/z/1543qc8Ee (code at the end of the post, very similar to the deleted constructor in `warn-throw-out-noexcept-coro.cpp`): The RVO seems to be mandated by the standard

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

2023-03-09 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG54225c457a33: [Coroutines] Fix premature conversion of return object (authored by bruno). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

2023-03-09 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added a comment. Test failures are unrelated, thanks for the review @ChuanqiXu Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145639/new/ https://reviews.llvm.org/D145639 ___ cfe-commits mailing

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

2023-03-09 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. This is basically a reverting of https://reviews.llvm.org/D117087. So it should be good according to our previous talk. Comment at:

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

2023-03-08 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno created this revision. bruno added a reviewer: ChuanqiXu. Herald added subscribers: hoy, modimo, wenlei. Herald added a project: All. bruno requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix