Skip to content

Commit

Permalink
Stop considering moved-out locals when computing auto traits for gene…
Browse files Browse the repository at this point in the history
…rators
  • Loading branch information
nbdd0121 committed Jun 4, 2023
1 parent c2583e2 commit 9652ea9
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 24 deletions.
71 changes: 54 additions & 17 deletions compiler/rustc_mir_transform/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>>,

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down
25 changes: 25 additions & 0 deletions tests/ui/async-await/temp-borrow-nonsend.drop_tracking.stderr
Original file line number Diff line number Diff line change
@@ -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

25 changes: 25 additions & 0 deletions tests/ui/async-await/temp-borrow-nonsend.no_drop_tracking.stderr
Original file line number Diff line number Diff line change
@@ -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

28 changes: 28 additions & 0 deletions tests/ui/async-await/temp-borrow-nonsend.rs
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 9652ea9

Please sign in to comment.