-
Notifications
You must be signed in to change notification settings - Fork 263
Remove low-level coroutine suspension notifications #1228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove low-level coroutine suspension notifications #1228
Conversation
|
Tests passed locally... |
| IAsyncAction NoSuspend_IAsyncAction() | ||
| { | ||
| co_await resume_after(0s); | ||
| co_await 0s; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this change have to do with ripping out the low-level hooks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This workaround was added because of the low level hooks preventing Clang from building this code.
So removing resume_after helps make sure that we have coverage for this functionality on Clang.
|
It generally looks good to me, but @oldnewthing made the last few tweaks to coroutine support so I'd appreciate his eyes on this change as well. |
oldnewthing
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I never thought we would be rid of this code.
kennykerr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woot!
The opposite of #372
Fixes #1219