-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Async fn resume after completion #66321
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
Merged
bors
merged 12 commits into
rust-lang:master
from
ninjasource:async-fn-resume-after-completion
Nov 29, 2019
+232
−66
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
ec41fda
Squash
fb0972d
Fixed merge issue
88821ed
Fixed merge issue
6ef625f
Remove duplication using single variant for error
4b85ee2
Fixed tidy errors
eda2d41
Removed FIXME comment
63b36e7
Reduced repetition by refactoring new body to constructor function
65c3996
Fixed unit test
ed66492
Block indent formatting
e88948a
Fail fast if generator_kind is None
6531ba8
Moved tests and fixed merge conflict
851492c
Ignore wasm for panic tests
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
46 changes: 46 additions & 0 deletions
46
src/test/ui/async-await/issues/issue-65419/issue-65419-async-fn-resume-after-completion.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
// issue 65419 - Attempting to run an async fn after completion mentions generators when it should | ||
// be talking about `async fn`s instead. | ||
|
||
// run-fail | ||
// error-pattern: thread 'main' panicked at '`async fn` resumed after completion' | ||
// edition:2018 | ||
// ignore-wasm no panic or subprocess support | ||
// ignore-emscripten no panic or subprocess support | ||
|
||
#![feature(generators, generator_trait)] | ||
|
||
async fn foo() { | ||
} | ||
|
||
fn main() { | ||
let mut future = Box::pin(foo()); | ||
executor::block_on(future.as_mut()); | ||
executor::block_on(future.as_mut()); | ||
} | ||
|
||
mod executor { | ||
use core::{ | ||
future::Future, | ||
pin::Pin, | ||
task::{Context, Poll, RawWaker, RawWakerVTable, Waker}, | ||
}; | ||
|
||
pub fn block_on<F: Future>(mut future: F) -> F::Output { | ||
let mut future = unsafe { Pin::new_unchecked(&mut future) }; | ||
|
||
static VTABLE: RawWakerVTable = RawWakerVTable::new( | ||
|_| unimplemented!("clone"), | ||
|_| unimplemented!("wake"), | ||
|_| unimplemented!("wake_by_ref"), | ||
|_| (), | ||
); | ||
let waker = unsafe { Waker::from_raw(RawWaker::new(core::ptr::null(), &VTABLE)) }; | ||
let mut context = Context::from_waker(&waker); | ||
|
||
loop { | ||
if let Poll::Ready(val) = future.as_mut().poll(&mut context) { | ||
break val; | ||
} | ||
} | ||
} | ||
} |
52 changes: 52 additions & 0 deletions
52
src/test/ui/async-await/issues/issue-65419/issue-65419-async-fn-resume-after-panic.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
// issue 65419 - Attempting to run an async fn after completion mentions generators when it should | ||
// be talking about `async fn`s instead. Should also test what happens when it panics. | ||
|
||
// run-fail | ||
// error-pattern: thread 'main' panicked at '`async fn` resumed after panicking' | ||
// edition:2018 | ||
// ignore-wasm no panic or subprocess support | ||
// ignore-emscripten no panic or subprocess support | ||
|
||
#![feature(generators, generator_trait)] | ||
|
||
use std::panic; | ||
|
||
async fn foo() { | ||
panic!(); | ||
} | ||
|
||
fn main() { | ||
let mut future = Box::pin(foo()); | ||
panic::catch_unwind(panic::AssertUnwindSafe(|| { | ||
executor::block_on(future.as_mut()); | ||
})); | ||
|
||
executor::block_on(future.as_mut()); | ||
} | ||
|
||
mod executor { | ||
use core::{ | ||
future::Future, | ||
pin::Pin, | ||
task::{Context, Poll, RawWaker, RawWakerVTable, Waker}, | ||
}; | ||
|
||
pub fn block_on<F: Future>(mut future: F) -> F::Output { | ||
let mut future = unsafe { Pin::new_unchecked(&mut future) }; | ||
|
||
static VTABLE: RawWakerVTable = RawWakerVTable::new( | ||
|_| unimplemented!("clone"), | ||
|_| unimplemented!("wake"), | ||
|_| unimplemented!("wake_by_ref"), | ||
|_| (), | ||
); | ||
let waker = unsafe { Waker::from_raw(RawWaker::new(core::ptr::null(), &VTABLE)) }; | ||
let mut context = Context::from_waker(&waker); | ||
|
||
loop { | ||
if let Poll::Ready(val) = future.as_mut().poll(&mut context) { | ||
break val; | ||
} | ||
} | ||
} | ||
} |
25 changes: 25 additions & 0 deletions
25
src/test/ui/async-await/issues/issue-65419/issue-65419-generator-resume-after-completion.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
// issue 65419 - Attempting to run an `async fn` after completion mentions generators when it should | ||
// be talking about `async fn`s instead. Regression test added to make sure generators still | ||
// panic when resumed after completion. | ||
|
||
// run-fail | ||
// error-pattern:generator resumed after completion | ||
// edition:2018 | ||
// ignore-wasm no panic or subprocess support | ||
// ignore-emscripten no panic or subprocess support | ||
|
||
#![feature(generators, generator_trait)] | ||
|
||
use std::{ | ||
ops::Generator, | ||
pin::Pin, | ||
}; | ||
|
||
fn main() { | ||
let mut g = || { | ||
yield; | ||
}; | ||
Pin::new(&mut g).resume(); // Yields once. | ||
Pin::new(&mut g).resume(); // Completes here. | ||
Pin::new(&mut g).resume(); // Panics here. | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
So... the error is
RuntimeError: unreachable
. Check whether any tests check for that error pattern, or if there are any wasm panic tests and how to check them. I think it would also be OK to just add// ignore-wasm
to these tests if there's no way to specify different error patterns for different platformsThere 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.
(Assuming that the ignore-wasm comment is how this is done.) Search for it in the test suite, if it exists, that's it, if not, find out how to disable tests on wasm by searching for "wasm"
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.
The reason it's OK to disable the tests on wasm is that here we only want to see a specific panic message. We have tests to see that panicking in general works, so assuming panicking works and the tests work on most platforms, there's basically no way they'd be broken on wasm. Wasn't just generally doesn't show the panic message, but that's not something you need to solve in this PR