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

Stop considering moved-out locals when computing auto traits for generators (rebased) #128846

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
44 changes: 39 additions & 5 deletions compiler/rustc_mir_transform/src/coroutine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,11 @@ use rustc_middle::ty::{
};
use rustc_middle::{bug, span_bug};
use rustc_mir_dataflow::impls::{
MaybeBorrowedLocals, MaybeLiveLocals, MaybeRequiresStorage, MaybeStorageLive,
always_storage_live_locals,
MaybeBorrowedLocals, MaybeInitializedPlaces, MaybeLiveLocals, MaybeRequiresStorage,
MaybeStorageLive, always_storage_live_locals,
};
use rustc_mir_dataflow::{Analysis, Results, ResultsVisitor};
use rustc_mir_dataflow::move_paths::MoveData;
use rustc_mir_dataflow::{self, Analysis, MaybeReachable, Results, ResultsVisitor};
use rustc_span::def_id::{DefId, LocalDefId};
use rustc_span::{Span, sym};
use rustc_target::spec::PanicStrategy;
Expand Down Expand Up @@ -633,6 +634,12 @@ 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 (i.e. it has been moved out of) does
/// not need to be accounted for in auto trait checking.
init_locals: DenseBitSet<Local>,

/// The set of saved locals live at each suspension point.
live_locals_at_suspension_points: Vec<DenseBitSet<CoroutineSavedLocal>>,

Expand Down Expand Up @@ -686,10 +693,18 @@ fn locals_live_across_suspend_points<'tcx>(
let mut liveness =
MaybeLiveLocals.iterate_to_fixpoint(tcx, body, Some("coroutine")).into_results_cursor(body);

let move_data = MoveData::gather_moves(body, tcx, |_| true);

// Calculate the set of locals which are initialized
let mut inits = MaybeInitializedPlaces::new(tcx, body, &move_data)
.iterate_to_fixpoint(tcx, body, Some("coroutine"))
.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();
let mut source_info_at_suspension_points = Vec::new();
let mut live_locals_at_any_suspension_point = DenseBitSet::new_empty(body.local_decls.len());
let mut init_locals_at_any_suspension_point = DenseBitSet::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 @@ -727,12 +742,26 @@ 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: DenseBitSet<_> = DenseBitSet::new_empty(body.local_decls.len());
if let MaybeReachable::Reachable(bitset) = inits.get() {
for move_path_index in bitset.iter() {
if let Some(local) = 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 @@ -757,6 +786,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 @@ -929,6 +959,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 @@ -950,6 +981,9 @@ fn compute_layout<'tcx>(
// this code runs on pre-cleanup MIR, and `ignore_for_traits = false` is the safer
// default.
let ignore_for_traits = match decl.local_info {
// If only the storage is required to be live, but local is not initialized,
// then we can ignore such type for auto trait purposes.
_ if !init_locals.contains(local) => true,
// 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, .. }) => {
Expand Down
44 changes: 0 additions & 44 deletions tests/ui/async-await/drop-track-field-assign-nonsend.rs

This file was deleted.

23 changes: 0 additions & 23 deletions tests/ui/async-await/drop-track-field-assign-nonsend.stderr

This file was deleted.

43 changes: 0 additions & 43 deletions tests/ui/async-await/drop-track-field-assign.rs

This file was deleted.

11 changes: 4 additions & 7 deletions tests/ui/async-await/field-assign-nonsend.stderr
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:20:39
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
|
Expand Down
23 changes: 23 additions & 0 deletions tests/ui/async-await/temp-borrow-nonsend.rs
Original file line number Diff line number Diff line change
@@ -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: Send>(_: T) {}

fn main() {
assert_send(test());
}
Loading