From 1dedae4e0a4bb94d2c41675ba29fd59fe894eadc Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Sun, 24 Sep 2023 04:58:50 +0100 Subject: [PATCH 1/3] Remove drop-track-field-assign[-nonsend] test These tests are identical to the ones without drop-track prefix, so remove. --- .../drop-track-field-assign-nonsend.rs | 44 ------------------- .../drop-track-field-assign-nonsend.stderr | 23 ---------- .../ui/async-await/drop-track-field-assign.rs | 43 ------------------ 3 files changed, 110 deletions(-) delete mode 100644 tests/ui/async-await/drop-track-field-assign-nonsend.rs delete mode 100644 tests/ui/async-await/drop-track-field-assign-nonsend.stderr delete mode 100644 tests/ui/async-await/drop-track-field-assign.rs diff --git a/tests/ui/async-await/drop-track-field-assign-nonsend.rs b/tests/ui/async-await/drop-track-field-assign-nonsend.rs deleted file mode 100644 index 7002836ee47ff..0000000000000 --- a/tests/ui/async-await/drop-track-field-assign-nonsend.rs +++ /dev/null @@ -1,44 +0,0 @@ -// Derived from an ICE found in tokio-xmpp during a crater run. -//@ edition:2021 - -#![allow(dead_code)] - -#[derive(Clone)] -struct InfoResult { - node: Option> -} - -struct Agent { - info_result: InfoResult -} - -impl Agent { - async fn handle(&mut self) { - let mut info = self.info_result.clone(); - info.node = None; - let element = parse_info(info); - let _ = send_element(element).await; - } -} - -struct Element { -} - -async fn send_element(_: Element) {} - -fn parse(_: &[u8]) -> Result<(), ()> { - Ok(()) -} - -fn parse_info(_: InfoResult) -> Element { - Element { } -} - -fn assert_send(_: T) {} - -fn main() { - let agent = Agent { info_result: InfoResult { node: None } }; - // FIXME: It would be nice for this to work. See #94067. - assert_send(agent.handle()); - //~^ cannot be sent between threads safely -} diff --git a/tests/ui/async-await/drop-track-field-assign-nonsend.stderr b/tests/ui/async-await/drop-track-field-assign-nonsend.stderr deleted file mode 100644 index ce2cee6ed4735..0000000000000 --- a/tests/ui/async-await/drop-track-field-assign-nonsend.stderr +++ /dev/null @@ -1,23 +0,0 @@ -error: future cannot be sent between threads safely - --> $DIR/drop-track-field-assign-nonsend.rs:42:17 - | -LL | assert_send(agent.handle()); - | ^^^^^^^^^^^^^^ future returned by `handle` is not `Send` - | - = help: within `impl Future`, the trait `Send` is not implemented for `Rc`, which is required by `impl Future: Send` -note: future is not `Send` as this value is used across an await - --> $DIR/drop-track-field-assign-nonsend.rs:20:39 - | -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 -note: required by a bound in `assert_send` - --> $DIR/drop-track-field-assign-nonsend.rs:37:19 - | -LL | fn assert_send(_: T) {} - | ^^^^ required by this bound in `assert_send` - -error: aborting due to 1 previous error - diff --git a/tests/ui/async-await/drop-track-field-assign.rs b/tests/ui/async-await/drop-track-field-assign.rs deleted file mode 100644 index 491f80d062bbb..0000000000000 --- a/tests/ui/async-await/drop-track-field-assign.rs +++ /dev/null @@ -1,43 +0,0 @@ -// Derived from an ICE found in tokio-xmpp during a crater run. -//@ edition:2021 -//@ build-pass - -#![allow(dead_code)] - -#[derive(Clone)] -struct InfoResult { - node: Option -} - -struct Agent { - info_result: InfoResult -} - -impl Agent { - async fn handle(&mut self) { - let mut info = self.info_result.clone(); - info.node = Some("bar".into()); - let element = parse_info(info); - send_element(element).await; - } -} - -struct Element { -} - -async fn send_element(_: Element) {} - -fn parse(_: &[u8]) -> Result<(), ()> { - Ok(()) -} - -fn parse_info(_: InfoResult) -> Element { - Element { } -} - -fn main() { - let mut agent = Agent { - info_result: InfoResult { node: None } - }; - let _ = agent.handle(); -} From dddb7cf86d828299478fe8e11eee029bb9675dbe Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Sun, 4 Jun 2023 16:50:59 +0100 Subject: [PATCH 2/3] Stop considering moved-out locals when computing auto traits for generators --- compiler/rustc_mir_transform/src/coroutine.rs | 78 +++++++++++++++---- .../async-await/field-assign-nonsend.stderr | 13 ++-- tests/ui/async-await/temp-borrow-nonsend.rs | 23 ++++++ 3 files changed, 89 insertions(+), 25 deletions(-) create mode 100644 tests/ui/async-await/temp-borrow-nonsend.rs diff --git a/compiler/rustc_mir_transform/src/coroutine.rs b/compiler/rustc_mir_transform/src/coroutine.rs index 82528109be9ab..aa0293ffe3c6e 100644 --- a/compiler/rustc_mir_transform/src/coroutine.rs +++ b/compiler/rustc_mir_transform/src/coroutine.rs @@ -66,10 +66,12 @@ use rustc_middle::mir::*; use rustc_middle::ty::{self, CoroutineArgs, CoroutineArgsExt, InstanceKind, Ty, TyCtxt}; use rustc_middle::{bug, span_bug}; 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::Analysis; +use rustc_mir_dataflow::{self, Analysis, MaybeReachable}; use rustc_span::def_id::{DefId, LocalDefId}; use rustc_span::symbol::sym; use rustc_span::Span; @@ -725,6 +727,10 @@ struct LivenessInfo { /// Which locals are live across any suspension point. saved_locals: CoroutineSavedLocals, + /// 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>, @@ -784,10 +790,25 @@ fn locals_live_across_suspend_points<'tcx>( .iterate_to_fixpoint() .into_results_cursor(body); + let param_env = tcx.param_env(body.source.def_id()); + let move_data = + MoveData::gather_moves(body, tcx, param_env).unwrap_or_else(|(move_data, _)| { + tcx.sess.delay_span_bug(body.span, "gather_moves failed"); + move_data + }); + 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 { @@ -826,12 +847,27 @@ fn locals_live_across_suspend_points<'tcx>( // The coroutine 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()); + if let MaybeReachable::Reachable(bitset) = inits.get() { + for move_path_index in bitset.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); } @@ -856,6 +892,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, @@ -1030,6 +1067,7 @@ fn compute_layout<'tcx>( ) { let LivenessInfo { saved_locals, + init_locals, live_locals_at_suspension_points, source_info_at_suspension_points, storage_conflicts, @@ -1046,20 +1084,26 @@ fn compute_layout<'tcx>( let decl = &body.local_decls[local]; debug!(?decl); - // 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. - let ignore_for_traits = 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 + let ignore_for_traits = 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, }; let decl = CoroutineSavedTy { ty: decl.ty, source_info: decl.source_info, ignore_for_traits }; diff --git a/tests/ui/async-await/field-assign-nonsend.stderr b/tests/ui/async-await/field-assign-nonsend.stderr index 525a2cc78b493..9fec60618b8e2 100644 --- a/tests/ui/async-await/field-assign-nonsend.stderr +++ b/tests/ui/async-await/field-assign-nonsend.stderr @@ -4,15 +4,12 @@ error: future cannot be sent between threads safely LL | assert_send(agent.handle()); | ^^^^^^^^^^^^^^ future returned by `handle` is not `Send` | - = help: within `impl Future`, the trait `Send` is not implemented for `Rc`, which is required by `impl Future: Send` -note: future is not `Send` as this value is used across an await - --> $DIR/field-assign-nonsend.rs:20:39 + = help: within `impl Future`, the trait `Send` is not implemented for `Rc` +note: captured value is not `Send` because `&mut` references cannot be sent unless their referent is `Send` + --> $DIR/field-assign-nonsend.rs:16: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:37:19 | 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..247626a72d5c4 --- /dev/null +++ b/tests/ui/async-await/temp-borrow-nonsend.rs @@ -0,0 +1,23 @@ +// check-pass +// 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()); +} From 9c519d5cb55f84745a1461c2a4eff7884c56e83d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 8 Aug 2024 15:31:33 -0400 Subject: [PATCH 3/3] Fix and bless --- compiler/rustc_mir_transform/src/coroutine.rs | 14 ++++---------- tests/ui/async-await/field-assign-nonsend.stderr | 2 +- tests/ui/async-await/temp-borrow-nonsend.rs | 4 ++-- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coroutine.rs b/compiler/rustc_mir_transform/src/coroutine.rs index aa0293ffe3c6e..7c98de90b06af 100644 --- a/compiler/rustc_mir_transform/src/coroutine.rs +++ b/compiler/rustc_mir_transform/src/coroutine.rs @@ -791,18 +791,13 @@ fn locals_live_across_suspend_points<'tcx>( .into_results_cursor(body); let param_env = tcx.param_env(body.source.def_id()); - let move_data = - MoveData::gather_moves(body, tcx, param_env).unwrap_or_else(|(move_data, _)| { - tcx.sess.delay_span_bug(body.span, "gather_moves failed"); - move_data - }); - let mdpe = MoveDataParamEnv { move_data, param_env }; + let move_data = MoveData::gather_moves(body, tcx, param_env, |_| true); // Calculate the set of locals which are initialized - let mut inits = MaybeInitializedPlaces::new(tcx, body, &mdpe) + let mut inits = MaybeInitializedPlaces::new(tcx, body, &move_data) .into_engine(tcx, body) .iterate_to_fixpoint() - .into_results_cursor(body_ref); + .into_results_cursor(body); let mut storage_liveness_map = IndexVec::from_elem(None, &body.basic_blocks); let mut live_locals_at_suspension_points = Vec::new(); @@ -851,8 +846,7 @@ fn locals_live_across_suspend_points<'tcx>( let mut init_locals: BitSet<_> = BitSet::new_empty(body.local_decls.len()); if let MaybeReachable::Reachable(bitset) = inits.get() { for move_path_index in bitset.iter() { - if let Some(local) = mdpe.move_data.move_paths[move_path_index].place.as_local() - { + if let Some(local) = move_data.move_paths[move_path_index].place.as_local() { init_locals.insert(local); } } diff --git a/tests/ui/async-await/field-assign-nonsend.stderr b/tests/ui/async-await/field-assign-nonsend.stderr index 9fec60618b8e2..9960c5fcf67a2 100644 --- a/tests/ui/async-await/field-assign-nonsend.stderr +++ b/tests/ui/async-await/field-assign-nonsend.stderr @@ -4,7 +4,7 @@ error: future cannot be sent between threads safely LL | assert_send(agent.handle()); | ^^^^^^^^^^^^^^ future returned by `handle` is not `Send` | - = help: within `impl Future`, the trait `Send` is not implemented for `Rc` + = help: within `impl Future`, the trait `Send` is not implemented for `Rc`, which is required by `impl Future: Send` note: captured value is not `Send` because `&mut` references cannot be sent unless their referent is `Send` --> $DIR/field-assign-nonsend.rs:16:21 | diff --git a/tests/ui/async-await/temp-borrow-nonsend.rs b/tests/ui/async-await/temp-borrow-nonsend.rs index 247626a72d5c4..d5f573347fcec 100644 --- a/tests/ui/async-await/temp-borrow-nonsend.rs +++ b/tests/ui/async-await/temp-borrow-nonsend.rs @@ -1,5 +1,5 @@ -// check-pass -// edition:2021 +//@ check-pass +//@ edition:2021 use core::marker::PhantomData;