Reland Allow normal function results of @yield_once coroutines #71645
Conversation
asl
requested review from
kavon,
hborla,
slavapestov and
xedin
as code owners
|
Please test with swiftlang/llvm-project#8199 |
|
preset=buildbot,tools=RA,stdlib=DA |
|
Tagging @rxwei @BradLarson |
asl
force-pushed
the
end-apply-result
branch
from
e9f5cfc to
debe0dd
Compare
|
Please test with swiftlang/llvm-project#8199 |
|
preset=buildbot,tools=RA,stdlib=DA |
|
preset=buildbot,tools=RA,stdlib=DA |
asl
force-pushed
the
end-apply-result
branch
from
debe0dd to
cd920a7
Compare
|
Please test with swiftlang/llvm-project#8199 |
|
@rjmccall ping |
|
@rjmccall @hborla @xedin @slavapestov @kavon @tkremenek Another weekly ping. How can we proceed with reviews? It is pending since November and it's almost March now. Please let me know if you are not interested in these PRs, so we can just abandon these contributions Thanks! |
|
The review has been slow, but describing it as "pending since November" is not accurate. Your patch was approved, committed, and then got reverted because it was breaking a lot of code. I'll take a look at the revised approach. |
I think it's not accurate as well. Indeed there was a breakage that was exposed in one particular configuration outlining subtle issues how swift coroutines are implemented at LLVM side. To fix this the workaround was introduced at Swift side as it is still under discussion what would be a proper LLVM fix. Anyway I appreciate your time and thanks for reviews! |
|
@rjmccall Thanks, John, much appreaciated. I will rebase, re-test as much as possible and we will see how it will go. |
asl
force-pushed
the
end-apply-result
branch
from
cd920a7 to
ad6751c
Compare
|
Please test with swiftlang/llvm-project#8199 |
|
preset=buildbot,tools=RA,stdlib=DA |
|
So, the Linux fails with debug stdlib look unrelated. Everything else is passing, including debug stdlib configuration on MacOS that failed before. |
|
Please test with swiftlang/llvm-project#8199 |
|
preset=buildbot,tools=RA,stdlib=DA |
|
Please test with swiftlang/llvm-project#8199 |
|
preset=buildbot,tools=RA,stdlib=DA |
…h never returns. This is not true to Swift coroutines as unwind path should end with error result.
|
Please test with swiftlang/llvm-project#8199 |
|
Please test with swiftlang/llvm-project#8199 |
The main difference as compared to #69843 is workaround to LLVM coroutines lowering. LLVM assumes that unwind patch of
retconcoroutines never returns. This is not quite true in case of Swift coroutines as we might abort a coroutines in case when something is thrown during coroutine execution. In such case the unwind path should terminate normally, but with error result set.This certainly should be fixed at LLVM side properly. For now workaround this issue by ending LLVM corouting normally always, returning undefs on unwind path.