From d1c9696b7dd791a64486bc20a689952086a3d5cd Mon Sep 17 00:00:00 2001 From: DrMeepster <19316085+DrMeepster@users.noreply.github.com> Date: Tue, 4 Jul 2023 00:39:30 -0700 Subject: [PATCH] bring back un_derefer and rewrite it again --- .../src/type_check/liveness/polonius.rs | 4 +- compiler/rustc_mir_dataflow/src/impls/mod.rs | 8 +- compiler/rustc_mir_dataflow/src/lib.rs | 1 + .../src/move_paths/builder.rs | 117 +++++++++--------- .../rustc_mir_dataflow/src/move_paths/mod.rs | 58 ++------- compiler/rustc_mir_dataflow/src/un_derefer.rs | 36 ++++++ 6 files changed, 112 insertions(+), 112 deletions(-) create mode 100644 compiler/rustc_mir_dataflow/src/un_derefer.rs diff --git a/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs b/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs index b344ab46adb..012075d7114 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs @@ -20,7 +20,7 @@ struct UseFactsExtractor<'me, 'tcx> { } // A Visitor to walk through the MIR and extract point-wise facts -impl UseFactsExtractor<'_, '_> { +impl<'tcx> UseFactsExtractor<'_, 'tcx> { fn location_to_index(&self, location: Location) -> LocationIndex { self.location_table.mid_index(location) } @@ -45,7 +45,7 @@ impl UseFactsExtractor<'_, '_> { self.path_accessed_at_base.push((path, self.location_to_index(location))); } - fn place_to_mpi(&self, place: &Place<'_>) -> Option { + fn place_to_mpi(&self, place: &Place<'tcx>) -> Option { match self.move_data.rev_lookup.find(place.as_ref()) { LookupResult::Exact(mpi) => Some(mpi), LookupResult::Parent(mmpi) => mmpi, diff --git a/compiler/rustc_mir_dataflow/src/impls/mod.rs b/compiler/rustc_mir_dataflow/src/impls/mod.rs index 98cec1c6760..cb74bea724a 100644 --- a/compiler/rustc_mir_dataflow/src/impls/mod.rs +++ b/compiler/rustc_mir_dataflow/src/impls/mod.rs @@ -731,11 +731,11 @@ fn switch_on_enum_discriminant<'mir, 'tcx>( struct OnMutBorrow(F); -impl Visitor<'_> for OnMutBorrow +impl<'tcx, F> Visitor<'tcx> for OnMutBorrow where - F: FnMut(&mir::Place<'_>), + F: FnMut(&mir::Place<'tcx>), { - fn visit_rvalue(&mut self, rvalue: &mir::Rvalue<'_>, location: Location) { + fn visit_rvalue(&mut self, rvalue: &mir::Rvalue<'tcx>, location: Location) { // FIXME: Does `&raw const foo` allow mutation? See #90413. match rvalue { mir::Rvalue::Ref(_, mir::BorrowKind::Mut { .. }, place) @@ -756,7 +756,7 @@ where fn for_each_mut_borrow<'tcx>( mir: &impl MirVisitable<'tcx>, location: Location, - f: impl FnMut(&mir::Place<'_>), + f: impl FnMut(&mir::Place<'tcx>), ) { let mut vis = OnMutBorrow(f); diff --git a/compiler/rustc_mir_dataflow/src/lib.rs b/compiler/rustc_mir_dataflow/src/lib.rs index d43446bc5b2..900d438f8d5 100644 --- a/compiler/rustc_mir_dataflow/src/lib.rs +++ b/compiler/rustc_mir_dataflow/src/lib.rs @@ -43,6 +43,7 @@ pub mod impls; pub mod move_paths; pub mod rustc_peek; pub mod storage; +pub mod un_derefer; pub mod value_analysis; fluent_messages! { "../messages.ftl" } diff --git a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs index dc7e9ab3ce6..537df1588a3 100644 --- a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs +++ b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs @@ -4,7 +4,6 @@ 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; @@ -55,7 +54,7 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { }) .collect(), projections: Default::default(), - derefer_sidetable: Default::default(), + un_derefer: Default::default(), }, move_paths, path_map, @@ -100,11 +99,10 @@ 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> { - let deref_chain = self.builder.data.rev_lookup.deref_chain(place.as_ref()); + let data = &mut self.builder.data; debug!("lookup({:?})", place); - let mut base = - self.builder.data.rev_lookup.find_local(deref_chain.first().unwrap_or(&place).local); + let mut base = data.rev_lookup.find_local(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,55 +111,60 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { // from `*(u.f: &_)` isn't allowed. let mut union_path = None; - 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, - 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)); + for (place_ref, elem) in data.rev_lookup.un_derefer.iter_projections(place.as_ref()) { + 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 { + return Err(MoveError::cannot_move_out_of( + self.loc, + InteriorOfSliceOrArray { ty: place_ty, is_index: true }, + )); + } + } + + _ => {} + }; + + if union_path.is_none() { + // inlined from add_move_path because of a borrowck conflict with the iterator + base = + *data.rev_lookup.projections.entry((base, elem.lift())).or_insert_with(|| { + MoveDataBuilder::new_move_path( + &mut data.move_paths, + &mut data.path_map, + &mut data.init_path_map, + Some(base), + place_ref.project_deeper(&[elem], tcx), + ) + }) } } @@ -282,10 +285,8 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { fn gather_statement(&mut self, stmt: &Statement<'tcx>) { match &stmt.kind { 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.data.rev_lookup.derefer_sidetable.insert(place.local, *reffed); - } + assert!(self.builder.body.local_decls[place.local].is_deref_temp()); + self.builder.data.rev_lookup.un_derefer.insert(place.as_local().unwrap(), *reffed); } StatementKind::Assign(box (place, rval)) => { self.create_move_path(*place); @@ -306,7 +307,7 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { StatementKind::StorageLive(_) => {} StatementKind::StorageDead(local) => { // DerefTemp locals (results of CopyForDeref) don't actually move anything. - if !self.builder.data.rev_lookup.derefer_sidetable.contains_key(&local) { + if !self.builder.body.local_decls[*local].is_deref_temp() { self.gather_move(Place::from(*local)); } } diff --git a/compiler/rustc_mir_dataflow/src/move_paths/mod.rs b/compiler/rustc_mir_dataflow/src/move_paths/mod.rs index aa901f66d3f..0f7da4f46e3 100644 --- a/compiler/rustc_mir_dataflow/src/move_paths/mod.rs +++ b/compiler/rustc_mir_dataflow/src/move_paths/mod.rs @@ -1,4 +1,5 @@ use crate::move_paths::builder::MoveDat; +use crate::un_derefer::UnDerefer; use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; use rustc_index::{IndexSlice, IndexVec}; use rustc_middle::mir::*; @@ -300,8 +301,7 @@ pub struct MovePathLookup<'tcx> { /// elem to the associated MovePathIndex. projections: FxHashMap<(MovePathIndex, AbstractElem), MovePathIndex>, - /// Maps `DerefTemp` locals to the `Place`s assigned to them. - derefer_sidetable: FxHashMap>, + un_derefer: UnDerefer<'tcx>, } mod builder; @@ -317,47 +317,24 @@ impl<'tcx> MovePathLookup<'tcx> { // 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 deref_chain = self.deref_chain(place); + pub fn find(&self, place: PlaceRef<'tcx>) -> LookupResult { + let mut result = self.find_local(place.local); - 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()) { + for (_, elem) in self.un_derefer.iter_projections(place) { + if let Some(&subpath) = self.projections.get(&(result, elem.lift())) { + result = subpath; + } else { 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 { - let deref_chain = self.deref_chain(Place::from(local).as_ref()); + let deref_chain = self.un_derefer.deref_chain(local); - let local = match deref_chain.last() { + let local = match deref_chain.first() { Some(place) => place.local, None => local, }; @@ -374,21 +351,6 @@ impl<'tcx> MovePathLookup<'tcx> { ) -> impl DoubleEndedIterator + ExactSizeIterator + '_ { 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> { - 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 - } } #[derive(Debug)] diff --git a/compiler/rustc_mir_dataflow/src/un_derefer.rs b/compiler/rustc_mir_dataflow/src/un_derefer.rs new file mode 100644 index 00000000000..fdd2eb0233d --- /dev/null +++ b/compiler/rustc_mir_dataflow/src/un_derefer.rs @@ -0,0 +1,36 @@ +use rustc_data_structures::fx::FxHashMap; +use rustc_middle::mir::*; + +use std::iter::once; + +/// Used for reverting changes made by `DerefSeparator` +#[derive(Default, Debug)] +pub struct UnDerefer<'tcx> { + deref_chains: FxHashMap>>, +} + +impl<'tcx> UnDerefer<'tcx> { + pub fn insert(&mut self, local: Local, reffed: Place<'tcx>) { + let mut chain = self.deref_chains.remove(&reffed.local).unwrap_or_default(); + chain.push(reffed); + self.deref_chains.insert(local, chain); + } + + /// Returns the chain of places behind `DerefTemp` locals + pub fn deref_chain(&self, local: Local) -> &[Place<'tcx>] { + self.deref_chains.get(&local).map(Vec::as_slice).unwrap_or_default() + } + + pub fn iter_projections( + &self, + place: PlaceRef<'tcx>, + ) -> impl Iterator, PlaceElem<'tcx>)> + DoubleEndedIterator + '_ { + let deref_chain = self.deref_chain(place.local); + + deref_chain + .iter() + .map(Place::as_ref) + .chain(once(place)) + .flat_map(|place| place.iter_projections()) + } +}