From ae9ac0e383b8054ccded79ce26e48a14b485cb3c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 22 Nov 2024 14:39:29 +1100 Subject: [PATCH] Remove the `DefinitelyInitializedPlaces` analysis. Its only use is in the `tests/ui/mir-dataflow/def_inits-1.rs` where it is tested via `rustc_peek_definite_init`. Also, it's probably buggy. It's supposed to be the inverse of `MaybeUninitializedPlaces`, and it mostly is, except that `apply_terminator_effect` is a little different, and `apply_switch_int_edge_effects` is missing. Unlike `MaybeUninitializedPlaces`, which is used extensively in borrow checking, any bugs in `DefinitelyInitializedPlaces` are easy to overlook because it is only used in one small test. This commit removes the analysis. It also removes `rustc_peek_definite_init`, `Dual` and `MeetSemiLattice`, all of which are no longer needed. --- .../rustc_mir_dataflow/src/framework/fmt.rs | 13 -- .../src/framework/lattice.rs | 108 +------------ .../rustc_mir_dataflow/src/framework/mod.rs | 10 -- .../src/impls/initialized.rs | 146 ++---------------- compiler/rustc_mir_dataflow/src/impls/mod.rs | 3 +- compiler/rustc_mir_dataflow/src/rustc_peek.rs | 11 +- compiler/rustc_span/src/symbol.rs | 1 - tests/ui/mir-dataflow/def-inits-1.rs | 51 ------ tests/ui/mir-dataflow/def-inits-1.stderr | 28 ---- 9 files changed, 13 insertions(+), 358 deletions(-) delete mode 100644 tests/ui/mir-dataflow/def-inits-1.rs delete mode 100644 tests/ui/mir-dataflow/def-inits-1.stderr diff --git a/compiler/rustc_mir_dataflow/src/framework/fmt.rs b/compiler/rustc_mir_dataflow/src/framework/fmt.rs index ed540cd148c45..c177d98106f3e 100644 --- a/compiler/rustc_mir_dataflow/src/framework/fmt.rs +++ b/compiler/rustc_mir_dataflow/src/framework/fmt.rs @@ -230,16 +230,3 @@ where write!(f, "{}", ctxt.move_data().move_paths[*self]) } } - -impl DebugWithContext for crate::lattice::Dual -where - T: DebugWithContext, -{ - fn fmt_with(&self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { - (self.0).fmt_with(ctxt, f) - } - - fn fmt_diff_with(&self, old: &Self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { - (self.0).fmt_diff_with(&old.0, ctxt, f) - } -} diff --git a/compiler/rustc_mir_dataflow/src/framework/lattice.rs b/compiler/rustc_mir_dataflow/src/framework/lattice.rs index 4d03ee53b7c00..6d2a7a099a09e 100644 --- a/compiler/rustc_mir_dataflow/src/framework/lattice.rs +++ b/compiler/rustc_mir_dataflow/src/framework/lattice.rs @@ -25,8 +25,8 @@ //! //! ## `PartialOrd` //! -//! Given that they represent partially ordered sets, you may be surprised that [`JoinSemiLattice`] -//! and [`MeetSemiLattice`] do not have [`PartialOrd`] as a supertrait. This +//! Given that it represents a partially ordered set, you may be surprised that [`JoinSemiLattice`] +//! does not have [`PartialOrd`] as a supertrait. This //! is because most standard library types use lexicographic ordering instead of set inclusion for //! their `PartialOrd` impl. Since we do not actually need to compare lattice elements to run a //! dataflow analysis, there's no need for a newtype wrapper with a custom `PartialOrd` impl. The @@ -58,23 +58,6 @@ pub trait JoinSemiLattice: Eq { fn join(&mut self, other: &Self) -> bool; } -/// A [partially ordered set][poset] that has a [greatest lower bound][glb] for any pair of -/// elements in the set. -/// -/// Dataflow analyses only require that their domains implement [`JoinSemiLattice`], not -/// `MeetSemiLattice`. However, types that will be used as dataflow domains should implement both -/// so that they can be used with [`Dual`]. -/// -/// [glb]: https://en.wikipedia.org/wiki/Infimum_and_supremum -/// [poset]: https://en.wikipedia.org/wiki/Partially_ordered_set -pub trait MeetSemiLattice: Eq { - /// Computes the greatest lower bound of two elements, storing the result in `self` and - /// returning `true` if `self` has changed. - /// - /// The lattice meet operator is abbreviated as `∧`. - fn meet(&mut self, other: &Self) -> bool; -} - /// A set that has a "bottom" element, which is less than or equal to any other element. pub trait HasBottom { const BOTTOM: Self; @@ -105,17 +88,6 @@ impl JoinSemiLattice for bool { } } -impl MeetSemiLattice for bool { - fn meet(&mut self, other: &Self) -> bool { - if let (true, false) = (*self, *other) { - *self = false; - return true; - } - - false - } -} - impl HasBottom for bool { const BOTTOM: Self = false; @@ -145,18 +117,6 @@ impl JoinSemiLattice for IndexVec { } } -impl MeetSemiLattice for IndexVec { - fn meet(&mut self, other: &Self) -> bool { - assert_eq!(self.len(), other.len()); - - let mut changed = false; - for (a, b) in iter::zip(self, other) { - changed |= a.meet(b); - } - changed - } -} - /// A `BitSet` represents the lattice formed by the powerset of all possible values of /// the index type `T` ordered by inclusion. Equivalently, it is a tuple of "two-point" lattices, /// one for each possible value of `T`. @@ -166,60 +126,12 @@ impl JoinSemiLattice for BitSet { } } -impl MeetSemiLattice for BitSet { - fn meet(&mut self, other: &Self) -> bool { - self.intersect(other) - } -} - impl JoinSemiLattice for ChunkedBitSet { fn join(&mut self, other: &Self) -> bool { self.union(other) } } -impl MeetSemiLattice for ChunkedBitSet { - fn meet(&mut self, other: &Self) -> bool { - self.intersect(other) - } -} - -/// The counterpart of a given semilattice `T` using the [inverse order]. -/// -/// The dual of a join-semilattice is a meet-semilattice and vice versa. For example, the dual of a -/// powerset has the empty set as its top element and the full set as its bottom element and uses -/// set *intersection* as its join operator. -/// -/// [inverse order]: https://en.wikipedia.org/wiki/Duality_(order_theory) -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub struct Dual(pub T); - -impl BitSetExt for Dual> { - fn contains(&self, elem: T) -> bool { - self.0.contains(elem) - } - - fn union(&mut self, other: &HybridBitSet) { - self.0.union(other); - } - - fn subtract(&mut self, other: &HybridBitSet) { - self.0.subtract(other); - } -} - -impl JoinSemiLattice for Dual { - fn join(&mut self, other: &Self) -> bool { - self.0.meet(&other.0) - } -} - -impl MeetSemiLattice for Dual { - fn meet(&mut self, other: &Self) -> bool { - self.0.join(&other.0) - } -} - /// Extends a type `T` with top and bottom elements to make it a partially ordered set in which no /// value of `T` is comparable with any other. /// @@ -257,22 +169,6 @@ impl JoinSemiLattice for FlatSet { } } -impl MeetSemiLattice for FlatSet { - fn meet(&mut self, other: &Self) -> bool { - let result = match (&*self, other) { - (Self::Bottom, _) | (_, Self::Top) => return false, - (Self::Elem(ref a), Self::Elem(ref b)) if a == b => return false, - - (Self::Top, Self::Elem(ref x)) => Self::Elem(x.clone()), - - _ => Self::Bottom, - }; - - *self = result; - true - } -} - impl HasBottom for FlatSet { const BOTTOM: Self = Self::Bottom; diff --git a/compiler/rustc_mir_dataflow/src/framework/mod.rs b/compiler/rustc_mir_dataflow/src/framework/mod.rs index 244dfe26ad362..359d9280c52a9 100644 --- a/compiler/rustc_mir_dataflow/src/framework/mod.rs +++ b/compiler/rustc_mir_dataflow/src/framework/mod.rs @@ -378,16 +378,6 @@ impl> GenKill for MaybeReachable { } } -impl GenKill for lattice::Dual> { - fn gen_(&mut self, elem: T) { - self.0.insert(elem); - } - - fn kill(&mut self, elem: T) { - self.0.remove(elem); - } -} - // NOTE: DO NOT CHANGE VARIANT ORDER. The derived `Ord` impls rely on the current order. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] enum Effect { diff --git a/compiler/rustc_mir_dataflow/src/impls/initialized.rs b/compiler/rustc_mir_dataflow/src/impls/initialized.rs index 9bb50d1e056bd..2c10d4b1cd352 100644 --- a/compiler/rustc_mir_dataflow/src/impls/initialized.rs +++ b/compiler/rustc_mir_dataflow/src/impls/initialized.rs @@ -12,7 +12,7 @@ use crate::framework::SwitchIntEdgeEffects; use crate::move_paths::{HasMoveData, InitIndex, InitKind, LookupResult, MoveData, MovePathIndex}; use crate::{ Analysis, GenKill, MaybeReachable, drop_flag_effects, drop_flag_effects_for_function_entry, - drop_flag_effects_for_location, lattice, on_all_children_bits, on_lookup_result_bits, + drop_flag_effects_for_location, on_all_children_bits, on_lookup_result_bits, }; /// `MaybeInitializedPlaces` tracks all places that might be @@ -42,10 +42,10 @@ use crate::{ /// } /// ``` /// -/// To determine whether a place *must* be initialized at a -/// particular control-flow point, one can take the set-difference -/// between this data and the data from `MaybeUninitializedPlaces` at the -/// corresponding control-flow point. +/// To determine whether a place is *definitely* initialized at a +/// particular control-flow point, one can take the set-complement +/// of the data from `MaybeUninitializedPlaces` at the corresponding +/// control-flow point. /// /// Similarly, at a given `drop` statement, the set-intersection /// between this data and `MaybeUninitializedPlaces` yields the set of @@ -117,10 +117,10 @@ impl<'a, 'tcx> HasMoveData<'tcx> for MaybeInitializedPlaces<'a, 'tcx> { /// } /// ``` /// -/// To determine whether a place *must* be uninitialized at a -/// particular control-flow point, one can take the set-difference -/// between this data and the data from `MaybeInitializedPlaces` at the -/// corresponding control-flow point. +/// To determine whether a place is *definitely* uninitialized at a +/// particular control-flow point, one can take the set-complement +/// of the data from `MaybeInitializedPlaces` at the corresponding +/// control-flow point. /// /// Similarly, at a given `drop` statement, the set-intersection /// between this data and `MaybeInitializedPlaces` yields the set of @@ -170,57 +170,6 @@ impl<'tcx> HasMoveData<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { } } -/// `DefinitelyInitializedPlaces` tracks all places that are definitely -/// initialized upon reaching a particular point in the control flow -/// for a function. -/// -/// For example, in code like the following, we have corresponding -/// dataflow information shown in the right-hand comments. -/// -/// ```rust -/// struct S; -/// fn foo(pred: bool) { // definite-init: -/// // { } -/// let a = S; let mut b = S; let c; let d; // {a, b } -/// -/// if pred { -/// drop(a); // { b, } -/// b = S; // { b, } -/// -/// } else { -/// drop(b); // {a, } -/// d = S; // {a, d} -/// -/// } // { } -/// -/// c = S; // { c } -/// } -/// ``` -/// -/// To determine whether a place *may* be uninitialized at a -/// particular control-flow point, one can take the set-complement -/// of this data. -/// -/// Similarly, at a given `drop` statement, the set-difference between -/// this data and `MaybeInitializedPlaces` yields the set of places -/// that would require a dynamic drop-flag at that statement. -pub struct DefinitelyInitializedPlaces<'a, 'tcx> { - body: &'a Body<'tcx>, - move_data: &'a MoveData<'tcx>, -} - -impl<'a, 'tcx> DefinitelyInitializedPlaces<'a, 'tcx> { - pub fn new(body: &'a Body<'tcx>, move_data: &'a MoveData<'tcx>) -> Self { - DefinitelyInitializedPlaces { body, move_data } - } -} - -impl<'a, 'tcx> HasMoveData<'tcx> for DefinitelyInitializedPlaces<'a, 'tcx> { - fn move_data(&self) -> &MoveData<'tcx> { - self.move_data - } -} - /// `EverInitializedPlaces` tracks all places that might have ever been /// initialized upon reaching a particular point in the control flow /// for a function, without an intervening `StorageDead`. @@ -293,19 +242,6 @@ impl<'tcx> MaybeUninitializedPlaces<'_, 'tcx> { } } -impl<'a, 'tcx> DefinitelyInitializedPlaces<'a, 'tcx> { - fn update_bits( - trans: &mut >::Domain, - path: MovePathIndex, - state: DropFlagState, - ) { - match state { - DropFlagState::Absent => trans.kill(path), - DropFlagState::Present => trans.gen_(path), - } - } -} - impl<'tcx> Analysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> { /// There can be many more `MovePathIndex` than there are locals in a MIR body. /// We use a chunked bitset to avoid paying too high a memory footprint. @@ -554,70 +490,6 @@ impl<'tcx> Analysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { } } -impl<'a, 'tcx> Analysis<'tcx> for DefinitelyInitializedPlaces<'a, 'tcx> { - /// Use set intersection as the join operator. - type Domain = lattice::Dual>; - - const NAME: &'static str = "definite_init"; - - fn bottom_value(&self, _: &mir::Body<'tcx>) -> Self::Domain { - // bottom = initialized (start_block_effect counters this at outset) - lattice::Dual(BitSet::new_filled(self.move_data().move_paths.len())) - } - - // sets on_entry bits for Arg places - fn initialize_start_block(&self, _: &mir::Body<'tcx>, state: &mut Self::Domain) { - state.0.clear(); - - drop_flag_effects_for_function_entry(self.body, self.move_data, |path, s| { - assert!(s == DropFlagState::Present); - state.0.insert(path); - }); - } - - fn apply_statement_effect( - &mut self, - trans: &mut Self::Domain, - _statement: &mir::Statement<'tcx>, - location: Location, - ) { - drop_flag_effects_for_location(self.body, self.move_data, location, |path, s| { - Self::update_bits(trans, path, s) - }) - } - - fn apply_terminator_effect<'mir>( - &mut self, - trans: &mut Self::Domain, - terminator: &'mir mir::Terminator<'tcx>, - location: Location, - ) -> TerminatorEdges<'mir, 'tcx> { - drop_flag_effects_for_location(self.body, self.move_data, location, |path, s| { - Self::update_bits(trans, path, s) - }); - terminator.edges() - } - - fn apply_call_return_effect( - &mut self, - trans: &mut Self::Domain, - _block: mir::BasicBlock, - return_places: CallReturnPlaces<'_, 'tcx>, - ) { - return_places.for_each(|place| { - // when a call returns successfully, that means we need to set - // the bits for that dest_place to 1 (initialized). - on_lookup_result_bits( - self.move_data(), - self.move_data().rev_lookup.find(place.as_ref()), - |mpi| { - trans.gen_(mpi); - }, - ); - }); - } -} - impl<'tcx> Analysis<'tcx> for EverInitializedPlaces<'_, 'tcx> { /// There can be many more `InitIndex` than there are locals in a MIR body. /// We use a chunked bitset to avoid paying too high a memory footprint. diff --git a/compiler/rustc_mir_dataflow/src/impls/mod.rs b/compiler/rustc_mir_dataflow/src/impls/mod.rs index 9b5bfa9bfa00a..57ded63c9baa2 100644 --- a/compiler/rustc_mir_dataflow/src/impls/mod.rs +++ b/compiler/rustc_mir_dataflow/src/impls/mod.rs @@ -9,8 +9,7 @@ mod storage_liveness; pub use self::borrowed_locals::{MaybeBorrowedLocals, borrowed_locals}; pub use self::initialized::{ - DefinitelyInitializedPlaces, EverInitializedPlaces, MaybeInitializedPlaces, - MaybeUninitializedPlaces, + EverInitializedPlaces, MaybeInitializedPlaces, MaybeUninitializedPlaces, }; pub use self::liveness::{ MaybeLiveLocals, MaybeTransitiveLiveLocals, TransferFunction as LivenessTransferFunction, diff --git a/compiler/rustc_mir_dataflow/src/rustc_peek.rs b/compiler/rustc_mir_dataflow/src/rustc_peek.rs index 99d0ccde1052c..34ef8afdde322 100644 --- a/compiler/rustc_mir_dataflow/src/rustc_peek.rs +++ b/compiler/rustc_mir_dataflow/src/rustc_peek.rs @@ -12,9 +12,7 @@ use crate::errors::{ PeekMustBePlaceOrRefPlace, StopAfterDataFlowEndedCompilation, }; use crate::framework::BitSetExt; -use crate::impls::{ - DefinitelyInitializedPlaces, MaybeInitializedPlaces, MaybeLiveLocals, MaybeUninitializedPlaces, -}; +use crate::impls::{MaybeInitializedPlaces, MaybeLiveLocals, MaybeUninitializedPlaces}; use crate::move_paths::{HasMoveData, LookupResult, MoveData, MovePathIndex}; use crate::{Analysis, JoinSemiLattice, ResultsCursor}; @@ -56,13 +54,6 @@ pub fn sanity_check<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) { sanity_check_via_rustc_peek(tcx, flow_uninits.into_results_cursor(body)); } - if has_rustc_mir_with(tcx, def_id, sym::rustc_peek_definite_init).is_some() { - let flow_def_inits = - DefinitelyInitializedPlaces::new(body, &move_data).iterate_to_fixpoint(tcx, body, None); - - sanity_check_via_rustc_peek(tcx, flow_def_inits.into_results_cursor(body)); - } - if has_rustc_mir_with(tcx, def_id, sym::rustc_peek_liveness).is_some() { let flow_liveness = MaybeLiveLocals.iterate_to_fixpoint(tcx, body, None); diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index a2d9859645ff6..20dd41bb56c39 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1725,7 +1725,6 @@ symbols! { rustc_partition_reused, rustc_pass_by_value, rustc_peek, - rustc_peek_definite_init, rustc_peek_liveness, rustc_peek_maybe_init, rustc_peek_maybe_uninit, diff --git a/tests/ui/mir-dataflow/def-inits-1.rs b/tests/ui/mir-dataflow/def-inits-1.rs deleted file mode 100644 index 30460824a1678..0000000000000 --- a/tests/ui/mir-dataflow/def-inits-1.rs +++ /dev/null @@ -1,51 +0,0 @@ -// General test of maybe_uninits state computed by MIR dataflow. - -#![feature(core_intrinsics, rustc_attrs)] - -use std::intrinsics::rustc_peek; -use std::mem::{drop, replace}; - -struct S(i32); - -#[rustc_mir(rustc_peek_definite_init,stop_after_dataflow)] -fn foo(test: bool, x: &mut S, y: S, mut z: S) -> S { - let ret; - // `ret` starts off uninitialized - rustc_peek(&ret); //~ ERROR rustc_peek: bit not set - - // All function formal parameters start off initialized. - - rustc_peek(&x); - rustc_peek(&y); - rustc_peek(&z); - - ret = if test { - ::std::mem::replace(x, y) - } else { - z = y; - z - }; - - // `z` may be uninitialized here. - rustc_peek(&z); //~ ERROR rustc_peek: bit not set - - // `y` is definitely uninitialized here. - rustc_peek(&y); //~ ERROR rustc_peek: bit not set - - // `x` is still (definitely) initialized (replace above is a reborrow). - rustc_peek(&x); - - ::std::mem::drop(x); - - // `x` is *definitely* uninitialized here - rustc_peek(&x); //~ ERROR rustc_peek: bit not set - - // `ret` is now definitely initialized (via `if` above). - rustc_peek(&ret); - - ret -} -fn main() { - foo(true, &mut S(13), S(14), S(15)); - foo(false, &mut S(13), S(14), S(15)); -} diff --git a/tests/ui/mir-dataflow/def-inits-1.stderr b/tests/ui/mir-dataflow/def-inits-1.stderr deleted file mode 100644 index e2bddb54d9ba8..0000000000000 --- a/tests/ui/mir-dataflow/def-inits-1.stderr +++ /dev/null @@ -1,28 +0,0 @@ -error: rustc_peek: bit not set - --> $DIR/def-inits-1.rs:14:5 - | -LL | rustc_peek(&ret); - | ^^^^^^^^^^^^^^^^ - -error: rustc_peek: bit not set - --> $DIR/def-inits-1.rs:30:5 - | -LL | rustc_peek(&z); - | ^^^^^^^^^^^^^^ - -error: rustc_peek: bit not set - --> $DIR/def-inits-1.rs:33:5 - | -LL | rustc_peek(&y); - | ^^^^^^^^^^^^^^ - -error: rustc_peek: bit not set - --> $DIR/def-inits-1.rs:41:5 - | -LL | rustc_peek(&x); - | ^^^^^^^^^^^^^^ - -error: stop_after_dataflow ended compilation - -error: aborting due to 5 previous errors -