From d580ea4de8eb9aca1c145db438bf890b4e30c432 Mon Sep 17 00:00:00 2001 From: lqd Date: Mon, 14 Oct 2019 13:43:44 +0200 Subject: [PATCH] Remove use of `AllFacts` everywhere and divide it into type-safe step-dedicated contexts --- polonius-engine/src/output/datafrog_opt.rs | 18 +-- polonius-engine/src/output/initialization.rs | 20 ++- polonius-engine/src/output/liveness.rs | 49 ++----- .../src/output/location_insensitive.rs | 8 +- polonius-engine/src/output/mod.rs | 128 ++++++++++++------ polonius-engine/src/output/naive.rs | 11 +- 6 files changed, 114 insertions(+), 120 deletions(-) diff --git a/polonius-engine/src/output/datafrog_opt.rs b/polonius-engine/src/output/datafrog_opt.rs index 253a0ffea93..f979143795e 100644 --- a/polonius-engine/src/output/datafrog_opt.rs +++ b/polonius-engine/src/output/datafrog_opt.rs @@ -21,12 +21,10 @@ pub(super) fn compute( 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( // 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( // 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)), ); diff --git a/polonius-engine/src/output/initialization.rs b/polonius-engine/src/output/initialization.rs index 16034edeba6..25e30b93c06 100644 --- a/polonius-engine/src/output/initialization.rs +++ b/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( - 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, cfg_edge: &Relation<(T::Point, T::Point)>, output: &mut Output, ) -> Relation<(T::Variable, T::Point)> { @@ -19,11 +15,11 @@ pub(super) fn init_var_maybe_initialized_on_exit( // 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 diff --git a/polonius-engine/src/output/liveness.rs b/polonius-engine/src/output/liveness.rs index 50546637545..bf30721e4e1 100644 --- a/polonius-engine/src/output/liveness.rs +++ b/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( - 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, cfg_edge_rel: &Relation<(T::Point, T::Point)>, var_maybe_initialized_on_exit_rel: Relation<(T::Variable, T::Point)>, output: &mut Output, @@ -32,14 +28,15 @@ pub(super) fn compute_live_regions( 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( 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( } } } - -pub(super) fn init_region_live_at( - 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, - output: &mut Output, -) -> 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::(&mut region_live_at, cfg_edge, universal_regions); - - region_live_at -} diff --git a/polonius-engine/src/output/location_insensitive.rs b/polonius-engine/src/output/location_insensitive.rs index 20e0e3be530..23153559bcc 100644 --- a/polonius-engine/src/output/location_insensitive.rs +++ b/polonius-engine/src/output/location_insensitive.rs @@ -21,8 +21,6 @@ pub(super) fn compute( 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( // 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)), ); diff --git a/polonius-engine/src/output/mod.rs b/polonius-engine/src/output/mod.rs index 5019dd0d12c..e69f7f532ae 100644 --- a/polonius-engine/src/output/mod.rs +++ b/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 { pub var_maybe_initialized_on_exit: FxHashMap>, } -struct Context { - all_facts: AllFacts, +/// Subset of `AllFacts` dedicated to initialization +struct InitializationContext { + 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 { + 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, + potential_errors: Option>, } impl Output { + /// 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, 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::( + &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 Output { // 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 Output { } 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) } diff --git a/polonius-engine/src/output/naive.rs b/polonius-engine/src/output/naive.rs index e7bcb8f1632..f4812352660 100644 --- a/polonius-engine/src/output/naive.rs +++ b/polonius-engine/src/output/naive.rs @@ -23,8 +23,6 @@ pub(super) fn compute( 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; @@ -57,13 +55,12 @@ pub(super) fn compute( let errors = iteration.variable("errors"); // load initial facts. - subset.extend(all_facts.outlives.iter()); - requires.extend(all_facts.borrow_region.iter()); + subset.extend(ctx.outlives.iter()); + requires.extend(ctx.borrow_region.iter()); 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