Skip to content

Some place analysis tweaks #159637

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.

Sign up for GitHub

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

Merged
rust-bors merged 4 commits into from
Jul 27, 2026
Merged

Some place analysis tweaks #159637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
87c8b4e
Don't re-derive the terminator
nnethercote Jul 20, 2026
205f590
Clarify `MoveData`
nnethercote Jul 20, 2026
06a0c50
Simplify a discriminants iteration
nnethercote Jul 21, 2026
3fb482f
Remove `MaybeUninitializedPlaces::include_inactive_in_otherwise`
nnethercote Jul 21, 2026
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
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/borrow_set.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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 {:?}",
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_borrowck/src/polonius/legacy/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/used_muts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_dataflow/src/drop_flag_effects.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]);

Expand Down
39 changes: 11 additions & 28 deletions compiler/rustc_mir_dataflow/src/impls/initialized.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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<mir::BasicBlock>,
}

Expand All @@ -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()),
}
}
Expand All @@ -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<mir::BasicBlock>,
Expand Down Expand Up @@ -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()),

@cjgillot cjgillot Jul 26, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the guard removed? And not just changed to be if self.mark_inactive_variants_as_uninit?

View changes since the review

@nnethercote nnethercote Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair question, because the reason is non-obvious: apply_switch_int_edge_effect is only called if get_switch_int_data returns Some. (This happens within Forward::apply_effects_in_block.) And get_switch_int_data returns None if the condition is false. So the _ => return, is unreachable in practice.

};

// Mark all move paths that correspond to variants other than this one as maybe
Expand Down Expand Up @@ -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]
Expand Down
28 changes: 14 additions & 14 deletions compiler/rustc_mir_dataflow/src/move_paths/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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),
Expand All @@ -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,
Expand All @@ -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<MovePathIndex, MovePath<'tcx>>,
path_map: &mut IndexVec<MovePathIndex, SmallVec<[MoveOutIndex; 4]>>,
move_out_path_map: &mut IndexVec<MovePathIndex, SmallVec<[MoveOutIndex; 4]>>,
init_path_map: &mut IndexVec<MovePathIndex, SmallVec<[InitIndex; 4]>>,
parent: Option<MovePathIndex>,
place: Place<'tcx>,
Expand All @@ -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![]);
Expand Down Expand Up @@ -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),
Expand All @@ -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))
})
}

Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
Loading
Loading