Skip to content

Commit

Permalink
Rollup merge of rust-lang#105250 - Swatinem:async-rm-resumety, r=oli-obk
Browse files Browse the repository at this point in the history
Replace usage of `ResumeTy` in async lowering with `Context`

Replaces using `ResumeTy` / `get_context` in favor of using `&'static mut Context<'_>`.

Usage of the `'static` lifetime here is technically "cheating", and replaces the raw pointer in `ResumeTy` and the `get_context` fn that pulls the correct lifetimes out of thin air.

fixes rust-lang#104828 and rust-lang#104321 (comment)

r? ```@oli-obk```
  • Loading branch information
matthiaskrgr authored Dec 5, 2022
2 parents a0d00c8 + 9dbb189 commit 90ccb17
Show file tree
Hide file tree
Showing 14 changed files with 68 additions and 38 deletions.
50 changes: 34 additions & 16 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use rustc_hir::def::Res;
use rustc_hir::definitions::DefPathData;
use rustc_session::errors::report_lit_error;
use rustc_span::source_map::{respan, DesugaringKind, Span, Spanned};
use rustc_span::symbol::{sym, Ident};
use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::DUMMY_SP;
use thin_vec::thin_vec;

Expand Down Expand Up @@ -592,14 +592,38 @@ impl<'hir> LoweringContext<'_, 'hir> {
) -> hir::ExprKind<'hir> {
let output = ret_ty.unwrap_or_else(|| hir::FnRetTy::DefaultReturn(self.lower_span(span)));

// Resume argument type: `ResumeTy`
let unstable_span =
self.mark_span_with_reason(DesugaringKind::Async, span, self.allow_gen_future.clone());
let resume_ty = hir::QPath::LangItem(hir::LangItem::ResumeTy, unstable_span, None);
// Resume argument type, which should be `&mut Context<'_>`.
// NOTE: Using the `'static` lifetime here is technically cheating.
// The `Future::poll` argument really is `&'a mut Context<'b>`, but we cannot
// express the fact that we are not storing it across yield-points yet,
// and we would thus run into lifetime errors.
// See <https://github.com/rust-lang/rust/issues/68923>.
// Our lowering makes sure we are not mis-using the `_task_context` input type
// in the sense that we are indeed not using it across yield points. We
// get a fresh `&mut Context` for each resume / call of `Future::poll`.
// This "cheating" was previously done with a `ResumeTy` that contained a raw
// pointer, and a `get_context` accessor that pulled the `Context` lifetimes
// out of thin air.
let context_lifetime_ident = Ident::with_dummy_span(kw::StaticLifetime);
let context_lifetime = self.arena.alloc(hir::Lifetime {
hir_id: self.next_id(),
ident: context_lifetime_ident,
res: hir::LifetimeName::Static,
});
let context_path =
hir::QPath::LangItem(hir::LangItem::Context, self.lower_span(span), None);
let context_ty = hir::MutTy {
ty: self.arena.alloc(hir::Ty {
hir_id: self.next_id(),
kind: hir::TyKind::Path(context_path),
span: self.lower_span(span),
}),
mutbl: hir::Mutability::Mut,
};
let input_ty = hir::Ty {
hir_id: self.next_id(),
kind: hir::TyKind::Path(resume_ty),
span: unstable_span,
kind: hir::TyKind::Rptr(context_lifetime, context_ty),
span: self.lower_span(span),
};

// The closure/generator `FnDecl` takes a single (resume) argument of type `input_ty`.
Expand Down Expand Up @@ -710,7 +734,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
/// mut __awaitee => loop {
/// match unsafe { ::std::future::Future::poll(
/// <::std::pin::Pin>::new_unchecked(&mut __awaitee),
/// ::std::future::get_context(task_context),
/// task_context,
/// ) } {
/// ::std::task::Poll::Ready(result) => break result,
/// ::std::task::Poll::Pending => {}
Expand Down Expand Up @@ -751,7 +775,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
// unsafe {
// ::std::future::Future::poll(
// ::std::pin::Pin::new_unchecked(&mut __awaitee),
// ::std::future::get_context(task_context),
// task_context,
// )
// }
let poll_expr = {
Expand All @@ -769,16 +793,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
arena_vec![self; ref_mut_awaitee],
Some(expr_hir_id),
);
let get_context = self.expr_call_lang_item_fn_mut(
gen_future_span,
hir::LangItem::GetContext,
arena_vec![self; task_context],
Some(expr_hir_id),
);
let call = self.expr_call_lang_item_fn(
span,
hir::LangItem::FuturePoll,
arena_vec![self; new_unchecked, get_context],
arena_vec![self; new_unchecked, task_context],
Some(expr_hir_id),
);
self.arena.alloc(self.expr_unsafe(call))
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,10 +286,9 @@ language_item_table! {

// FIXME(swatinem): the following lang items are used for async lowering and
// should become obsolete eventually.
ResumeTy, sym::ResumeTy, resume_ty, Target::Struct, GenericRequirement::None;
IdentityFuture, sym::identity_future, identity_future_fn, Target::Fn, GenericRequirement::None;
GetContext, sym::get_context, get_context_fn, Target::Fn, GenericRequirement::None;

Context, sym::Context, context, Target::Struct, GenericRequirement::None;
FuturePoll, sym::poll, future_poll_fn, Target::Method(MethodKind::Trait { body: false }), GenericRequirement::None;

FromFrom, sym::from, from_fn, Target::Method(MethodKind::Trait { body: false }), GenericRequirement::None;
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ symbols! {
Capture,
Center,
Clone,
Context,
Continue,
Copy,
Count,
Expand Down Expand Up @@ -264,7 +265,6 @@ symbols! {
Relaxed,
Release,
Result,
ResumeTy,
Return,
Right,
Rust,
Expand Down Expand Up @@ -754,7 +754,6 @@ symbols! {
generic_associated_types_extended,
generic_const_exprs,
generic_param_attrs,
get_context,
global_allocator,
global_asm,
globs,
Expand Down
10 changes: 8 additions & 2 deletions library/core/src/future/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub use poll_fn::{poll_fn, PollFn};
/// non-Send/Sync as well, and we don't want that.
///
/// It also simplifies the HIR lowering of `.await`.
#[cfg_attr(not(bootstrap), lang = "ResumeTy")]
// FIXME(swatinem): This type can be removed when bumping the bootstrap compiler
#[doc(hidden)]
#[unstable(feature = "gen_future", issue = "50547")]
#[derive(Debug, Copy, Clone)]
Expand All @@ -61,6 +61,7 @@ unsafe impl Sync for ResumeTy {}
/// This function returns a `GenFuture` underneath, but hides it in `impl Trait` to give
/// better error messages (`impl Future` rather than `GenFuture<[closure.....]>`).
// This is `const` to avoid extra errors after we recover from `const async fn`
// FIXME(swatinem): This fn can be removed when bumping the bootstrap compiler
#[cfg_attr(bootstrap, lang = "from_generator")]
#[doc(hidden)]
#[unstable(feature = "gen_future", issue = "50547")]
Expand Down Expand Up @@ -102,7 +103,8 @@ where
GenFuture(gen)
}

#[lang = "get_context"]
// FIXME(swatinem): This fn can be removed when bumping the bootstrap compiler
#[cfg_attr(bootstrap, lang = "get_context")]
#[doc(hidden)]
#[unstable(feature = "gen_future", issue = "50547")]
#[must_use]
Expand All @@ -113,6 +115,10 @@ pub unsafe fn get_context<'a, 'b>(cx: ResumeTy) -> &'a mut Context<'b> {
unsafe { &mut *cx.0.as_ptr().cast() }
}

// FIXME(swatinem): This fn is currently needed to work around shortcomings
// in type and lifetime inference.
// See the comment at the bottom of `LoweringContext::make_async_expr` and
// <https://github.com/rust-lang/rust/issues/104826>.
#[cfg_attr(not(bootstrap), lang = "identity_future")]
#[doc(hidden)]
#[unstable(feature = "gen_future", issue = "50547")]
Expand Down
1 change: 1 addition & 0 deletions library/core/src/task/wake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ impl RawWakerVTable {
/// Currently, `Context` only serves to provide access to a [`&Waker`](Waker)
/// which can be used to wake the current task.
#[stable(feature = "futures_api", since = "1.36.0")]
#[cfg_attr(not(bootstrap), lang = "Context")]
pub struct Context<'a> {
waker: &'a Waker,
// Ensure we future-proof against variance changes by forcing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ LL | async fn bar2<T>(_: T) -> ! {
LL | | panic!()
LL | | }
| |_^
= note: required because it captures the following types: `ResumeTy`, `Option<bool>`, `impl Future<Output = !>`, `()`
= note: required because it captures the following types: `&mut Context<'_>`, `Option<bool>`, `impl Future<Output = !>`, `()`
note: required because it's used within this `async fn` body
--> $DIR/async-await-let-else.rs:21:32
|
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/async-await/issue-68112.drop_tracking.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ note: required because it appears within the type `impl Future<Output = Arc<RefC
|
LL | fn make_non_send_future2() -> impl Future<Output = Arc<RefCell<i32>>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: required because it captures the following types: `ResumeTy`, `impl Future<Output = Arc<RefCell<i32>>>`, `()`, `Ready<i32>`
= note: required because it captures the following types: `&mut Context<'_>`, `impl Future<Output = Arc<RefCell<i32>>>`, `()`, `Ready<i32>`
note: required because it's used within this `async` block
--> $DIR/issue-68112.rs:60:20
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ note: required because it appears within the type `impl Future<Output = Arc<RefC
|
LL | fn make_non_send_future2() -> impl Future<Output = Arc<RefCell<i32>>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: required because it captures the following types: `ResumeTy`, `impl Future<Output = Arc<RefCell<i32>>>`, `()`, `i32`, `Ready<i32>`
= note: required because it captures the following types: `&mut Context<'_>`, `impl Future<Output = Arc<RefCell<i32>>>`, `()`, `i32`, `Ready<i32>`
note: required because it's used within this `async` block
--> $DIR/issue-68112.rs:60:20
|
Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/async-await/issue-69446-fnmut-capture.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ LL | | });
|
= note: `FnMut` closures only have access to their captured variables while they are executing...
= note: ...therefore, they cannot allow references to captured variables to escape
= note: requirement occurs because of a mutable reference to `Context<'_>`
= note: mutable references are invariant over their type parameter
= help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance

error: aborting due to previous error

Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ LL | async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
| ___________________________________________________________________^
LL | | }
| |_^
= note: required because it captures the following types: `ResumeTy`, `impl Future<Output = ()>`, `()`
= note: required because it captures the following types: `&mut Context<'_>`, `impl Future<Output = ()>`, `()`
note: required because it's used within this `async` block
--> $DIR/issue-70935-complex-spans.rs:16:5
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ LL | async fn foo() {
|
= help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `NotSend`
= note: required because it appears within the type `(NotSend,)`
= note: required because it captures the following types: `ResumeTy`, `(NotSend,)`, `()`, `impl Future<Output = ()>`
= note: required because it captures the following types: `&mut Context<'_>`, `(NotSend,)`, `()`, `impl Future<Output = ()>`
note: required because it's used within this `async fn` body
--> $DIR/partial-drop-partial-reinit.rs:31:16
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ LL | async fn foo() {
|
= help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `NotSend`
= note: required because it appears within the type `(NotSend,)`
= note: required because it captures the following types: `ResumeTy`, `(NotSend,)`, `impl Future<Output = ()>`, `()`
= note: required because it captures the following types: `&mut Context<'_>`, `(NotSend,)`, `impl Future<Output = ()>`, `()`
note: required because it's used within this `async fn` body
--> $DIR/partial-drop-partial-reinit.rs:31:16
|
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/regions/closure-in-projection-issue-97405.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ fn good_generic_fn<T>() {
// This should fail because `T` ends up in the upvars of the closure.
fn bad_generic_fn<T: Copy>(t: T) {
assert_static(opaque(async move { t; }).next());
//~^ ERROR the associated type `<impl Iterator as Iterator>::Item` may not live long enough
//~^ ERROR the parameter type `T` may not live long enough
assert_static(opaque(move || { t; }).next());
//~^ ERROR the associated type `<impl Iterator as Iterator>::Item` may not live long enough
assert_static(opaque(opaque(async move { t; }).next()).next());
//~^ ERROR the associated type `<impl Iterator as Iterator>::Item` may not live long enough
//~^ ERROR the parameter type `T` may not live long enough
}

fn main() {}
20 changes: 12 additions & 8 deletions src/test/ui/regions/closure-in-projection-issue-97405.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
error[E0310]: the associated type `<impl Iterator as Iterator>::Item` may not live long enough
error[E0310]: the parameter type `T` may not live long enough
--> $DIR/closure-in-projection-issue-97405.rs:24:5
|
LL | assert_static(opaque(async move { t; }).next());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ...so that the type `T` will meet its required lifetime bounds
|
= help: consider adding an explicit lifetime bound `<impl Iterator as Iterator>::Item: 'static`...
= note: ...so that the type `<impl Iterator as Iterator>::Item` will meet its required lifetime bounds
help: consider adding an explicit lifetime bound...
|
LL | fn bad_generic_fn<T: Copy + 'static>(t: T) {
| +++++++++

error[E0310]: the associated type `<impl Iterator as Iterator>::Item` may not live long enough
--> $DIR/closure-in-projection-issue-97405.rs:26:5
Expand All @@ -16,14 +18,16 @@ LL | assert_static(opaque(move || { t; }).next());
= help: consider adding an explicit lifetime bound `<impl Iterator as Iterator>::Item: 'static`...
= note: ...so that the type `<impl Iterator as Iterator>::Item` will meet its required lifetime bounds

error[E0310]: the associated type `<impl Iterator as Iterator>::Item` may not live long enough
error[E0310]: the parameter type `T` may not live long enough
--> $DIR/closure-in-projection-issue-97405.rs:28:5
|
LL | assert_static(opaque(opaque(async move { t; }).next()).next());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ...so that the type `T` will meet its required lifetime bounds
|
= help: consider adding an explicit lifetime bound `<impl Iterator as Iterator>::Item: 'static`...
= note: ...so that the type `<impl Iterator as Iterator>::Item` will meet its required lifetime bounds
help: consider adding an explicit lifetime bound...
|
LL | fn bad_generic_fn<T: Copy + 'static>(t: T) {
| +++++++++

error: aborting due to 3 previous errors

Expand Down

0 comments on commit 90ccb17

Please sign in to comment.