diff --git a/compiler/rustc_borrowck/src/borrow_set.rs b/compiler/rustc_borrowck/src/borrow_set.rs index c92f94bf3c28c..0260dc333068a 100644 --- a/compiler/rustc_borrowck/src/borrow_set.rs +++ b/compiler/rustc_borrowck/src/borrow_set.rs @@ -217,7 +217,7 @@ impl LocalsStateAtExit { HasStorageDead(DenseBitSet::new_empty(body.local_decls.len())); has_storage_dead.visit_body(body); let mut has_storage_dead_or_moved = has_storage_dead.0; - for move_out in &move_data.moves { + for move_out in &move_data.move_outs { has_storage_dead_or_moved.insert(move_data.base_local(move_out.path)); } LocalsStateAtExit::SomeAreInvalidated { has_storage_dead_or_moved } diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 0e4b29200ff98..9e6510ea33c60 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -129,7 +129,7 @@ impl<'diag, 'tcx> MirBorrowckCtxt<'_, 'diag, 'tcx> { } let is_partial_move = move_site_vec.iter().any(|move_site| { - let move_out = self.move_data.moves[(*move_site).moi]; + let move_out = self.move_data.move_outs[(*move_site).moi]; let moved_place = &self.move_data.move_paths[move_out.path].place; // `*(_1)` where `_1` is a `Box` is actually a move out. let is_box_move = moved_place.as_ref().projection == [ProjectionElem::Deref] @@ -215,7 +215,7 @@ impl<'diag, 'tcx> MirBorrowckCtxt<'_, 'diag, 'tcx> { let mut seen_spans = FxIndexSet::default(); for move_site in &move_site_vec { - let move_out = self.move_data.moves[(*move_site).moi]; + let move_out = self.move_data.move_outs[(*move_site).moi]; let moved_place = &self.move_data.move_paths[move_out.path].place; let move_spans = self.move_spans(moved_place.as_ref(), move_out.source); @@ -281,7 +281,7 @@ impl<'diag, 'tcx> MirBorrowckCtxt<'_, 'diag, 'tcx> { _ => true, }; - let mpi = self.move_data.moves[move_out_indices[0]].path; + let mpi = self.move_data.move_outs[move_out_indices[0]].path; let place = &self.move_data.move_paths[mpi].place; let ty = place.ty(self.body, self.infcx.tcx).ty; @@ -3855,9 +3855,9 @@ impl<'diag, 'tcx> MirBorrowckCtxt<'_, 'diag, 'tcx> { // worry about the other case: that is, if there is a move of a.b.c, it is already // marked as a move of a.b and a as well, so we will generate the correct errors // there. - for moi in &self.move_data.loc_map[location] { + for moi in &self.move_data.move_out_loc_map[location] { debug!("report_use_of_moved_or_uninitialized: moi={:?}", moi); - let path = self.move_data.moves[*moi].path; + let path = self.move_data.move_outs[*moi].path; if mpis.contains(&path) { debug!( "report_use_of_moved_or_uninitialized: found {:?}", diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 1e52c9baebae7..96c99e68f95e8 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -2372,8 +2372,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { // of the union - we should error in that case. let tcx = this.infcx.tcx; if base.ty(this.body(), tcx).ty.is_union() - && this.move_data.path_map[mpi].iter().any(|moi| { - this.move_data.moves[*moi].source.is_predecessor_of(location, this.body) + && this.move_data.move_out_path_map[mpi].iter().any(|moi| { + this.move_data.move_outs[*moi].source.is_predecessor_of(location, this.body) }) { return; diff --git a/compiler/rustc_borrowck/src/polonius/legacy/mod.rs b/compiler/rustc_borrowck/src/polonius/legacy/mod.rs index 05fd6e39476b2..0ae3ff3c04790 100644 --- a/compiler/rustc_borrowck/src/polonius/legacy/mod.rs +++ b/compiler/rustc_borrowck/src/polonius/legacy/mod.rs @@ -132,9 +132,9 @@ fn emit_move_facts( // moved_out_at // deinitialisation is assumed to always happen! - facts - .path_moved_at_base - .extend(move_data.moves.iter().map(|mo| (mo.path, location_table.mid_index(mo.source)))); + facts.path_moved_at_base.extend( + move_data.move_outs.iter().map(|mo| (mo.path, location_table.mid_index(mo.source))), + ); } /// Emit universal regions facts, and their relations. diff --git a/compiler/rustc_borrowck/src/used_muts.rs b/compiler/rustc_borrowck/src/used_muts.rs index e743b0ce6e901..16a9962f1190a 100644 --- a/compiler/rustc_borrowck/src/used_muts.rs +++ b/compiler/rustc_borrowck/src/used_muts.rs @@ -93,8 +93,8 @@ impl<'tcx> Visitor<'tcx> for GatherUsedMutsVisitor<'_, '_, '_, 'tcx> { fn visit_local(&mut self, local: Local, place_context: PlaceContext, location: Location) { if place_context.is_place_assignment() && self.temporary_used_locals.contains(&local) { // Propagate the Local assigned at this Location as a used mutable local variable - for moi in &self.mbcx.move_data.loc_map[location] { - let mpi = &self.mbcx.move_data.moves[*moi].path; + for moi in &self.mbcx.move_data.move_out_loc_map[location] { + let mpi = &self.mbcx.move_data.move_outs[*moi].path; let path = &self.mbcx.move_data.move_paths[*mpi]; debug!( "assignment of {:?} to {:?}, adding {:?} to used mutable set", diff --git a/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs b/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs index 1402a1a8b9136..b6fc1219a8503 100644 --- a/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs +++ b/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs @@ -114,7 +114,7 @@ pub fn drop_flag_effects_for_location<'tcx, F>( debug!("drop_flag_effects_for_location({:?})", loc); // first, move out of the RHS - for mi in &move_data.loc_map[loc] { + for mi in &move_data.move_out_loc_map[loc] { let path = mi.move_path_index(move_data); debug!("moving out of path {:?}", move_data.move_paths[path]); diff --git a/compiler/rustc_mir_dataflow/src/impls/initialized.rs b/compiler/rustc_mir_dataflow/src/impls/initialized.rs index b4cbe7d1da14c..4ca5ee56315a4 100644 --- a/compiler/rustc_mir_dataflow/src/impls/initialized.rs +++ b/compiler/rustc_mir_dataflow/src/impls/initialized.rs @@ -59,25 +59,21 @@ impl<'tcx> MaybePlacesSwitchIntData<'tcx> { { match enum_place.ty(body, tcx).ty.kind() { ty::Adt(enum_def, _) => { - // The value of each discriminant, in AdtDef order. - let discriminant_vals: SmallVec<[u128; 4]> = - enum_def.discriminants(tcx).map(|(_, discr)| discr.val).collect(); - let mut i = 0; - // For each value in the SwitchInt, find the VariantIdx for the variant // with that value. This works because `discriminant_vals` and // `targets.all_values()` are guaranteed to list variants in the same - // order. (If that ever changes we will get out-of-bounds panics here.) + // AdtDef order. (If that ever changes the `expect` will panic.) + let mut discriminants = enum_def.discriminants(tcx); let variants = targets .all_values() .iter() .map(|value| { - loop { - if discriminant_vals[i] == value.get() { - return VariantIdx::new(i); - } - i += 1; - } + // On each call to this closure `find` only consumes part of + // the `discriminants` iterator. + discriminants + .find(|(_, discr)| discr.val == value.get()) + .expect("SwitchInt vals should match a variant") + .0 }) .collect(); @@ -232,7 +228,6 @@ pub struct MaybeUninitializedPlaces<'a, 'tcx> { move_data: &'a MoveData<'tcx>, mark_inactive_variants_as_uninit: bool, - include_inactive_in_otherwise: bool, skip_unreachable_unwind: DenseBitSet, } @@ -243,7 +238,6 @@ impl<'a, 'tcx> MaybeUninitializedPlaces<'a, 'tcx> { body, move_data, mark_inactive_variants_as_uninit: false, - include_inactive_in_otherwise: false, skip_unreachable_unwind: DenseBitSet::new_empty(body.basic_blocks.len()), } } @@ -258,13 +252,6 @@ impl<'a, 'tcx> MaybeUninitializedPlaces<'a, 'tcx> { self } - /// Ensures definitely inactive variants are included in the set of uninitialized places for - /// blocks reached through an `otherwise` edge. - pub fn include_inactive_in_otherwise(mut self) -> Self { - self.include_inactive_in_otherwise = true; - self - } - pub fn skipping_unreachable_unwind( mut self, unreachable_unwind: DenseBitSet, @@ -589,10 +576,7 @@ impl<'tcx> Analysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { SwitchTargetIndex::Normal(target_idx) => { InactiveVariants::Active(data.variants[target_idx]) } - SwitchTargetIndex::Otherwise if self.include_inactive_in_otherwise => { - InactiveVariants::Inactives(data.variants.clone()) - } - _ => return, + SwitchTargetIndex::Otherwise => InactiveVariants::Inactives(data.variants.clone()), }; // Mark all move paths that correspond to variants other than this one as maybe @@ -658,10 +642,9 @@ impl<'tcx> Analysis<'tcx> for EverInitializedPlaces<'_, 'tcx> { terminator: &'mir mir::Terminator<'tcx>, location: Location, ) -> TerminatorEdges<'mir, 'tcx> { - let (body, move_data) = (self.body, self.move_data()); - let term = body[location.block].terminator(); + let move_data = self.move_data(); let init_loc_map = &move_data.init_loc_map; - debug!(?term); + debug!(?terminator); debug!("initializes move_indexes {:?}", init_loc_map[location]); state.gen_all( init_loc_map[location] diff --git a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs index c9198da72c750..74aaa19bf2373 100644 --- a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs +++ b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs @@ -23,7 +23,7 @@ struct MoveDataBuilder<'a, 'tcx, F> { impl<'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> MoveDataBuilder<'a, 'tcx, F> { fn new(body: &'a Body<'tcx>, tcx: TyCtxt<'tcx>, filter: F) -> Self { let mut move_paths = IndexVec::new(); - let mut path_map = IndexVec::new(); + let mut move_out_path_map = IndexVec::new(); let mut init_path_map = IndexVec::new(); let locals = body @@ -36,7 +36,7 @@ impl<'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> MoveDataBuilder<'a, 'tcx, F> { if filter(l.ty) { Some(new_move_path( &mut move_paths, - &mut path_map, + &mut move_out_path_map, &mut init_path_map, None, Place::from(i), @@ -52,15 +52,15 @@ impl<'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> MoveDataBuilder<'a, 'tcx, F> { loc: Location::START, tcx, data: MoveData { - moves: IndexVec::new(), - loc_map: LocationMap::new(body), + move_outs: IndexVec::new(), + move_out_loc_map: LocationMap::new(body), rev_lookup: MovePathLookup { locals, projections: Default::default(), un_derefer: Default::default(), }, move_paths, - path_map, + move_out_path_map, inits: IndexVec::new(), init_loc_map: LocationMap::new(body), init_path_map, @@ -72,7 +72,7 @@ impl<'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> MoveDataBuilder<'a, 'tcx, F> { fn new_move_path<'tcx>( move_paths: &mut IndexVec>, - path_map: &mut IndexVec>, + move_out_path_map: &mut IndexVec>, init_path_map: &mut IndexVec>, parent: Option, place: Place<'tcx>, @@ -85,7 +85,7 @@ fn new_move_path<'tcx>( move_paths[move_path].next_sibling = next_sibling; } - let path_map_ent = path_map.push(smallvec![]); + let path_map_ent = move_out_path_map.push(smallvec![]); assert_eq!(path_map_ent, move_path); let init_path_map_ent = init_path_map.push(smallvec![]); @@ -279,7 +279,7 @@ impl<'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> MoveDataBuilder<'a, 'tcx, F> { base = *data.rev_lookup.projections.entry((base, move_elem)).or_insert_with(|| { new_move_path( &mut data.move_paths, - &mut data.path_map, + &mut data.move_out_path_map, &mut data.init_path_map, Some(base), place_ref.project_deeper(&[elem], tcx), @@ -305,12 +305,12 @@ impl<'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> MoveDataBuilder<'a, 'tcx, F> { mk_place: impl FnOnce(TyCtxt<'tcx>) -> Place<'tcx>, ) -> MovePathIndex { let MoveDataBuilder { - data: MoveData { rev_lookup, move_paths, path_map, init_path_map, .. }, + data: MoveData { rev_lookup, move_paths, move_out_path_map, init_path_map, .. }, tcx, .. } = self; *rev_lookup.projections.entry((base, elem)).or_insert_with(move || { - new_move_path(move_paths, path_map, init_path_map, Some(base), mk_place(*tcx)) + new_move_path(move_paths, move_out_path_map, init_path_map, Some(base), mk_place(*tcx)) }) } @@ -323,7 +323,7 @@ impl<'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> MoveDataBuilder<'a, 'tcx, F> { fn finalize(self) -> MoveData<'tcx> { debug!("{}", { debug!("moves for {:?}:", self.body.span); - for (j, mo) in self.data.moves.iter_enumerated() { + for (j, mo) in self.data.move_outs.iter_enumerated() { debug!(" {:?} = {:?}", j, mo); } debug!("move paths for {:?}:", self.body.span); @@ -553,13 +553,13 @@ impl<'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> MoveDataBuilder<'a, 'tcx, F> { } fn record_move(&mut self, place: Place<'tcx>, path: MovePathIndex) { - let move_out = self.data.moves.push(MoveOut { path, source: self.loc }); + let move_out = self.data.move_outs.push(MoveOut { path, source: self.loc }); debug!( "gather_move({:?}, {:?}): adding move {:?} of {:?}", self.loc, place, move_out, path ); - self.data.path_map[path].push(move_out); - self.data.loc_map[self.loc].push(move_out); + self.data.move_out_path_map[path].push(move_out); + self.data.move_out_loc_map[self.loc].push(move_out); } fn gather_init(&mut self, place: PlaceRef<'tcx>, kind: InitKind) { diff --git a/compiler/rustc_mir_dataflow/src/move_paths/mod.rs b/compiler/rustc_mir_dataflow/src/move_paths/mod.rs index 7f8872b3e3493..f53137948f99c 100644 --- a/compiler/rustc_mir_dataflow/src/move_paths/mod.rs +++ b/compiler/rustc_mir_dataflow/src/move_paths/mod.rs @@ -14,6 +14,7 @@ use smallvec::SmallVec; use crate::un_derefer::UnDerefer; rustc_index::newtype_index! { + /// Index identifying a `MovePath`. #[orderable] #[debug_format = "mp{}"] pub struct MovePathIndex {} @@ -26,26 +27,31 @@ impl polonius_engine::Atom for MovePathIndex { } rustc_index::newtype_index! { + /// Index identifying a `MoveOut`. #[orderable] #[debug_format = "mo{}"] pub struct MoveOutIndex {} } rustc_index::newtype_index! { + /// Index identifying an `Init`. #[debug_format = "in{}"] pub struct InitIndex {} } impl MoveOutIndex { pub fn move_path_index(self, move_data: &MoveData<'_>) -> MovePathIndex { - move_data.moves[self].path + move_data.move_outs[self].path } } -/// `MovePath` is a canonicalized representation of a path that is -/// moved or assigned to. +/// `MovePath` is a canonicalized representation of a place that is of +/// interest to dataflow analysis, as identified by `gather_moves`. This +/// is primarily places that are moved or inited (assigned). Each +/// `MovePath` is assigned a `MovePathIndex` by which it can be referred +/// to. /// -/// It follows a tree structure. +/// `MovePath` follows a tree structure. /// /// Given `struct X { m: M, n: N }` and `x: X`, moves like `drop x.m;` /// move *out* of the place `x.m`. @@ -54,6 +60,8 @@ impl MoveOutIndex { /// one of them will link to the other via the `next_sibling` field, /// and the other will have no entry in its `next_sibling` field), and /// they both have the MovePath representing `x` as their parent. +/// (All tree roots are locals). This structure allows easy traversal +/// between related paths `x` and `x.m` and `x.n`. #[derive(Clone)] pub struct MovePath<'tcx> { pub next_sibling: Option, @@ -166,20 +174,28 @@ where #[derive(Debug)] pub struct MoveData<'tcx> { + /// All the gathered `MovePath`s. pub move_paths: IndexVec>, - pub moves: IndexVec, - /// Each Location `l` is mapped to the MoveOut's that are effects - /// of executing the code at `l`. (There can be multiple MoveOut's - /// for a given `l` because each MoveOut is associated with one - /// particular path being moved.) - pub loc_map: LocationMap>, - pub path_map: IndexVec>, + + /// All the `MoveOut`s. + pub move_outs: IndexVec, + /// Map from locations to `MoveOut`s. `SmallVec` because each location might cause more than + /// one `MoveOut`. Used during analysis and diagnostics. + pub move_out_loc_map: LocationMap>, + /// Map from `MovePath`s (places) to `MoveOuts`. `SmallVec` because each `MovePath` may be + /// moved-out of more than once. Used mostly for diagnostics. + pub move_out_path_map: IndexVec>, + + /// Map from places/locals to `MovePath`s. pub rev_lookup: MovePathLookup<'tcx>, + + /// All the `Init`s. pub inits: IndexVec, - /// Each Location `l` is mapped to the Inits that are effects - /// of executing the code at `l`. Only very rarely (e.g. inline asm) - /// is there more than one Init at any `l`. + /// Map from locations to `Init`s. `SmallVec` because each location might cause more than one + /// `Init`, though more than one is very rare (e.g. inline asm). pub init_loc_map: LocationMap>, + /// Map from `MovePath`s (places) to `Init`s. `SmallVec` because each `MovePath` (place) might + /// be inited more than once. pub init_path_map: IndexVec>, } @@ -223,7 +239,7 @@ where } /// `MoveOut` represents a point in a program that moves out of some -/// L-value; i.e., "creates" uninitialized memory. +/// L-value; i.e., "creates" uninitialized memory. The dual of `Init`. /// /// With respect to dataflow analysis: /// - Generated by moves and declaration of uninitialized variables. @@ -242,7 +258,7 @@ impl fmt::Debug for MoveOut { } } -/// `Init` represents a point in a program that initializes some L-value; +/// `Init` represents a point in a program that initializes some L-value. The dual of `MoveOut`. #[derive(Copy, Clone)] pub struct Init { /// path being initialized @@ -254,7 +270,7 @@ pub struct Init { } /// Initializations can be from an argument or from a statement. Arguments -/// do not have locations, in those cases the `Local` is kept.. +/// do not have locations, in those cases the `Local` is kept. #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum InitLocation { Argument(Local), @@ -287,7 +303,7 @@ impl Init { } } -/// Tables mapping from a place to its MovePathIndex. +/// Tables mapping from a place to its `MovePathIndex`. #[derive(Debug)] pub struct MovePathLookup<'tcx> { locals: IndexVec>, @@ -307,7 +323,13 @@ mod builder; #[derive(Copy, Clone, Debug)] pub enum LookupResult { + /// This exact thing has a move path. E.g. we looked up `x` or `x.m` and it has been moved. Exact(MovePathIndex), + + /// - If the field is `None`, neither the exact thing nor any ancestor of it has a move path. + /// E.g. we looked up `x.m` and neither it nor `x` have a move path. + /// - If the field is `Some`, the exact thing has no move path, but an ancestor does. E.g. we + /// looked up `x.m` which has no move path but `x` has one. Not possible for locals. Parent(Option), } @@ -317,10 +339,12 @@ impl<'tcx> MovePathLookup<'tcx> { // unknown place, but will rather return the nearest available // parent. pub fn find(&self, place: PlaceRef<'tcx>) -> LookupResult { + // Look first in the locals (roots). let Some(mut result) = self.find_local(place.local) else { return LookupResult::Parent(None); }; + // Look for a projection through the found local. for (_, elem) in self.un_derefer.iter_projections(place) { let subpath = match MoveSubPath::of(elem.kind()) { MoveSubPathResult::One(kind) => self.projections.get(&(result, kind)), @@ -338,6 +362,7 @@ impl<'tcx> MovePathLookup<'tcx> { LookupResult::Exact(result) } + /// For locals, which are roots. #[inline] pub fn find_local(&self, local: Local) -> Option { self.locals[local] diff --git a/compiler/rustc_mir_transform/src/elaborate_drops.rs b/compiler/rustc_mir_transform/src/elaborate_drops.rs index 01235b1824218..1fc3f55f8e145 100644 --- a/compiler/rustc_mir_transform/src/elaborate_drops.rs +++ b/compiler/rustc_mir_transform/src/elaborate_drops.rs @@ -68,7 +68,6 @@ impl<'tcx> crate::MirPass<'tcx> for ElaborateDrops { let dead_unwinds = compute_dead_unwinds(body, &mut inits); let uninits = MaybeUninitializedPlaces::new(tcx, body, &env.move_data) - .include_inactive_in_otherwise() .mark_inactive_variants_as_uninit() .skipping_unreachable_unwind(dead_unwinds) .iterate_to_fixpoint(tcx, body, Some("elaborate_drops"))