Rollup merge of #69644 - ecstatic-morse:unified-dataflow-cleanup, r=e…

…ddyb

Remove framework in `dataflow/mod.rs` in favor of "generic" one

This is the culmination of the work described in rust-lang/compiler-team#202. All dataflow analyses (including the one in `clippy`) have been ported to use the framework in `dataflow/generic`, which can efficiently handle both gen/kill and generic problems. This PR moves the framework in `dataflow/generic` to `dataflow/framework`, and removes the gen/kill framework in `dataflow/mod.rs`.

More comprehensive documentation for the new framework is tracked in rust-lang/rustc-dev-guide#564.

`clippy` will need to change the path it uses to import the dataflow analysis traits.

src/librustc_mir/borrow_check/mod.rs

@@ -29,12 +29,12 @@ use std::mem;
 use std::rc::Rc;
 
 use crate::dataflow;
-use crate::dataflow::generic::{Analysis, BorrowckFlowState as Flows, BorrowckResults};
 use crate::dataflow::indexes::{BorrowIndex, InitIndex, MoveOutIndex, MovePathIndex};
 use crate::dataflow::move_paths::{InitLocation, LookupResult, MoveData, MoveError};
 use crate::dataflow::Borrows;
 use crate::dataflow::EverInitializedPlaces;
 use crate::dataflow::MoveDataParamEnv;
+use crate::dataflow::{Analysis, BorrowckFlowState as Flows, BorrowckResults};
 use crate::dataflow::{MaybeInitializedPlaces, MaybeUninitializedPlaces};
 use crate::transform::MirSource;
 
@@ -298,7 +298,7 @@ fn do_mir_borrowck<'a, 'tcx>(
         mbcx.report_move_errors(errors);
     }
 
-    dataflow::generic::visit_results(
+    dataflow::visit_results(
         &*body,
         traversal::reverse_postorder(&*body).map(|(bb, _)| bb),
         &results,
@@ -509,7 +509,7 @@ crate struct MirBorrowckCtxt<'cx, 'tcx> {
 // 2. loans made in overlapping scopes do not conflict
 // 3. assignments do not affect things loaned out as immutable
 // 4. moves do not affect things loaned out in any way
-impl<'cx, 'tcx> dataflow::generic::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx> {
+impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx> {
     type FlowState = Flows<'cx, 'tcx>;
 
     fn visit_statement(

src/librustc_mir/borrow_check/nll.rs

@@ -21,9 +21,9 @@ use std::str::FromStr;
 use self::mir_util::PassWhere;
 use polonius_engine::{Algorithm, Output};
 
-use crate::dataflow::generic::ResultsCursor;
 use crate::dataflow::move_paths::{InitKind, InitLocation, MoveData};
 use crate::dataflow::MaybeInitializedPlaces;
+use crate::dataflow::ResultsCursor;
 use crate::transform::MirSource;
 use crate::util as mir_util;
 use crate::util::pretty;

src/librustc_mir/borrow_check/type_check/liveness/mod.rs

@@ -3,9 +3,9 @@ use rustc::ty::{RegionVid, TyCtxt};
 use rustc_data_structures::fx::FxHashSet;
 use std::rc::Rc;
 
-use crate::dataflow::generic::ResultsCursor;
 use crate::dataflow::move_paths::MoveData;
 use crate::dataflow::MaybeInitializedPlaces;
+use crate::dataflow::ResultsCursor;
 
 use crate::borrow_check::{
     constraints::OutlivesConstraintSet,

src/librustc_mir/borrow_check/type_check/liveness/trace.rs

@@ -8,10 +8,10 @@ use rustc_trait_selection::traits::query::type_op::outlives::DropckOutlives;
 use rustc_trait_selection::traits::query::type_op::TypeOp;
 use std::rc::Rc;
 
-use crate::dataflow::generic::ResultsCursor;
 use crate::dataflow::indexes::MovePathIndex;
 use crate::dataflow::move_paths::{HasMoveData, MoveData};
 use crate::dataflow::MaybeInitializedPlaces;
+use crate::dataflow::ResultsCursor;
 
 use crate::borrow_check::{
     region_infer::values::{self, PointIndex, RegionValueElements},

src/librustc_mir/borrow_check/type_check/mod.rs

@@ -39,9 +39,9 @@ use rustc_trait_selection::traits::query::type_op::custom::CustomTypeOp;
 use rustc_trait_selection::traits::query::{Fallible, NoSolution};
 use rustc_trait_selection::traits::{self, ObligationCause, PredicateObligations};
 
-use crate::dataflow::generic::ResultsCursor;
 use crate::dataflow::move_paths::MoveData;
 use crate::dataflow::MaybeInitializedPlaces;
+use crate::dataflow::ResultsCursor;
 use crate::transform::promote_consts::should_suggest_const_in_array_repeat_expressions_attribute;
 
 use crate::borrow_check::{

src/librustc_mir/dataflow/generic/mod.rs → src/librustc_mir/dataflow/framework/mod.rs

@@ -1,26 +1,25 @@
 //! A framework that can express both [gen-kill] and generic dataflow problems.
 //!
-//! There is another interface for dataflow in the compiler in `librustc_mir/dataflow/mod.rs`. The
-//! interface in this module will eventually [replace that one][design-meeting].
+//! To actually use this framework, you must implement either the `Analysis` or the
+//! `GenKillAnalysis` trait. If your transfer function can be expressed with only gen/kill
+//! operations, prefer `GenKillAnalysis` since it will run faster while iterating to fixpoint. The
+//! `impls` module contains several examples of gen/kill dataflow analyses.
 //!
-//! To actually use this framework, you must implement either the `Analysis` or the `GenKillAnalysis`
-//! trait. If your transfer function can be expressed with only gen/kill operations, prefer
-//! `GenKillAnalysis` since it will run faster while iterating to fixpoint. Create an `Engine` using
-//! the appropriate constructor and call `iterate_to_fixpoint`. You can use a `ResultsCursor` to
-//! inspect the fixpoint solution to your dataflow problem.
+//! Create an `Engine` for your analysis using the `into_engine` method on the `Analysis` trait,
+//! then call `iterate_to_fixpoint`. From there, you can use a `ResultsCursor` to inspect the
+//! fixpoint solution to your dataflow problem, or implement the `ResultsVisitor` interface and use
+//! `visit_results`. The following example uses the `ResultsCursor` approach.
 //!
 //! ```ignore(cross-crate-imports)
-//! fn do_my_analysis(tcx: TyCtxt<'tcx>, body: &mir::Body<'tcx>, did: DefId) {
-//!     let analysis = MyAnalysis::new();
-//!
-//!     // If `MyAnalysis` implements `GenKillAnalysis`.
-//!     let results = Engine::new_gen_kill(tcx, body, did, analysis).iterate_to_fixpoint();
+//! use rustc_mir::dataflow::Analysis; // Makes `into_engine` available.
 //!
-//!     // If `MyAnalysis` implements `Analysis`.
-//!     // let results = Engine::new_generic(tcx, body, did, analysis).iterate_to_fixpoint();
-//!
-//!     let mut cursor = ResultsCursor::new(body, results);
+//! fn do_my_analysis(tcx: TyCtxt<'tcx>, body: &mir::Body<'tcx>, did: DefId) {
+//!     let analysis = MyAnalysis::new()
+//!         .into_engine(tcx, body, did)
+//!         .iterate_to_fixpoint()
+//!         .into_results_cursor(body);
 //!
+//!     // Print the dataflow state *after* each statement in the start block.
 //!     for (_, statement_index) in body.block_data[START_BLOCK].statements.iter_enumerated() {
 //!         cursor.seek_after(Location { block: START_BLOCK, statement_index });
 //!         let state = cursor.get();
@@ -30,7 +29,6 @@
 //! ```
 //!
 //! [gen-kill]: https://en.wikipedia.org/wiki/Data-flow_analysis#Bit_vector_problems
-//! [design-meeting]https://github.com/rust-lang/compiler-team/issues/202
 
 use std::io;
 
@@ -41,8 +39,6 @@ use rustc_hir::def_id::DefId;
 use rustc_index::bit_set::{BitSet, HybridBitSet};
 use rustc_index::vec::{Idx, IndexVec};
 
-use crate::dataflow::BottomValue;
-
 mod cursor;
 mod engine;
 mod graphviz;
@@ -95,6 +91,47 @@ where
     }
 }
 
+/// Parameterization for the precise form of data flow that is used.
+///
+/// `BottomValue` determines whether the initial entry set for each basic block is empty or full.
+/// This also determines the semantics of the lattice `join` operator used to merge dataflow
+/// results, since dataflow works by starting at the bottom and moving monotonically to a fixed
+/// point.
+///
+/// This means, for propagation across the graph, that you either want to start at all-zeroes and
+/// then use Union as your merge when propagating, or you start at all-ones and then use Intersect
+/// as your merge when propagating.
+pub trait BottomValue {
+    /// Specifies the initial value for each bit in the entry set for each basic block.
+    const BOTTOM_VALUE: bool;
+
+    /// Merges `in_set` into `inout_set`, returning `true` if `inout_set` changed.
+    ///
+    /// It is almost certainly wrong to override this, since it automatically applies
+    /// * `inout_set & in_set` if `BOTTOM_VALUE == true`
+    /// * `inout_set | in_set` if `BOTTOM_VALUE == false`
+    ///
+    /// This means that if a bit is not `BOTTOM_VALUE`, it is propagated into all target blocks.
+    /// For clarity, the above statement again from a different perspective:
+    /// A bit in the block's entry set is `!BOTTOM_VALUE` if *any* predecessor block's bit value is
+    /// `!BOTTOM_VALUE`.
+    ///
+    /// There are situations where you want the opposite behaviour: propagate only if *all*
+    /// predecessor blocks's value is `!BOTTOM_VALUE`.
+    /// E.g. if you want to know whether a bit is *definitely* set at a specific location. This
+    /// means that all code paths leading to the location must have set the bit, instead of any
+    /// code path leading there.
+    ///
+    /// If you want this kind of "definitely set" analysis, you need to
+    /// 1. Invert `BOTTOM_VALUE`
+    /// 2. Reset the `entry_set` in `start_block_effect` to `!BOTTOM_VALUE`
+    /// 3. Override `join` to do the opposite from what it's doing now.
+    #[inline]
+    fn join<T: Idx>(&self, inout_set: &mut BitSet<T>, in_set: &BitSet<T>) -> bool {
+        if !Self::BOTTOM_VALUE { inout_set.union(in_set) } else { inout_set.intersect(in_set) }
+    }
+}
+
 /// Define the domain of a dataflow problem.
 ///
 /// This trait specifies the lattice on which this analysis operates. For now, this must be a

src/librustc_mir/dataflow/impls/borrowed_locals.rs

@@ -1,6 +1,6 @@
 pub use super::*;
 
-use crate::dataflow::generic::{AnalysisDomain, GenKill, GenKillAnalysis};
+use crate::dataflow::{AnalysisDomain, GenKill, GenKillAnalysis};
 use rustc::mir::visit::Visitor;
 use rustc::mir::*;
 use rustc::ty::{ParamEnv, TyCtxt};

src/librustc_mir/dataflow/impls/borrows.rs

@@ -8,8 +8,8 @@ use rustc_index::bit_set::BitSet;
 use crate::borrow_check::{
     places_conflict, BorrowSet, PlaceConflictBias, PlaceExt, RegionInferenceContext, ToRegionVid,
 };
-use crate::dataflow::generic::{self, GenKill};
 use crate::dataflow::BottomValue;
+use crate::dataflow::{self, GenKill};
 
 use std::rc::Rc;
 
@@ -226,7 +226,7 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> {
     }
 }
 
-impl<'tcx> generic::AnalysisDomain<'tcx> for Borrows<'_, 'tcx> {
+impl<'tcx> dataflow::AnalysisDomain<'tcx> for Borrows<'_, 'tcx> {
     type Idx = BorrowIndex;
 
     const NAME: &'static str = "borrows";
@@ -245,7 +245,7 @@ impl<'tcx> generic::AnalysisDomain<'tcx> for Borrows<'_, 'tcx> {
     }
 }
 
-impl<'tcx> generic::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
+impl<'tcx> dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
     fn before_statement_effect(
         &self,
         trans: &mut impl GenKill<Self::Idx>,