-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Fix for async drop inside async gen fn #141932
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
Fix for async drop inside async gen fn #141932
Conversation
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
r? oli-obk |
This comment has been minimized.
This comment has been minimized.
8e0c3d0
to
8e20c56
Compare
// yield (const false) is used for async drop yields inside AsyncGen | ||
// We need to convert value to Poll<OptRet>::Pending |
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.
Can we directly generate the right yield in the async drop logic?
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.
TerminatorKind::Yield value is Operand
, but we need Rvalue::Aggregate
(make_aggregate_adt
).
As far as I understand, we can do Operand
with an additional local variable.
So, instead of 'return Poll::Pending;', the generated code will be 'x = Poll::Pending; return x;'
Value for Yield is generated here:
let value = Operand::Constant(Box::new(ConstOperand { |
Does it cost additional temporary local?
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.
We can generate a const operand directly for a value of Poll::Pending(())
, so that would not be the issue.
8e20c56
to
d5a9a00
Compare
Oh heh that works, nice @bors r+ rollup |
Rollup of 11 pull requests Successful merges: - #141890 (Add link to correct documentation in htmldocck.py) - #141932 (Fix for async drop inside async gen fn) - #141960 (Use non-2015 edition paths in tests that do not test for their resolution) - #141968 (Run wfcheck in one big loop instead of per module) - #141969 (Triagebot: Remove `assign.users_on_vacation`) - #141985 (Ensure query keys are printed with reduced queries) - #141999 (Visit the ident in `PreciseCapturingNonLifetimeArg`.) - #142005 (Change `tag_field` to `FieldIdx` in `Variants::Multiple`) - #142017 (Fix incorrect use of "recommend" over "recommended") - #142024 (Don't refer to 'this tail expression' in expansion.) - #142025 (Don't refer to 'local binding' in extern macro.) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #141932 - azhogin:azhogin/async-drop-inside-asyncgen-fix, r=oli-obk Fix for async drop inside async gen fn Return value (for yield) is corrected for async drop inside async gen function. In CFG, when internal async drop future is polled and returned `Poll<()>::Pending`, then async gen resume function returns `Poll<(OptRet)>::Pending`. Fixes #140530
Return value (for yield) is corrected for async drop inside async gen function.
In CFG, when internal async drop future is polled and returned
Poll<()>::Pending
, then async gen resume function returnsPoll<(OptRet)>::Pending
.Fixes #140530