Remove use of AllFacts everywhere and divide it into type-safe step…

…-dedicated contexts

polonius-engine/src/output/datafrog_opt.rs

@@ -21,12 +21,10 @@ pub(super) fn compute<T: FactTypes>(
     let timer = Instant::now();
 
     let errors = {
-        let all_facts = &ctx.all_facts;
-
         // Static inputs
+        let region_live_at_rel = &ctx.region_live_at;
         let cfg_edge_rel = &ctx.cfg_edge;
         let killed_rel = &ctx.killed;
-        let region_live_at_rel = &ctx.region_live_at;
 
         // Create a new iteration context, ...
         let mut iteration = Iteration::new();
@@ -130,16 +128,14 @@ pub(super) fn compute<T: FactTypes>(
 
         // Make "variable" versions of the relations, needed for joins.
         borrow_region_op.extend(
-            all_facts
-                .borrow_region
+            ctx.borrow_region
                 .iter()
                 .map(|&(origin, loan, point)| ((origin, point), loan)),
         );
         invalidates.extend(
-            all_facts
-                .invalidates
+            ctx.invalidates
                 .iter()
-                .map(|&(point, loan)| ((loan, point), ())),
+                .map(|&(loan, point)| ((loan, point), ())),
         );
         region_live_at_var.extend(
             region_live_at_rel
@@ -149,16 +145,14 @@ pub(super) fn compute<T: FactTypes>(
 
         // subset(origin1, origin2, point) :- outlives(origin1, origin2, point).
         subset_o1p.extend(
-            all_facts
-                .outlives
+            ctx.outlives
                 .iter()
                 .map(|&(origin1, origin2, point)| ((origin1, point), origin2)),
         );
 
         // requires(origin, loan, point) :- borrow_region(origin, loan, point).
         requires_op.extend(
-            all_facts
-                .borrow_region
+            ctx.borrow_region
                 .iter()
                 .map(|&(origin, loan, point)| ((origin, point), loan)),
         );

polonius-engine/src/output/initialization.rs

@@ -1,16 +1,12 @@
 use std::time::Instant;
 
-use crate::output::Output;
-use facts::FactTypes;
+use crate::facts::FactTypes;
+use crate::output::{InitializationContext, Output};
 
 use datafrog::{Iteration, Relation, RelationLeaper};
 
 pub(super) fn init_var_maybe_initialized_on_exit<T: FactTypes>(
-    child: Vec<(T::Path, T::Path)>,
-    path_belongs_to_var: Vec<(T::Path, T::Variable)>,
-    initialized_at: Vec<(T::Path, T::Point)>,
-    moved_out_at: Vec<(T::Path, T::Point)>,
-    path_accessed_at: Vec<(T::Path, T::Point)>,
+    ctx: InitializationContext<T>,
     cfg_edge: &Relation<(T::Point, T::Point)>,
     output: &mut Output<T>,
 ) -> Relation<(T::Variable, T::Point)> {
@@ -19,11 +15,11 @@ pub(super) fn init_var_maybe_initialized_on_exit<T: FactTypes>(
 
     // Relations
     //let parent: Relation<(Path, Path)> = child.iter().map(|&(child_path, parent_path)| (parent_path, child_path)).collect();
-    let child: Relation<(T::Path, T::Path)> = child.into();
-    let path_belongs_to_var: Relation<(T::Path, T::Variable)> = path_belongs_to_var.into();
-    let initialized_at: Relation<(T::Path, T::Point)> = initialized_at.into();
-    let moved_out_at: Relation<(T::Path, T::Point)> = moved_out_at.into();
-    let _path_accessed_at: Relation<(T::Path, T::Point)> = path_accessed_at.into();
+    let child: Relation<(T::Path, T::Path)> = ctx.child.into();
+    let path_belongs_to_var: Relation<(T::Path, T::Variable)> = ctx.path_belongs_to_var.into();
+    let initialized_at: Relation<(T::Path, T::Point)> = ctx.initialized_at.into();
+    let moved_out_at: Relation<(T::Path, T::Point)> = ctx.moved_out_at.into();
+    let _path_accessed_at: Relation<(T::Path, T::Point)> = ctx.path_accessed_at.into();
 
     // Variables
 

polonius-engine/src/output/liveness.rs

@@ -13,17 +13,13 @@
 use std::collections::BTreeSet;
 use std::time::Instant;
 
-use crate::output::Output;
-use facts::FactTypes;
+use crate::facts::FactTypes;
+use crate::output::{LivenessContext, Output};
 
 use datafrog::{Iteration, Relation, RelationLeaper};
 
 pub(super) fn compute_live_regions<T: FactTypes>(
-    var_used: Vec<(T::Variable, T::Point)>,
-    var_drop_used: Vec<(T::Variable, T::Point)>,
-    var_defined: Vec<(T::Variable, T::Point)>,
-    var_uses_region: Vec<(T::Variable, T::Origin)>,
-    var_drops_region: Vec<(T::Variable, T::Origin)>,
+    ctx: LivenessContext<T>,
     cfg_edge_rel: &Relation<(T::Point, T::Point)>,
     var_maybe_initialized_on_exit_rel: Relation<(T::Variable, T::Point)>,
     output: &mut Output<T>,
@@ -32,14 +28,15 @@ pub(super) fn compute_live_regions<T: FactTypes>(
     let mut iteration = Iteration::new();
 
     // Relations
-    let var_defined_rel: Relation<(T::Variable, T::Point)> = var_defined.into();
+    let var_defined_rel: Relation<(T::Variable, T::Point)> = ctx.var_defined.into();
     let cfg_edge_reverse_rel: Relation<(T::Point, T::Point)> = cfg_edge_rel
         .iter()
         .map(|&(point1, point2)| (point2, point1))
         .collect();
-    let var_uses_region_rel: Relation<(T::Variable, T::Origin)> = var_uses_region.into();
-    let var_drops_region_rel: Relation<(T::Variable, T::Origin)> = var_drops_region.into();
-    let var_drop_used_rel: Relation<((T::Variable, T::Point), ())> = var_drop_used
+    let var_uses_region_rel: Relation<(T::Variable, T::Origin)> = ctx.var_uses_region.into();
+    let var_drops_region_rel: Relation<(T::Variable, T::Origin)> = ctx.var_drops_region.into();
+    let var_drop_used_rel: Relation<((T::Variable, T::Point), ())> = ctx
+        .var_drop_used
         .into_iter()
         .map(|(var, point)| ((var, point), ()))
         .collect();
@@ -55,7 +52,7 @@ pub(super) fn compute_live_regions<T: FactTypes>(
     let region_live_at_var = iteration.variable::<(T::Origin, T::Point)>("region_live_at");
 
     // This propagates the relation `var_live(var, point) :- var_used(var, point)`:
-    var_live_var.insert(var_used.into());
+    var_live_var.insert(ctx.var_used.into());
 
     // var_maybe_initialized_on_entry(var, point2) :-
     //     var_maybe_initialized_on_exit(var, point1),
@@ -173,31 +170,3 @@ pub(super) fn make_universal_regions_live<T: FactTypes>(
         }
     }
 }
-
-pub(super) fn init_region_live_at<T: FactTypes>(
-    var_used: Vec<(T::Variable, T::Point)>,
-    var_drop_used: Vec<(T::Variable, T::Point)>,
-    var_defined: Vec<(T::Variable, T::Point)>,
-    var_uses_region: Vec<(T::Variable, T::Origin)>,
-    var_drops_region: Vec<(T::Variable, T::Origin)>,
-    var_maybe_initialized_on_exit: Relation<(T::Variable, T::Point)>,
-    cfg_edge: &Relation<(T::Point, T::Point)>,
-    universal_regions: Vec<T::Origin>,
-    output: &mut Output<T>,
-) -> Vec<(T::Origin, T::Point)> {
-    debug!("init_region_live_at()");
-    let mut region_live_at = compute_live_regions(
-        var_used,
-        var_drop_used,
-        var_defined,
-        var_uses_region,
-        var_drops_region,
-        cfg_edge,
-        var_maybe_initialized_on_exit,
-        output,
-    );
-
-    make_universal_regions_live::<T>(&mut region_live_at, cfg_edge, universal_regions);
-
-    region_live_at
-}

polonius-engine/src/output/location_insensitive.rs

@@ -21,8 +21,6 @@ pub(super) fn compute<T: FactTypes>(
     let timer = Instant::now();
 
     let potential_errors = {
-        let all_facts = &ctx.all_facts;
-
         // Static inputs
         let region_live_at = &ctx.region_live_at;
         let invalidates = &ctx.invalidates;
@@ -40,16 +38,14 @@ pub(super) fn compute<T: FactTypes>(
 
         // subset(origin1, origin2) :- outlives(origin1, origin2, _point)
         subset.extend(
-            all_facts
-                .outlives
+            ctx.outlives
                 .iter()
                 .map(|&(origin1, origin2, _point)| (origin1, origin2)),
         );
 
         // requires(origin, loan) :- borrow_region(origin, loan, _point).
         requires.extend(
-            all_facts
-                .borrow_region
+            ctx.borrow_region
                 .iter()
                 .map(|&(origin, loan, _point)| (origin, loan)),
         );

polonius-engine/src/output/mod.rs

@@ -12,7 +12,6 @@ use datafrog::Relation;
 use rustc_hash::{FxHashMap, FxHashSet};
 use std::borrow::Cow;
 use std::collections::{BTreeMap, BTreeSet};
-use std::mem;
 
 use crate::facts::{AllFacts, Atom, FactTypes};
 
@@ -94,61 +93,102 @@ pub struct Output<T: FactTypes> {
     pub var_maybe_initialized_on_exit: FxHashMap<T::Point, Vec<T::Variable>>,
 }
 
-struct Context<T: FactTypes> {
-    all_facts: AllFacts<T>,
+/// Subset of `AllFacts` dedicated to initialization
+struct InitializationContext<T: FactTypes> {
+    child: Vec<(T::Path, T::Path)>,
+    path_belongs_to_var: Vec<(T::Path, T::Variable)>,
+    initialized_at: Vec<(T::Path, T::Point)>,
+    moved_out_at: Vec<(T::Path, T::Point)>,
+    path_accessed_at: Vec<(T::Path, T::Point)>,
+}
+
+/// Subset of `AllFacts` dedicated to liveness
+struct LivenessContext<T: FactTypes> {
+    var_used: Vec<(T::Variable, T::Point)>,
+    var_defined: Vec<(T::Variable, T::Point)>,
+    var_drop_used: Vec<(T::Variable, T::Point)>,
+    var_uses_region: Vec<(T::Variable, T::Origin)>,
+    var_drops_region: Vec<(T::Variable, T::Origin)>,
+}
 
-    // `Relation`s used by multiple variants as static inputs
+/// Subset of `AllFacts` dedicated to borrow checking, and data ready to use by the variants
+struct Context<'ctx, T: FactTypes> {
+    // `Relation`s used as static inputs, by all variants
     region_live_at: Relation<(T::Origin, T::Point)>,
     invalidates: Relation<(T::Loan, T::Point)>,
-    cfg_edge: Relation<(T::Point, T::Point)>,
+
+    // static inputs used via `Variable`s, by all variants
+    outlives: &'ctx Vec<(T::Origin, T::Origin, T::Point)>,
+    borrow_region: &'ctx Vec<(T::Origin, T::Loan, T::Point)>,
+
+    // static inputs used by variants other than `LocationInsensitive`
     killed: Relation<(T::Loan, T::Point)>,
 
+    // while this static input is unused by `LocationInsensitive`, it's depended on by initialization
+    // and liveness, so already computed by the time we get to borrowcking.
+    cfg_edge: Relation<(T::Point, T::Point)>,
+
     // Partial results possibly used by other variants as input
-    potential_errors: FxHashSet<T::Loan>,
+    potential_errors: Option<FxHashSet<T::Loan>>,
 }
 
 impl<T: FactTypes> Output<T> {
+    /// All variants require the same initial preparations, done in multiple
+    /// successive steps:
+    /// - compute initialization data
+    /// - compute liveness
+    /// - prepare static inputs as shared `Relation`s
+    /// - in cases where `LocationInsensitive` variant is ran as a filtering pre-pass,
+    ///   partial results can also be stored in the context, so that the following
+    ///   variant can use it to prune its own input data
     pub fn compute(all_facts: &AllFacts<T>, algorithm: Algorithm, dump_enabled: bool) -> Self {
-        // All variants require the same initial preparations, done in multiple
-        // successive steps:
-        // - compute initialization data
-        // - compute liveness
-        // - prepare static inputs as shared `Relation`s
-        // - in cases where `LocationInsensitive` variant is ran as a filtering pre-pass,
-        //   partial results can also be stored in the context, so that the following
-        //   variant can use it to prune its own input data
-
-        // TODO: remove the need for this clone in concert here and in rustc
-        let mut all_facts = all_facts.clone();
-
         let mut result = Output::new(dump_enabled);
 
-        let cfg_edge = mem::replace(&mut all_facts.cfg_edge, Vec::new()).into();
+        // TODO: remove all the cloning thereafter, but that needs to be done in concert with rustc
+
+        let cfg_edge = all_facts.cfg_edge.clone().into();
+
+        // 1) Initialization
+        let initialization_ctx = InitializationContext {
+            child: all_facts.child.clone(),
+            path_belongs_to_var: all_facts.path_belongs_to_var.clone(),
+            initialized_at: all_facts.initialized_at.clone(),
+            moved_out_at: all_facts.moved_out_at.clone(),
+            path_accessed_at: all_facts.path_accessed_at.clone(),
+        };
 
-        // Initialization
         let var_maybe_initialized_on_exit = initialization::init_var_maybe_initialized_on_exit(
-            mem::replace(&mut all_facts.child, Vec::new()),
-            mem::replace(&mut all_facts.path_belongs_to_var, Vec::new()),
-            mem::replace(&mut all_facts.initialized_at, Vec::new()),
-            mem::replace(&mut all_facts.moved_out_at, Vec::new()),
-            mem::replace(&mut all_facts.path_accessed_at, Vec::new()),
+            initialization_ctx,
             &cfg_edge,
             &mut result,
         );
 
-        // Liveness
-        let region_live_at = liveness::init_region_live_at(
-            mem::replace(&mut all_facts.var_used, Vec::new()),
-            mem::replace(&mut all_facts.var_drop_used, Vec::new()),
-            mem::replace(&mut all_facts.var_defined, Vec::new()),
-            mem::replace(&mut all_facts.var_uses_region, Vec::new()),
-            mem::replace(&mut all_facts.var_drops_region, Vec::new()),
-            var_maybe_initialized_on_exit,
+        // 2) Liveness
+        let universal_regions = all_facts.universal_region.clone();
+
+        let liveness_ctx = LivenessContext {
+            var_used: all_facts.var_used.clone(),
+            var_defined: all_facts.var_defined.clone(),
+            var_drop_used: all_facts.var_drop_used.clone(),
+            var_uses_region: all_facts.var_uses_region.clone(),
+            var_drops_region: all_facts.var_drops_region.clone(),
+        };
+
+        let mut region_live_at = liveness::compute_live_regions(
+            liveness_ctx,
             &cfg_edge,
-            mem::replace(&mut all_facts.universal_region, Vec::new()),
+            var_maybe_initialized_on_exit,
             &mut result,
         );
 
+        liveness::make_universal_regions_live::<T>(
+            &mut region_live_at,
+            &cfg_edge,
+            universal_regions,
+        );
+
+        // 3) Borrow checking
+
         // Prepare data as datafrog relations, ready to join.
         //
         // Note: if rustc and polonius had more interaction, we could also delay or avoid
@@ -160,26 +200,28 @@ impl<T: FactTypes> Output<T> {
         // be recorded in separate MIR walks, we might also avoid generating those facts.
 
         let region_live_at = region_live_at.into();
-        let killed = mem::replace(&mut all_facts.killed, Vec::new()).into();
 
-        // TODO: flip the order of this relation's arguments in rustc
-        // from `invalidates(loan, point)` to `invalidates(point, loan)`.
+        // TODO: also flip the order of this relation's arguments in rustc
+        // from `invalidates(point, loan)` to `invalidates(loan, point)`.
         // to avoid this allocation.
         let invalidates = Relation::from_iter(
             all_facts
                 .invalidates
                 .iter()
-                .map(|&(loan, point)| (point, loan)),
+                .map(|&(point, loan)| (loan, point)),
         );
 
+        let killed = all_facts.killed.clone().into();
+
         // Ask the variants to compute errors in their own way
         let mut ctx = Context {
-            all_facts,
             region_live_at,
-            cfg_edge,
             invalidates,
+            cfg_edge,
+            outlives: &all_facts.outlives,
+            borrow_region: &all_facts.borrow_region,
             killed,
-            potential_errors: FxHashSet::default(),
+            potential_errors: None,
         };
 
         let errors = match algorithm {
@@ -196,8 +238,8 @@ impl<T: FactTypes> Output<T> {
                 } else {
                     // Record these potential errors as they can be used to limit the next
                     // variant's work to only these loans.
-                    ctx.potential_errors
-                        .extend(potential_errors.iter().map(|&(loan, _)| loan));
+                    ctx.potential_errors =
+                        Some(potential_errors.iter().map(|&(loan, _)| loan).collect());
 
                     datafrog_opt::compute(&ctx, &mut result)
                 }