diff --git a/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs b/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs index 496a342cf4c55..4aa7fad897b8a 100644 --- a/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs +++ b/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs @@ -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. 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: impl FnMut(MovePathIndex), ) { let LookupResult::Exact(enum_mpi) = move_data.rev_lookup.find(enum_place.as_ref()) else { return; @@ -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 { + on_all_children_bits(move_data, variant_mpi, |mpi| handle_active_variant(mpi)); } } } diff --git a/compiler/rustc_mir_dataflow/src/framework/direction.rs b/compiler/rustc_mir_dataflow/src/framework/direction.rs index e955e38ad10fa..984569f9cf0b8 100644 --- a/compiler/rustc_mir_dataflow/src/framework/direction.rs +++ b/compiler/rustc_mir_dataflow/src/framework/direction.rs @@ -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}; @@ -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 { @@ -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>, @@ -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); diff --git a/compiler/rustc_mir_dataflow/src/framework/mod.rs b/compiler/rustc_mir_dataflow/src/framework/mod.rs index 9cadec100b534..641bc4730ed8d 100644 --- a/compiler/rustc_mir_dataflow/src/framework/mod.rs +++ b/compiler/rustc_mir_dataflow/src/framework/mod.rs @@ -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!(); } diff --git a/compiler/rustc_mir_dataflow/src/impls/initialized.rs b/compiler/rustc_mir_dataflow/src/impls/initialized.rs index 18165b0b9bd08..da36c7a3ae2c8 100644 --- a/compiler/rustc_mir_dataflow/src/impls/initialized.rs +++ b/compiler/rustc_mir_dataflow/src/impls/initialized.rs @@ -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 { @@ -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, } @@ -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()), } } @@ -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, @@ -431,17 +454,63 @@ impl<'tcx> Analysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> { data: &mut Self::SwitchIntData, state: &mut Self::Domain, value: SwitchTargetValue, + otherwise_state: Option<&mut Self::Domain>, + ) { + let SwitchTargetValue::Normal(value) = value else { + return; + }; + + let handle_inactive_variant = |mpi| state.kill(mpi); + + // Kill all move paths that correspond to variants we know to be inactive along this + // particular outgoing edge of a `SwitchInt`. + match otherwise_state { + Some(otherwise_state) => { + drop_flag_effects::on_all_variants( + self.move_data, + data.enum_place, + data.next_discr(value), + handle_inactive_variant, + |mpi| otherwise_state.kill(mpi), + ); + } + None => { + drop_flag_effects::on_all_variants( + self.move_data, + data.enum_place, + data.next_discr(value), + handle_inactive_variant, + |_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), ) { - 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( - self.move_data, - data.enum_place, - data.next_discr(value), - |mpi| state.kill(mpi), + 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)); } } @@ -544,17 +613,63 @@ impl<'tcx> Analysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { data: &mut Self::SwitchIntData, state: &mut Self::Domain, value: SwitchTargetValue, + otherwise_state: Option<&mut Self::Domain>, + ) { + let SwitchTargetValue::Normal(value) = value else { + return; + }; + + let handle_inactive_variant = |mpi| state.gen_(mpi); + + // Mark all move paths that correspond to variants other than this one as maybe + // uninitialized (in reality, they are *definitely* uninitialized). + match otherwise_state { + Some(otherwise_state) => { + drop_flag_effects::on_all_variants( + self.move_data, + data.enum_place, + data.next_discr(value), + handle_inactive_variant, + |mpi| otherwise_state.gen_(mpi), + ); + } + None => { + drop_flag_effects::on_all_variants( + self.move_data, + data.enum_place, + data.next_discr(value), + handle_inactive_variant, + |_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), ) { - 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( - self.move_data, - data.enum_place, - data.next_discr(value), - |mpi| state.gen_(mpi), + 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)); } } diff --git a/compiler/rustc_mir_transform/src/elaborate_drops.rs b/compiler/rustc_mir_transform/src/elaborate_drops.rs index 42c8cb0b906e8..6683f6f05168e 100644 --- a/compiler/rustc_mir_transform/src/elaborate_drops.rs +++ b/compiler/rustc_mir_transform/src/elaborate_drops.rs @@ -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); diff --git a/compiler/rustc_mir_transform/src/remove_uninit_drops.rs b/compiler/rustc_mir_transform/src/remove_uninit_drops.rs index 9044a88295cc4..25d6fa1bab9fd 100644 --- a/compiler/rustc_mir_transform/src/remove_uninit_drops.rs +++ b/compiler/rustc_mir_transform/src/remove_uninit_drops.rs @@ -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); diff --git a/tests/mir-opt/sroa/lifetimes.foo.ScalarReplacementOfAggregates.diff b/tests/mir-opt/sroa/lifetimes.foo.ScalarReplacementOfAggregates.diff index e2d3c6c41b897..a162e6e526261 100644 --- a/tests/mir-opt/sroa/lifetimes.foo.ScalarReplacementOfAggregates.diff +++ b/tests/mir-opt/sroa/lifetimes.foo.ScalarReplacementOfAggregates.diff @@ -24,16 +24,15 @@ let _24: &[core::fmt::rt::Argument<'_>; 2]; let mut _26: &std::boxed::Box; let mut _27: &u32; - let mut _28: bool; + let mut _28: isize; let mut _29: isize; let mut _30: isize; - let mut _31: isize; -+ let _32: std::result::Result, ::Err>; -+ let _33: u32; ++ let _31: std::result::Result, ::Err>; ++ let _32: u32; scope 1 { - debug foo => _1; -+ debug ((foo: Foo).0: std::result::Result, ::Err>) => _32; -+ debug ((foo: Foo).1: u32) => _33; ++ debug ((foo: Foo).0: std::result::Result, ::Err>) => _31; ++ debug ((foo: Foo).1: u32) => _32; let _5: std::result::Result, ::Err>; scope 2 { debug x => _5; @@ -44,12 +43,12 @@ debug x => _8; let _8: std::boxed::Box; let _12: (&std::boxed::Box, &u32); -+ let _34: &std::boxed::Box; -+ let _35: &u32; ++ let _33: &std::boxed::Box; ++ let _34: &u32; scope 5 { - debug args => _12; -+ debug ((args: (&Box, &u32)).0: &std::boxed::Box) => _34; -+ debug ((args: (&Box, &u32)).1: &u32) => _35; ++ debug ((args: (&Box, &u32)).0: &std::boxed::Box) => _33; ++ debug ((args: (&Box, &u32)).1: &u32) => _34; let _15: [core::fmt::rt::Argument<'_>; 2]; scope 6 { debug args => _15; @@ -62,10 +61,9 @@ } bb0: { - _28 = const false; - StorageLive(_1); ++ StorageLive(_31); + StorageLive(_32); -+ StorageLive(_33); + nop; StorageLive(_2); StorageLive(_3); @@ -79,39 +77,37 @@ _2 = Result::, ::Err>::Ok(move _3); StorageDead(_3); - _1 = Foo:: { x: move _2, y: const 7_u32 }; -+ _32 = move _2; -+ _33 = const 7_u32; ++ _31 = move _2; ++ _32 = const 7_u32; + nop; StorageDead(_2); StorageLive(_5); - _28 = const true; - _5 = move (_1.0: std::result::Result, ::Err>); -+ _5 = move _32; ++ _5 = move _31; StorageLive(_6); - _6 = copy (_1.1: u32); -+ _6 = copy _33; ++ _6 = copy _32; _7 = discriminant(_5); switchInt(move _7) -> [0: bb2, otherwise: bb7]; } bb2: { StorageLive(_8); - _28 = const false; _8 = move ((_5 as Ok).0: std::boxed::Box); StorageLive(_9); StorageLive(_10); StorageLive(_11); - StorageLive(_12); ++ StorageLive(_33); + StorageLive(_34); -+ StorageLive(_35); + nop; StorageLive(_13); _13 = &_8; StorageLive(_14); _14 = &_6; - _12 = (move _13, move _14); -+ _34 = move _13; -+ _35 = move _14; ++ _33 = move _13; ++ _34 = move _14; + nop; StorageDead(_14); StorageDead(_13); @@ -119,7 +115,7 @@ StorageLive(_16); StorageLive(_17); - _26 = deref_copy (_12.0: &std::boxed::Box); -+ _26 = deref_copy _34; ++ _26 = deref_copy _33; _17 = &(*_26); _16 = core::fmt::rt::Argument::<'_>::new_display::>(move _17) -> [return: bb3, unwind unreachable]; } @@ -129,7 +125,7 @@ StorageLive(_18); StorageLive(_19); - _27 = deref_copy (_12.1: &u32); -+ _27 = deref_copy _35; ++ _27 = deref_copy _34; _19 = &(*_27); _18 = core::fmt::rt::Argument::<'_>::new_display::(move _19) -> [return: bb4, unwind unreachable]; } @@ -163,8 +159,8 @@ StorageDead(_11); StorageDead(_15); - StorageDead(_12); ++ StorageDead(_33); + StorageDead(_34); -+ StorageDead(_35); + nop; StorageDead(_10); _9 = const (); @@ -185,29 +181,20 @@ bb9: { StorageDead(_6); - _29 = discriminant(_5); - switchInt(move _29) -> [0: bb11, otherwise: bb13]; + _28 = discriminant(_5); + switchInt(move _28) -> [0: bb10, otherwise: bb11]; } bb10: { - _28 = const false; StorageDead(_5); - StorageDead(_1); ++ StorageDead(_31); + StorageDead(_32); -+ StorageDead(_33); + nop; return; } bb11: { - switchInt(copy _28) -> [0: bb10, otherwise: bb12]; - } - - bb12: { - drop(((_5 as Ok).0: std::boxed::Box)) -> [return: bb10, unwind unreachable]; - } - - bb13: { drop(_5) -> [return: bb10, unwind unreachable]; } }