From 9652ea915c17496b55ca263a9c7d87e5f007a1fa Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Sun, 4 Jun 2023 16:50:59 +0100 Subject: [PATCH] Stop considering moved-out locals when computing auto traits for generators --- compiler/rustc_mir_transform/src/generator.rs | 71 ++++++++++++++----- ...ld-assign-nonsend.drop_tracking_mir.stderr | 11 ++- .../temp-borrow-nonsend.drop_tracking.stderr | 25 +++++++ ...emp-borrow-nonsend.no_drop_tracking.stderr | 25 +++++++ tests/ui/async-await/temp-borrow-nonsend.rs | 28 ++++++++ 5 files changed, 136 insertions(+), 24 deletions(-) create mode 100644 tests/ui/async-await/temp-borrow-nonsend.drop_tracking.stderr create mode 100644 tests/ui/async-await/temp-borrow-nonsend.no_drop_tracking.stderr create mode 100644 tests/ui/async-await/temp-borrow-nonsend.rs diff --git a/compiler/rustc_mir_transform/src/generator.rs b/compiler/rustc_mir_transform/src/generator.rs index 89567ed0ab882..dc926e93814bd 100644 --- a/compiler/rustc_mir_transform/src/generator.rs +++ b/compiler/rustc_mir_transform/src/generator.rs @@ -67,10 +67,12 @@ use rustc_middle::mir::*; use rustc_middle::ty::{self, AdtDef, Ty, TyCtxt}; use rustc_middle::ty::{GeneratorSubsts, SubstsRef}; use rustc_mir_dataflow::impls::{ - MaybeBorrowedLocals, MaybeLiveLocals, MaybeRequiresStorage, MaybeStorageLive, + MaybeBorrowedLocals, MaybeInitializedPlaces, MaybeLiveLocals, MaybeRequiresStorage, + MaybeStorageLive, }; +use rustc_mir_dataflow::move_paths::MoveData; use rustc_mir_dataflow::storage::always_storage_live_locals; -use rustc_mir_dataflow::{self, Analysis}; +use rustc_mir_dataflow::{self, Analysis, MoveDataParamEnv}; use rustc_span::def_id::{DefId, LocalDefId}; use rustc_span::symbol::sym; use rustc_span::Span; @@ -561,6 +563,10 @@ struct LivenessInfo { /// Which locals are live across any suspension point. saved_locals: GeneratorSavedLocals, + /// Which locals are live *and* initialized across any suspension point. + /// A local that is live but is not initialized does not need to accounted in auto trait checking. + init_locals: BitSet, + /// The set of saved locals live at each suspension point. live_locals_at_suspension_points: Vec>, @@ -615,10 +621,21 @@ fn locals_live_across_suspend_points<'tcx>( .iterate_to_fixpoint() .into_results_cursor(body_ref); + let param_env = tcx.param_env(body.source.def_id()); + let (_, move_data) = MoveData::gather_moves(body, tcx, param_env).unwrap(); + let mdpe = MoveDataParamEnv { move_data, param_env }; + + // Calculate the set of locals which are initialized + let mut inits = MaybeInitializedPlaces::new(tcx, body, &mdpe) + .into_engine(tcx, body) + .iterate_to_fixpoint() + .into_results_cursor(body_ref); + let mut storage_liveness_map = IndexVec::from_elem(None, &body.basic_blocks); let mut live_locals_at_suspension_points = Vec::new(); let mut source_info_at_suspension_points = Vec::new(); let mut live_locals_at_any_suspension_point = BitSet::new_empty(body.local_decls.len()); + let mut init_locals_at_any_suspension_point = BitSet::new_empty(body.local_decls.len()); for (block, data) in body.basic_blocks.iter_enumerated() { if let TerminatorKind::Yield { .. } = data.terminator().kind { @@ -657,12 +674,24 @@ fn locals_live_across_suspend_points<'tcx>( // The generator argument is ignored. live_locals.remove(SELF_ARG); - debug!("loc = {:?}, live_locals = {:?}", loc, live_locals); + inits.seek_to_block_end(block); + let mut init_locals: BitSet<_> = BitSet::new_empty(body.local_decls.len()); + for move_path_index in inits.get().iter() { + if let Some(local) = mdpe.move_data.move_paths[move_path_index].place.as_local() { + init_locals.insert(local); + } + } + init_locals.intersect(&live_locals); + + debug!( + "loc = {:?}, live_locals = {:?}, init_locals = {:?}", + loc, live_locals, init_locals + ); // Add the locals live at this suspension point to the set of locals which live across // any suspension points live_locals_at_any_suspension_point.union(&live_locals); - + init_locals_at_any_suspension_point.union(&init_locals); live_locals_at_suspension_points.push(live_locals); source_info_at_suspension_points.push(data.terminator().source_info); } @@ -687,6 +716,7 @@ fn locals_live_across_suspend_points<'tcx>( LivenessInfo { saved_locals, + init_locals: init_locals_at_any_suspension_point, live_locals_at_suspension_points, source_info_at_suspension_points, storage_conflicts, @@ -909,6 +939,7 @@ fn compute_layout<'tcx>( ) { let LivenessInfo { saved_locals, + init_locals, live_locals_at_suspension_points, source_info_at_suspension_points, storage_conflicts, @@ -926,20 +957,26 @@ fn compute_layout<'tcx>( debug!(?decl); let ignore_for_traits = if tcx.sess.opts.unstable_opts.drop_tracking_mir { - // Do not `assert_crate_local` here, as post-borrowck cleanup may have already cleared - // the information. This is alright, since `ignore_for_traits` is only relevant when - // this code runs on pre-cleanup MIR, and `ignore_for_traits = false` is the safer - // default. - match decl.local_info { - // Do not include raw pointers created from accessing `static` items, as those could - // well be re-created by another access to the same static. - ClearCrossCrate::Set(box LocalInfo::StaticRef { is_thread_local, .. }) => { - !is_thread_local + if !init_locals.contains(local) { + // If only the storage is required to be live, but local is not initialized, then we can + // ignore such type for auto trait purposes. + true + } else { + // Do not `assert_crate_local` here, as post-borrowck cleanup may have already cleared + // the information. This is alright, since `ignore_for_traits` is only relevant when + // this code runs on pre-cleanup MIR, and `ignore_for_traits = false` is the safer + // default. + match decl.local_info { + // Do not include raw pointers created from accessing `static` items, as those could + // well be re-created by another access to the same static. + ClearCrossCrate::Set(box LocalInfo::StaticRef { is_thread_local, .. }) => { + !is_thread_local + } + // Fake borrows are only read by fake reads, so do not have any reality in + // post-analysis MIR. + ClearCrossCrate::Set(box LocalInfo::FakeBorrow) => true, + _ => false, } - // Fake borrows are only read by fake reads, so do not have any reality in - // post-analysis MIR. - ClearCrossCrate::Set(box LocalInfo::FakeBorrow) => true, - _ => false, } } else { // FIXME(#105084) HIR-based drop tracking does not account for all the temporaries that diff --git a/tests/ui/async-await/field-assign-nonsend.drop_tracking_mir.stderr b/tests/ui/async-await/field-assign-nonsend.drop_tracking_mir.stderr index d1df8e91afa2c..bbf9a3fae7ad2 100644 --- a/tests/ui/async-await/field-assign-nonsend.drop_tracking_mir.stderr +++ b/tests/ui/async-await/field-assign-nonsend.drop_tracking_mir.stderr @@ -5,14 +5,11 @@ LL | assert_send(agent.handle()); | ^^^^^^^^^^^^^^ future returned by `handle` is not `Send` | = help: within `impl Future`, the trait `Send` is not implemented for `Rc` -note: future is not `Send` as this value is used across an await - --> $DIR/field-assign-nonsend.rs:23:39 +note: captured value is not `Send` because `&mut` references cannot be sent unless their referent is `Send` + --> $DIR/field-assign-nonsend.rs:19:21 | -LL | let mut info = self.info_result.clone(); - | -------- has type `InfoResult` which is not `Send` -... -LL | let _ = send_element(element).await; - | ^^^^^ await occurs here, with `mut info` maybe used later +LL | async fn handle(&mut self) { + | ^^^^^^^^^ has type `&mut Agent` which is not `Send`, because `Agent` is not `Send` note: required by a bound in `assert_send` --> $DIR/field-assign-nonsend.rs:40:19 | diff --git a/tests/ui/async-await/temp-borrow-nonsend.drop_tracking.stderr b/tests/ui/async-await/temp-borrow-nonsend.drop_tracking.stderr new file mode 100644 index 0000000000000..20b50582aa725 --- /dev/null +++ b/tests/ui/async-await/temp-borrow-nonsend.drop_tracking.stderr @@ -0,0 +1,25 @@ +error: future cannot be sent between threads safely + --> $DIR/temp-borrow-nonsend.rs:25:17 + | +LL | assert_send(test()); + | ^^^^^^ future returned by `test` is not `Send` + | + = help: within `impl Future`, the trait `Send` is not implemented for `*const ()` +note: future is not `Send` as this value is used across an await + --> $DIR/temp-borrow-nonsend.rs:19:11 + | +LL | let b = B(PhantomData); + | - has type `B` which is not `Send` +... +LL | foo().await; + | ^^^^^ await occurs here, with `b` maybe used later +LL | } + | - `b` is later dropped here +note: required by a bound in `assert_send` + --> $DIR/temp-borrow-nonsend.rs:22:19 + | +LL | fn assert_send(_: T) {} + | ^^^^ required by this bound in `assert_send` + +error: aborting due to previous error + diff --git a/tests/ui/async-await/temp-borrow-nonsend.no_drop_tracking.stderr b/tests/ui/async-await/temp-borrow-nonsend.no_drop_tracking.stderr new file mode 100644 index 0000000000000..20b50582aa725 --- /dev/null +++ b/tests/ui/async-await/temp-borrow-nonsend.no_drop_tracking.stderr @@ -0,0 +1,25 @@ +error: future cannot be sent between threads safely + --> $DIR/temp-borrow-nonsend.rs:25:17 + | +LL | assert_send(test()); + | ^^^^^^ future returned by `test` is not `Send` + | + = help: within `impl Future`, the trait `Send` is not implemented for `*const ()` +note: future is not `Send` as this value is used across an await + --> $DIR/temp-borrow-nonsend.rs:19:11 + | +LL | let b = B(PhantomData); + | - has type `B` which is not `Send` +... +LL | foo().await; + | ^^^^^ await occurs here, with `b` maybe used later +LL | } + | - `b` is later dropped here +note: required by a bound in `assert_send` + --> $DIR/temp-borrow-nonsend.rs:22:19 + | +LL | fn assert_send(_: T) {} + | ^^^^ required by this bound in `assert_send` + +error: aborting due to previous error + diff --git a/tests/ui/async-await/temp-borrow-nonsend.rs b/tests/ui/async-await/temp-borrow-nonsend.rs new file mode 100644 index 0000000000000..8f4df0edb8446 --- /dev/null +++ b/tests/ui/async-await/temp-borrow-nonsend.rs @@ -0,0 +1,28 @@ +// revisions: no_drop_tracking drop_tracking drop_tracking_mir +// [drop_tracking_mir] check-pass +// [drop_tracking] compile-flags: -Zdrop-tracking +// [drop_tracking_mir] compile-flags: -Zdrop-tracking-mir +// edition:2021 + +use core::marker::PhantomData; + +struct B(PhantomData<*const ()>); + +fn do_sth(_: &B) {} + +async fn foo() {} + +async fn test() { + let b = B(PhantomData); + do_sth(&b); + drop(b); + foo().await; +} + +fn assert_send(_: T) {} + +fn main() { + assert_send(test()); + //[no_drop_tracking]~^ cannot be sent between threads safely + //[drop_tracking]~^^ cannot be sent between threads safely +}