Stop considering moved-out locals when computing auto traits for generators

nbdd0121 · nbdd0121 · commit 9652ea915c17 · 2023-06-04T16:51:14.000+01:00
diff --git 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<Local>,
+
     /// The set of saved locals live at each suspension point.
     live_locals_at_suspension_points: Vec<BitSet<GeneratorSavedLocal>>,
 
@@ -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
@@ -5,14 +5,11 @@ LL |     assert_send(agent.handle());
    |                 ^^^^^^^^^^^^^^ future returned by `handle` is not `Send`
    |
    = help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `Rc<String>`
-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
@@ -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<Output = ()>`, 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: 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
@@ -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<Output = ()>`, 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: 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
@@ -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: Send>(_: T) {}
+
+fn main() {
+    assert_send(test());
+    //[no_drop_tracking]~^ cannot be sent between threads safely
+    //[drop_tracking]~^^ cannot be sent between threads safely
+}

Original file line number	Diff line number	Diff line change
`@@ -5,14 +5,11 @@ LL \| assert_send(agent.handle());`
`5`	`5`	\| ^^^^^^^^^^^^^^ future returned by `handle` is not `Send`
`6`	`6`	`\|`
`7`	`7`	= help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `Rc<String>`
`8`		-note: future is not `Send` as this value is used across an await
`9`		`- --> $DIR/field-assign-nonsend.rs:23:39`
	`8`	+note: captured value is not `Send` because `&mut` references cannot be sent unless their referent is `Send`
	`9`	`+ --> $DIR/field-assign-nonsend.rs:19:21`
`10`	`10`	`\|`
`11`		`-LL \| let mut info = self.info_result.clone();`
`12`		- \| -------- has type `InfoResult` which is not `Send`
`13`		`-...`
`14`		`-LL \| let _ = send_element(element).await;`
`15`		- \| ^^^^^ await occurs here, with `mut info` maybe used later
	`11`	`+LL \| async fn handle(&mut self) {`
	`12`	+ \| ^^^^^^^^^ has type `&mut Agent` which is not `Send`, because `Agent` is not `Send`
`16`	`13`	note: required by a bound in `assert_send`
`17`	`14`	`--> $DIR/field-assign-nonsend.rs:40:19`
`18`	`15`	`\|`