Skip to content
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

In ConstructCoroutineInClosureShim, pass receiver by mut ref, not mut pointer #123049

Merged
merged 1 commit into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions compiler/rustc_mir_transform/src/shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1015,8 +1015,8 @@ fn build_construct_coroutine_by_move_shim<'tcx>(
bug!();
};

// We use `*mut Self` here because we only need to emit an ABI-compatible shim body,
// rather than match the signature exactly.
// We use `&mut Self` here because we only need to emit an ABI-compatible shim body,
// rather than match the signature exactly (which might take `&self` instead).
//
// The self type here is a coroutine-closure, not a coroutine, and we never read from
// it because it never has any captures, because this is only true in the Fn/FnMut
Expand All @@ -1025,7 +1025,7 @@ fn build_construct_coroutine_by_move_shim<'tcx>(
if receiver_by_ref {
// Triple-check that there's no captures here.
assert_eq!(args.as_coroutine_closure().tupled_upvars_ty(), tcx.types.unit);
self_ty = Ty::new_mut_ptr(tcx, self_ty);
self_ty = Ty::new_mut_ref(tcx, tcx.lifetimes.re_erased, self_ty);
}

let poly_sig = args.as_coroutine_closure().coroutine_closure_sig().map_bound(|sig| {
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_ty_utils/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,12 @@ fn fn_sig_for_fn_abi<'tcx>(
coroutine_kind = ty::ClosureKind::FnOnce;

// Implementations of `FnMut` and `Fn` for coroutine-closures
// still take their receiver by ref.
if receiver_by_ref { Ty::new_mut_ptr(tcx, coroutine_ty) } else { coroutine_ty }
// still take their receiver by (mut) ref.
if receiver_by_ref {
Ty::new_mut_ref(tcx, tcx.lifetimes.re_erased, coroutine_ty)
} else {
coroutine_ty
}
} else {
tcx.closure_env_ty(coroutine_ty, coroutine_kind, env_region)
};
Expand Down
40 changes: 40 additions & 0 deletions src/tools/miri/tests/pass/async-closure-drop.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#![feature(async_closure, noop_waker, async_fn_traits)]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the old test from async-closure.rs -> async-closure-drop.rs and then added a new test that tests the calling at async-closure.rs. I wish git were smarter about renames like that lol.


use std::future::Future;
use std::pin::pin;
use std::task::*;

pub fn block_on<T>(fut: impl Future<Output = T>) -> T {
let mut fut = pin!(fut);
let ctx = &mut Context::from_waker(Waker::noop());

loop {
match fut.as_mut().poll(ctx) {
Poll::Pending => {}
Poll::Ready(t) => break t,
}
}
}

async fn call_once(f: impl async FnOnce(DropMe)) {
f(DropMe("world")).await;
}

#[derive(Debug)]
struct DropMe(&'static str);

impl Drop for DropMe {
fn drop(&mut self) {
println!("{}", self.0);
}
}

pub fn main() {
block_on(async {
let b = DropMe("hello");
let async_closure = async move |a: DropMe| {
println!("{a:?} {b:?}");
};
call_once(async_closure).await;
});
}
3 changes: 3 additions & 0 deletions src/tools/miri/tests/pass/async-closure-drop.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
DropMe("world") DropMe("hello")
world
hello
34 changes: 23 additions & 11 deletions src/tools/miri/tests/pass/async-closure.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![feature(async_closure, noop_waker, async_fn_traits)]

use std::future::Future;
use std::ops::{AsyncFnMut, AsyncFnOnce};
use std::pin::pin;
use std::task::*;

Expand All @@ -16,25 +17,36 @@ pub fn block_on<T>(fut: impl Future<Output = T>) -> T {
}
}

async fn call_once(f: impl async FnOnce(DropMe)) {
f(DropMe("world")).await;
async fn call_mut(f: &mut impl AsyncFnMut(i32)) {
f(0).await;
}

#[derive(Debug)]
struct DropMe(&'static str);
async fn call_once(f: impl AsyncFnOnce(i32)) {
f(1).await;
}

impl Drop for DropMe {
fn drop(&mut self) {
println!("{}", self.0);
}
async fn call_normal<F: Future<Output = ()>>(f: &impl Fn(i32) -> F) {
f(0).await;
}

async fn call_normal_once<F: Future<Output = ()>>(f: impl FnOnce(i32) -> F) {
f(1).await;
}

pub fn main() {
block_on(async {
let b = DropMe("hello");
let async_closure = async move |a: DropMe| {
println!("{a:?} {b:?}");
let b = 2i32;
let mut async_closure = async move |a: i32| {
println!("{a} {b}");
};
call_mut(&mut async_closure).await;
call_once(async_closure).await;

// No-capture closures implement `Fn`.
let async_closure = async move |a: i32| {
println!("{a}");
};
call_normal(&async_closure).await;
call_normal_once(async_closure).await;
});
}
7 changes: 4 additions & 3 deletions src/tools/miri/tests/pass/async-closure.stdout
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
DropMe("world") DropMe("hello")
world
hello
0 2
1 2
0
1
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// MIR for `main::{closure#0}::{closure#1}` 0 coroutine_closure_by_ref

fn main::{closure#0}::{closure#1}(_1: *mut {async closure@$DIR/async_closure_shims.rs:49:29: 49:48}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:49:49: 51:10} {
fn main::{closure#0}::{closure#1}(_1: &mut {async closure@$DIR/async_closure_shims.rs:49:29: 49:48}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:49:49: 51:10} {
let mut _0: {async closure body@$DIR/async_closure_shims.rs:49:49: 51:10};

bb0: {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// MIR for `main::{closure#0}::{closure#1}` 0 coroutine_closure_by_ref

fn main::{closure#0}::{closure#1}(_1: *mut {async closure@$DIR/async_closure_shims.rs:49:29: 49:48}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:49:49: 51:10} {
fn main::{closure#0}::{closure#1}(_1: &mut {async closure@$DIR/async_closure_shims.rs:49:29: 49:48}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:49:49: 51:10} {
let mut _0: {async closure body@$DIR/async_closure_shims.rs:49:49: 51:10};

bb0: {
Expand Down
Loading