Auto merge of #112882 - DrMeepster:new_un_derefer, r=oli-obk

Rewrite `UnDerefer`

Currently, `UnDerefer` is used by drop elaboration to undo the effects of the `Derefer` pass. However, it just recreates the original places with derefs in the middle of the projection. Because `ProjectionElem::Deref` is intended to be removed completely in the future, this will not work forever.

This PR introduces a `deref_chain` method that returns the places behind `DerefTemp` locals in a place and rewrites the move path code to use this. In the process, `UnDerefer` was merged into `MovePathLookup`. Now that move paths use the same places as in the MIR, the other uses of `UnDerefer` no longer require it.

See #98145
cc `@ouz-a`
r? `@oli-obk`
This commit is contained in:
bors 2023-07-03 02:46:39 +00:00
commit d5a74249c8
8 changed files with 138 additions and 137 deletions

View File

@ -222,7 +222,7 @@ fn do_mir_borrowck<'tcx>(
let (move_data, move_errors): (MoveData<'tcx>, Vec<(Place<'tcx>, MoveError<'tcx>)>) =
match MoveData::gather_moves(&body, tcx, param_env) {
Ok((_, move_data)) => (move_data, Vec::new()),
Ok(move_data) => (move_data, Vec::new()),
Err((move_data, move_errors)) => (move_data, move_errors),
};
let promoted_errors = promoted

View File

@ -43,7 +43,6 @@
pub mod move_paths;
pub mod rustc_peek;
pub mod storage;
pub mod un_derefer;
pub mod value_analysis;
fluent_messages! { "../messages.ftl" }

View File

@ -1,11 +1,10 @@
use crate::move_paths::FxHashMap;
use crate::un_derefer::UnDerefer;
use rustc_index::IndexVec;
use rustc_middle::mir::tcx::RvalueInitializationState;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, TyCtxt};
use smallvec::{smallvec, SmallVec};
use std::iter;
use std::mem;
use super::abs_domain::Lift;
@ -21,7 +20,6 @@ struct MoveDataBuilder<'a, 'tcx> {
param_env: ty::ParamEnv<'tcx>,
data: MoveData<'tcx>,
errors: Vec<(Place<'tcx>, MoveError<'tcx>)>,
un_derefer: UnDerefer<'tcx>,
}
impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
@ -35,25 +33,29 @@ fn new(body: &'a Body<'tcx>, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -
tcx,
param_env,
errors: Vec::new(),
un_derefer: UnDerefer { tcx: tcx, derefer_sidetable: Default::default() },
data: MoveData {
moves: IndexVec::new(),
loc_map: LocationMap::new(body),
rev_lookup: MovePathLookup {
locals: body
.local_decls
.indices()
.map(|i| {
Self::new_move_path(
&mut move_paths,
&mut path_map,
&mut init_path_map,
None,
Place::from(i),
.iter_enumerated()
.filter(|(_, l)| !l.is_deref_temp())
.map(|(i, _)| {
(
i,
Self::new_move_path(
&mut move_paths,
&mut path_map,
&mut init_path_map,
None,
Place::from(i),
),
)
})
.collect(),
projections: Default::default(),
derefer_sidetable: Default::default(),
},
move_paths,
path_map,
@ -98,13 +100,11 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
///
/// Maybe we should have separate "borrowck" and "moveck" modes.
fn move_path_for(&mut self, place: Place<'tcx>) -> Result<MovePathIndex, MoveError<'tcx>> {
if let Some(new_place) = self.builder.un_derefer.derefer(place.as_ref(), self.builder.body)
{
return self.move_path_for(new_place);
}
let deref_chain = self.builder.data.rev_lookup.deref_chain(place.as_ref());
debug!("lookup({:?})", place);
let mut base = self.builder.data.rev_lookup.locals[place.local];
let mut base =
self.builder.data.rev_lookup.find_local(deref_chain.first().unwrap_or(&place).local);
// The move path index of the first union that we find. Once this is
// some we stop creating child move paths, since moves from unions
@ -113,51 +113,55 @@ fn move_path_for(&mut self, place: Place<'tcx>) -> Result<MovePathIndex, MoveErr
// from `*(u.f: &_)` isn't allowed.
let mut union_path = None;
for (place_ref, elem) in place.as_ref().iter_projections() {
let body = self.builder.body;
let tcx = self.builder.tcx;
let place_ty = place_ref.ty(body, tcx).ty;
match place_ty.kind() {
ty::Ref(..) | ty::RawPtr(..) => {
return Err(MoveError::cannot_move_out_of(
self.loc,
BorrowedContent { target_place: place_ref.project_deeper(&[elem], tcx) },
));
}
ty::Adt(adt, _) if adt.has_dtor(tcx) && !adt.is_box() => {
return Err(MoveError::cannot_move_out_of(
self.loc,
InteriorOfTypeWithDestructor { container_ty: place_ty },
));
}
ty::Adt(adt, _) if adt.is_union() => {
union_path.get_or_insert(base);
}
ty::Slice(_) => {
return Err(MoveError::cannot_move_out_of(
self.loc,
InteriorOfSliceOrArray {
ty: place_ty,
is_index: matches!(elem, ProjectionElem::Index(..)),
},
));
}
ty::Array(..) => {
if let ProjectionElem::Index(..) = elem {
for place in deref_chain.into_iter().chain(iter::once(place)) {
for (place_ref, elem) in place.as_ref().iter_projections() {
let body = self.builder.body;
let tcx = self.builder.tcx;
let place_ty = place_ref.ty(body, tcx).ty;
match place_ty.kind() {
ty::Ref(..) | ty::RawPtr(..) => {
return Err(MoveError::cannot_move_out_of(
self.loc,
InteriorOfSliceOrArray { ty: place_ty, is_index: true },
BorrowedContent {
target_place: place_ref.project_deeper(&[elem], tcx),
},
));
}
ty::Adt(adt, _) if adt.has_dtor(tcx) && !adt.is_box() => {
return Err(MoveError::cannot_move_out_of(
self.loc,
InteriorOfTypeWithDestructor { container_ty: place_ty },
));
}
ty::Adt(adt, _) if adt.is_union() => {
union_path.get_or_insert(base);
}
ty::Slice(_) => {
return Err(MoveError::cannot_move_out_of(
self.loc,
InteriorOfSliceOrArray {
ty: place_ty,
is_index: matches!(elem, ProjectionElem::Index(..)),
},
));
}
ty::Array(..) => {
if let ProjectionElem::Index(..) = elem {
return Err(MoveError::cannot_move_out_of(
self.loc,
InteriorOfSliceOrArray { ty: place_ty, is_index: true },
));
}
}
_ => {}
};
if union_path.is_none() {
base = self
.add_move_path(base, elem, |tcx| place_ref.project_deeper(&[elem], tcx));
}
_ => {}
};
if union_path.is_none() {
base = self.add_move_path(base, elem, |tcx| place_ref.project_deeper(&[elem], tcx));
}
}
@ -198,10 +202,8 @@ fn create_move_path(&mut self, place: Place<'tcx>) {
}
}
pub type MoveDat<'tcx> = Result<
(FxHashMap<Local, Place<'tcx>>, MoveData<'tcx>),
(MoveData<'tcx>, Vec<(Place<'tcx>, MoveError<'tcx>)>),
>;
pub type MoveDat<'tcx> =
Result<MoveData<'tcx>, (MoveData<'tcx>, Vec<(Place<'tcx>, MoveError<'tcx>)>)>;
impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
fn finalize(self) -> MoveDat<'tcx> {
@ -217,11 +219,7 @@ fn finalize(self) -> MoveDat<'tcx> {
"done dumping moves"
});
if self.errors.is_empty() {
Ok((self.un_derefer.derefer_sidetable, self.data))
} else {
Err((self.data, self.errors))
}
if self.errors.is_empty() { Ok(self.data) } else { Err((self.data, self.errors)) }
}
}
@ -250,7 +248,7 @@ pub(super) fn gather_moves<'tcx>(
impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
fn gather_args(&mut self) {
for arg in self.body.args_iter() {
let path = self.data.rev_lookup.locals[arg];
let path = self.data.rev_lookup.find_local(arg);
let init = self.data.inits.push(Init {
path,
@ -286,7 +284,7 @@ fn gather_statement(&mut self, stmt: &Statement<'tcx>) {
StatementKind::Assign(box (place, Rvalue::CopyForDeref(reffed))) => {
assert!(place.projection.is_empty());
if self.builder.body.local_decls[place.local].is_deref_temp() {
self.builder.un_derefer.derefer_sidetable.insert(place.local, *reffed);
self.builder.data.rev_lookup.derefer_sidetable.insert(place.local, *reffed);
}
}
StatementKind::Assign(box (place, rval)) => {
@ -308,7 +306,7 @@ fn gather_statement(&mut self, stmt: &Statement<'tcx>) {
StatementKind::StorageLive(_) => {}
StatementKind::StorageDead(local) => {
// DerefTemp locals (results of CopyForDeref) don't actually move anything.
if !self.builder.un_derefer.derefer_sidetable.contains_key(&local) {
if !self.builder.data.rev_lookup.derefer_sidetable.contains_key(&local) {
self.gather_move(Place::from(*local));
}
}
@ -450,12 +448,6 @@ fn gather_operand(&mut self, operand: &Operand<'tcx>) {
fn gather_move(&mut self, place: Place<'tcx>) {
debug!("gather_move({:?}, {:?})", self.loc, place);
if let Some(new_place) = self.builder.un_derefer.derefer(place.as_ref(), self.builder.body)
{
self.gather_move(new_place);
return;
}
if let [ref base @ .., ProjectionElem::Subslice { from, to, from_end: false }] =
**place.projection
{
@ -512,11 +504,6 @@ fn record_move(&mut self, place: Place<'tcx>, path: MovePathIndex) {
fn gather_init(&mut self, place: PlaceRef<'tcx>, kind: InitKind) {
debug!("gather_init({:?}, {:?})", self.loc, place);
if let Some(new_place) = self.builder.un_derefer.derefer(place, self.builder.body) {
self.gather_init(new_place.as_ref(), kind);
return;
}
let mut place = place;
// Check if we are assigning into a field of a union, if so, lookup the place

View File

@ -1,5 +1,5 @@
use crate::move_paths::builder::MoveDat;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
use rustc_index::{IndexSlice, IndexVec};
use rustc_middle::mir::*;
use rustc_middle::ty::{ParamEnv, Ty, TyCtxt};
@ -175,7 +175,7 @@ pub struct MoveData<'tcx> {
/// particular path being moved.)
pub loc_map: LocationMap<SmallVec<[MoveOutIndex; 4]>>,
pub path_map: IndexVec<MovePathIndex, SmallVec<[MoveOutIndex; 4]>>,
pub rev_lookup: MovePathLookup,
pub rev_lookup: MovePathLookup<'tcx>,
pub inits: IndexVec<InitIndex, Init>,
/// Each Location `l` is mapped to the Inits that are effects
/// of executing the code at `l`.
@ -289,8 +289,8 @@ pub fn span<'tcx>(&self, body: &Body<'tcx>) -> Span {
/// Tables mapping from a place to its MovePathIndex.
#[derive(Debug)]
pub struct MovePathLookup {
locals: IndexVec<Local, MovePathIndex>,
pub struct MovePathLookup<'tcx> {
locals: FxIndexMap<Local, MovePathIndex>,
/// projections are made from a base-place and a projection
/// elem. The base-place will have a unique MovePathIndex; we use
@ -299,6 +299,9 @@ pub struct MovePathLookup {
/// base-place). For the remaining lookup, we map the projection
/// elem to the associated MovePathIndex.
projections: FxHashMap<(MovePathIndex, AbstractElem), MovePathIndex>,
/// Maps `DerefTemp` locals to the `Place`s assigned to them.
derefer_sidetable: FxHashMap<Local, Place<'tcx>>,
}
mod builder;
@ -309,27 +312,59 @@ pub enum LookupResult {
Parent(Option<MovePathIndex>),
}
impl MovePathLookup {
impl<'tcx> MovePathLookup<'tcx> {
// Unlike the builder `fn move_path_for` below, this lookup
// alternative will *not* create a MovePath on the fly for an
// unknown place, but will rather return the nearest available
// parent.
pub fn find(&self, place: PlaceRef<'_>) -> LookupResult {
let mut result = self.locals[place.local];
let deref_chain = self.deref_chain(place);
for elem in place.projection.iter() {
if let Some(&subpath) = self.projections.get(&(result, elem.lift())) {
result = subpath;
} else {
let local = match deref_chain.first() {
Some(place) => place.local,
None => place.local,
};
let mut result = *self.locals.get(&local).unwrap_or_else(|| {
bug!("base local ({local:?}) of deref_chain should not be a deref temp")
});
// this needs to be a closure because `place` has a different lifetime than `prefix`'s places
let mut subpaths_for_place = |place: PlaceRef<'_>| {
for elem in place.projection.iter() {
if let Some(&subpath) = self.projections.get(&(result, elem.lift())) {
result = subpath;
} else {
return Some(result);
}
}
None
};
for place in deref_chain {
if let Some(result) = subpaths_for_place(place.as_ref()) {
return LookupResult::Parent(Some(result));
}
}
if let Some(result) = subpaths_for_place(place) {
return LookupResult::Parent(Some(result));
}
LookupResult::Exact(result)
}
pub fn find_local(&self, local: Local) -> MovePathIndex {
self.locals[local]
let deref_chain = self.deref_chain(Place::from(local).as_ref());
let local = match deref_chain.last() {
Some(place) => place.local,
None => local,
};
*self.locals.get(&local).unwrap_or_else(|| {
bug!("base local ({local:?}) of deref_chain should not be a deref temp")
})
}
/// An enumerated iterator of `local`s and their associated
@ -337,7 +372,22 @@ pub fn find_local(&self, local: Local) -> MovePathIndex {
pub fn iter_locals_enumerated(
&self,
) -> impl DoubleEndedIterator<Item = (Local, MovePathIndex)> + ExactSizeIterator + '_ {
self.locals.iter_enumerated().map(|(l, &idx)| (l, idx))
self.locals.iter().map(|(&l, &idx)| (l, idx))
}
/// Returns the chain of places behind `DerefTemp` locals in `place`
pub fn deref_chain(&self, place: PlaceRef<'_>) -> Vec<Place<'tcx>> {
let mut prefix = Vec::new();
let mut local = place.local;
while let Some(&reffed) = self.derefer_sidetable.get(&local) {
prefix.insert(0, reffed);
local = reffed.local;
}
debug!("deref_chain({place:?}) = {prefix:?}");
prefix
}
}

View File

@ -34,7 +34,7 @@ fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
}
let param_env = tcx.param_env(def_id);
let (_, move_data) = MoveData::gather_moves(body, tcx, param_env).unwrap();
let move_data = MoveData::gather_moves(body, tcx, param_env).unwrap();
let mdpe = MoveDataParamEnv { move_data, param_env };
if has_rustc_mir_with(tcx, def_id, sym::rustc_peek_maybe_init).is_some() {

View File

@ -1,22 +0,0 @@
use rustc_data_structures::fx::FxHashMap;
use rustc_middle::mir::*;
use rustc_middle::ty::TyCtxt;
/// Used for reverting changes made by `DerefSeparator`
pub struct UnDerefer<'tcx> {
pub tcx: TyCtxt<'tcx>,
pub derefer_sidetable: FxHashMap<Local, Place<'tcx>>,
}
impl<'tcx> UnDerefer<'tcx> {
#[inline]
pub fn derefer(&self, place: PlaceRef<'tcx>, body: &Body<'tcx>) -> Option<Place<'tcx>> {
let reffed = self.derefer_sidetable.get(&place.local)?;
let new_place = reffed.project_deeper(place.projection, self.tcx);
if body.local_decls[new_place.local].is_deref_temp() {
return self.derefer(new_place.as_ref(), body);
}
Some(new_place)
}
}

View File

@ -10,7 +10,6 @@
use rustc_mir_dataflow::impls::{MaybeInitializedPlaces, MaybeUninitializedPlaces};
use rustc_mir_dataflow::move_paths::{LookupResult, MoveData, MovePathIndex};
use rustc_mir_dataflow::on_lookup_result_bits;
use rustc_mir_dataflow::un_derefer::UnDerefer;
use rustc_mir_dataflow::MoveDataParamEnv;
use rustc_mir_dataflow::{on_all_children_bits, on_all_drop_children_bits};
use rustc_mir_dataflow::{Analysis, ResultsCursor};
@ -54,20 +53,19 @@ fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let def_id = body.source.def_id();
let param_env = tcx.param_env_reveal_all_normalized(def_id);
let (side_table, move_data) = match MoveData::gather_moves(body, tcx, param_env) {
let move_data = match MoveData::gather_moves(body, tcx, param_env) {
Ok(move_data) => move_data,
Err((move_data, _)) => {
tcx.sess.delay_span_bug(
body.span,
"No `move_errors` should be allowed in MIR borrowck",
);
(Default::default(), move_data)
move_data
}
};
let un_derefer = UnDerefer { tcx: tcx, derefer_sidetable: side_table };
let elaborate_patch = {
let env = MoveDataParamEnv { move_data, param_env };
remove_dead_unwinds(tcx, body, &env, &un_derefer);
remove_dead_unwinds(tcx, body, &env);
let inits = MaybeInitializedPlaces::new(tcx, body, &env)
.into_engine(tcx, body)
@ -92,7 +90,6 @@ fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
init_data: InitializationData { inits, uninits },
drop_flags,
patch: MirPatch::new(body),
un_derefer: un_derefer,
reachable,
}
.elaborate()
@ -108,7 +105,6 @@ fn remove_dead_unwinds<'tcx>(
tcx: TyCtxt<'tcx>,
body: &mut Body<'tcx>,
env: &MoveDataParamEnv<'tcx>,
und: &UnDerefer<'tcx>,
) {
debug!("remove_dead_unwinds({:?})", body.span);
// We only need to do this pass once, because unwind edges can only
@ -121,9 +117,7 @@ fn remove_dead_unwinds<'tcx>(
.into_results_cursor(body);
for (bb, bb_data) in body.basic_blocks.iter_enumerated() {
let place = match bb_data.terminator().kind {
TerminatorKind::Drop { ref place, unwind: UnwindAction::Cleanup(_), .. } => {
und.derefer(place.as_ref(), body).unwrap_or(*place)
}
TerminatorKind::Drop { place, unwind: UnwindAction::Cleanup(_), .. } => place,
_ => continue,
};
@ -296,7 +290,6 @@ struct ElaborateDropsCtxt<'a, 'tcx> {
init_data: InitializationData<'a, 'tcx>,
drop_flags: IndexVec<MovePathIndex, Option<Local>>,
patch: MirPatch<'tcx>,
un_derefer: UnDerefer<'tcx>,
reachable: BitSet<BasicBlock>,
}
@ -342,9 +335,7 @@ fn collect_drop_flags(&mut self) {
}
let terminator = data.terminator();
let place = match terminator.kind {
TerminatorKind::Drop { ref place, .. } => {
self.un_derefer.derefer(place.as_ref(), self.body).unwrap_or(*place)
}
TerminatorKind::Drop { ref place, .. } => place,
_ => continue,
};
@ -401,11 +392,7 @@ fn elaborate_drops(&mut self) {
let terminator = data.terminator();
match terminator.kind {
TerminatorKind::Drop { mut place, target, unwind, replace } => {
if let Some(new_place) = self.un_derefer.derefer(place.as_ref(), self.body) {
place = new_place;
}
TerminatorKind::Drop { place, target, unwind, replace } => {
self.init_data.seek_before(loc);
match self.move_data().rev_lookup.find(place.as_ref()) {
LookupResult::Exact(path) => {

View File

@ -22,7 +22,7 @@
impl<'tcx> MirPass<'tcx> for RemoveUninitDrops {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let param_env = tcx.param_env(body.source.def_id());
let Ok((_,move_data)) = MoveData::gather_moves(body, tcx, param_env) else {
let Ok(move_data) = MoveData::gather_moves(body, tcx, param_env) else {
// We could continue if there are move errors, but there's not much point since our
// init data isn't complete.
return;