Skip to content

Update MaybeUninitializedPlaces and MaybeInitializedPlaces for otherwise #142707

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

Open
wants to merge 3 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
13 changes: 7 additions & 6 deletions compiler/rustc_mir_dataflow/src/drop_flag_effects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,16 +155,15 @@ where
}
}

/// Calls `handle_inactive_variant` for each descendant move path of `enum_place` that contains a
/// `Downcast` to a variant besides the `active_variant`.
///
/// NOTE: If there are no move paths corresponding to an inactive variant,
/// `handle_inactive_variant` will not be called for that variant.
pub(crate) fn on_all_inactive_variants<'tcx>(
/// Handles each variant that corresponds to one of the child move paths of `enum_place`. If the
/// variant is `active_variant`, `handle_active_variant` will be called (if provided). Otherwise,
/// `handle_inactive_variant` will be called.
pub(crate) fn on_all_variants<'tcx>(
move_data: &MoveData<'tcx>,
enum_place: mir::Place<'tcx>,
active_variant: VariantIdx,
mut handle_inactive_variant: impl FnMut(MovePathIndex),
mut handle_active_variant: Option<impl FnMut(MovePathIndex)>,
) {
let LookupResult::Exact(enum_mpi) = move_data.rev_lookup.find(enum_place.as_ref()) else {
return;
Expand All @@ -184,6 +183,8 @@ pub(crate) fn on_all_inactive_variants<'tcx>(

if variant_idx != active_variant {
on_all_children_bits(move_data, variant_mpi, |mpi| handle_inactive_variant(mpi));
} else if let Some(ref mut handle_active_variant) = handle_active_variant {
on_all_children_bits(move_data, variant_mpi, |mpi| handle_active_variant(mpi));
}
}
}
29 changes: 6 additions & 23 deletions compiler/rustc_mir_dataflow/src/framework/direction.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use std::ops::RangeInclusive;

use rustc_middle::mir::{
self, BasicBlock, CallReturnPlaces, Location, SwitchTargetValue, TerminatorEdges,
};
use rustc_middle::mir::{self, BasicBlock, CallReturnPlaces, Location, TerminatorEdges};

use super::visitor::ResultsVisitor;
use super::{Analysis, Effect, EffectIndex};
Expand Down Expand Up @@ -117,7 +115,7 @@ impl Direction for Backward {
let mut tmp = analysis.bottom_value(body);
for &value in &body.basic_blocks.switch_sources()[&(block, pred)] {
tmp.clone_from(exit_state);
analysis.apply_switch_int_edge_effect(&mut data, &mut tmp, value);
analysis.apply_switch_int_edge_effect(&mut data, &mut tmp, value, None);
propagate(pred, &tmp);
}
} else {
Expand Down Expand Up @@ -245,7 +243,7 @@ impl Direction for Forward {

fn apply_effects_in_block<'mir, 'tcx, A>(
analysis: &mut A,
body: &mir::Body<'tcx>,
_body: &mir::Body<'tcx>,
state: &mut A::Domain,
block: BasicBlock,
block_data: &'mir mir::BasicBlockData<'tcx>,
Expand Down Expand Up @@ -285,25 +283,10 @@ impl Direction for Forward {
}
}
TerminatorEdges::SwitchInt { targets, discr } => {
if let Some(mut data) = analysis.get_switch_int_data(block, discr) {
let mut tmp = analysis.bottom_value(body);
for (value, target) in targets.iter() {
tmp.clone_from(exit_state);
let value = SwitchTargetValue::Normal(value);
analysis.apply_switch_int_edge_effect(&mut data, &mut tmp, value);
propagate(target, &tmp);
}

// Once we get to the final, "otherwise" branch, there is no need to preserve
// `exit_state`, so pass it directly to `apply_switch_int_edge_effect` to save
// a clone of the dataflow state.
let otherwise = targets.otherwise();
analysis.apply_switch_int_edge_effect(
&mut data,
exit_state,
SwitchTargetValue::Otherwise,
if let Some(data) = analysis.get_switch_int_data(block, discr) {
analysis.apply_switch_int_edge_effect_for_targets(
targets, data, exit_state, propagate,
);
propagate(otherwise, exit_state);
} else {
for target in targets.all_targets() {
propagate(*target, exit_state);
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_mir_dataflow/src/framework/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,20 @@ pub trait Analysis<'tcx> {
_data: &mut Self::SwitchIntData,
_state: &mut Self::Domain,
_value: SwitchTargetValue,
_otherwise_state: Option<&mut Self::Domain>,
) {
unreachable!();
}

/// Calls `apply_switch_int_edge_effect` for each target in `targets` and calls `propagate` with
/// the new state. This is used in forward analysis for `MaybeUninitializedPlaces` and
/// `MaybeInitializedPlaces`.
fn apply_switch_int_edge_effect_for_targets(
&mut self,
_targets: &mir::SwitchTargets,
mut _data: Self::SwitchIntData,
_state: &mut Self::Domain,
mut _propagate: impl FnMut(mir::BasicBlock, &Self::Domain),
) {
unreachable!();
}
Expand Down
87 changes: 84 additions & 3 deletions compiler/rustc_mir_dataflow/src/impls/initialized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,26 @@ pub struct MaybeInitializedPlaces<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
body: &'a Body<'tcx>,
move_data: &'a MoveData<'tcx>,
exclude_inactive_in_otherwise: bool,
skip_unreachable_unwind: bool,
}

impl<'a, 'tcx> MaybeInitializedPlaces<'a, 'tcx> {
pub fn new(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, move_data: &'a MoveData<'tcx>) -> Self {
MaybeInitializedPlaces { tcx, body, move_data, skip_unreachable_unwind: false }
MaybeInitializedPlaces {
tcx,
body,
move_data,
exclude_inactive_in_otherwise: false,
skip_unreachable_unwind: false,
}
}

/// Ensures definitely inactive variants are excluded from the set of initialized places for
/// blocks reached through an `otherwise` edge.
pub fn exclude_inactive_in_otherwise(mut self) -> Self {
self.exclude_inactive_in_otherwise = true;
self
}

pub fn skipping_unreachable_unwind(mut self) -> Self {
Expand Down Expand Up @@ -208,6 +222,7 @@ 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 @@ -218,6 +233,7 @@ 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 @@ -232,6 +248,13 @@ 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 @@ -431,17 +454,46 @@ impl<'tcx> Analysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> {
data: &mut Self::SwitchIntData,
state: &mut Self::Domain,
value: SwitchTargetValue,
otherwise_state: Option<&mut Self::Domain>,
) {
if let SwitchTargetValue::Normal(value) = value {
// Kill all move paths that correspond to variants we know to be inactive along this
// particular outgoing edge of a `SwitchInt`.
drop_flag_effects::on_all_inactive_variants(
drop_flag_effects::on_all_variants(
self.move_data,
data.enum_place,
data.next_discr(value),
|mpi| state.kill(mpi),
otherwise_state.map(|state| |mpi| state.kill(mpi)),
);
}
}

fn apply_switch_int_edge_effect_for_targets(
&mut self,
targets: &mir::SwitchTargets,
mut data: Self::SwitchIntData,
state: &mut Self::Domain,
mut propagate: impl FnMut(mir::BasicBlock, &Self::Domain),
) {
let analyze_otherwise = self.exclude_inactive_in_otherwise
&& (1..data.discriminants.len()).contains(&targets.all_values().len());

let mut otherwise_state = if analyze_otherwise { Some(state.clone()) } else { None };
let mut target_state = MaybeReachable::Unreachable;

for (value, target) in targets.iter() {
target_state.clone_from(&state);
self.apply_switch_int_edge_effect(
&mut data,
&mut target_state,
SwitchTargetValue::Normal(value),
otherwise_state.as_mut(),
);
propagate(target, &target_state);
}

propagate(targets.otherwise(), otherwise_state.as_ref().unwrap_or(state));
}
}

Expand Down Expand Up @@ -544,18 +596,47 @@ impl<'tcx> Analysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> {
data: &mut Self::SwitchIntData,
state: &mut Self::Domain,
value: SwitchTargetValue,
otherwise_state: Option<&mut Self::Domain>,
) {
if let SwitchTargetValue::Normal(value) = value {
// Mark all move paths that correspond to variants other than this one as maybe
// uninitialized (in reality, they are *definitely* uninitialized).
drop_flag_effects::on_all_inactive_variants(
drop_flag_effects::on_all_variants(
self.move_data,
data.enum_place,
data.next_discr(value),
|mpi| state.gen_(mpi),
otherwise_state.map(|state| |mpi| state.gen_(mpi)),
);
}
}

fn apply_switch_int_edge_effect_for_targets(
&mut self,
targets: &mir::SwitchTargets,
mut data: Self::SwitchIntData,
state: &mut Self::Domain,
mut propagate: impl FnMut(mir::BasicBlock, &Self::Domain),
) {
let analyze_otherwise = self.include_inactive_in_otherwise
&& (1..data.discriminants.len()).contains(&targets.all_values().len());

let mut otherwise_state = if analyze_otherwise { Some(state.clone()) } else { None };
let mut target_state = MixedBitSet::new_empty(self.move_data().move_paths.len());

for (value, target) in targets.iter() {
target_state.clone_from(&state);
self.apply_switch_int_edge_effect(
&mut data,
&mut target_state,
SwitchTargetValue::Normal(value),
otherwise_state.as_mut(),
);
propagate(target, &target_state);
}

propagate(targets.otherwise(), otherwise_state.as_ref().unwrap_or(state));
}
}

/// There can be many more `InitIndex` than there are locals in a MIR body.
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_transform/src/elaborate_drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ impl<'tcx> crate::MirPass<'tcx> for ElaborateDrops {
let env = MoveDataTypingEnv { move_data, typing_env };

let mut inits = MaybeInitializedPlaces::new(tcx, body, &env.move_data)
.exclude_inactive_in_otherwise()
.skipping_unreachable_unwind()
.iterate_to_fixpoint(tcx, body, Some("elaborate_drops"))
.into_results_cursor(body);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_transform/src/remove_uninit_drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ impl<'tcx> crate::MirPass<'tcx> for RemoveUninitDrops {
let move_data = MoveData::gather_moves(body, tcx, |ty| ty.needs_drop(tcx, typing_env));

let mut maybe_inits = MaybeInitializedPlaces::new(tcx, body, &move_data)
.exclude_inactive_in_otherwise()
.iterate_to_fixpoint(tcx, body, Some("remove_uninit_drops"))
.into_results_cursor(body);

Expand Down
Loading
Loading