From cc6e79b20b7ff1a810075eb78ad9fdff2bb9e705 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Re=CC=81my=20Rakic?= Date: Sat, 12 Oct 2019 14:50:47 +0200 Subject: [PATCH 1/8] Introduce a context to store data common to all variants - Introduce a context to store data common to all variants: initialization, liveness, some of the facts as Relations, so that the Hybrid variant doesn't have to perform the same steps twice - inline the hybrid variant - make variants return a Relation of errors so we can reuse them more easily in the future - store partial results in the context: the locinsensitive potential errors will be used to filter loans in the optimized variant which follows it - make liveness and initialization use and produce Relations instead of cloning them via Vecs --- polonius-engine/src/output/datafrog_opt.rs | 74 ++------ polonius-engine/src/output/hybrid.rs | 26 --- polonius-engine/src/output/initialization.rs | 8 +- polonius-engine/src/output/liveness.rs | 33 ++-- .../src/output/location_insensitive.rs | 73 ++------ polonius-engine/src/output/mod.rs | 163 ++++++++++++++++-- polonius-engine/src/output/naive.rs | 74 +++----- 7 files changed, 219 insertions(+), 232 deletions(-) delete mode 100644 polonius-engine/src/output/hybrid.rs diff --git a/polonius-engine/src/output/datafrog_opt.rs b/polonius-engine/src/output/datafrog_opt.rs index b0fc9f02854..156b3f66a80 100644 --- a/polonius-engine/src/output/datafrog_opt.rs +++ b/polonius-engine/src/output/datafrog_opt.rs @@ -8,62 +8,34 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use datafrog::{Iteration, Relation, RelationLeaper}; use std::time::Instant; -use crate::output::initialization; -use crate::output::liveness; -use crate::output::Output; - -use datafrog::{Iteration, Relation, RelationLeaper}; -use facts::{AllFacts, FactTypes}; - -pub(super) fn compute(dump_enabled: bool, all_facts: AllFacts) -> Output { - let mut result = Output::new(dump_enabled); - - let var_maybe_initialized_on_exit = initialization::init_var_maybe_initialized_on_exit( - all_facts.child, - all_facts.path_belongs_to_var, - all_facts.initialized_at, - all_facts.moved_out_at, - all_facts.path_accessed_at, - &all_facts.cfg_edge, - &mut result, - ); - - let region_live_at = liveness::init_region_live_at( - all_facts.var_used, - all_facts.var_drop_used, - all_facts.var_defined, - all_facts.var_uses_region, - all_facts.var_drops_region, - var_maybe_initialized_on_exit, - &all_facts.cfg_edge, - all_facts.universal_region, - &mut result, - ); +use crate::facts::FactTypes; +use crate::output::{Context, Output}; +pub(super) fn compute( + ctx: &Context, + result: &mut Output, +) -> Relation<(T::Loan, T::Point)> { let timer = Instant::now(); let errors = { - // Create a new iteration context, ... - let mut iteration = Iteration::new(); + let all_facts = &ctx.all_facts; - // static inputs - let cfg_edge_rel = Relation::from_iter( - all_facts - .cfg_edge - .iter() - .map(|&(point1, point2)| (point1, point2)), - ); + // Static inputs + let cfg_edge_rel = &ctx.cfg_edge; + let killed_rel = &ctx.killed; + let region_live_at_rel = &ctx.region_live_at; - let killed_rel: Relation<(T::Loan, T::Point)> = all_facts.killed.into(); + // Create a new iteration context, ... + let mut iteration = Iteration::new(); // `invalidates` facts, stored ready for joins let invalidates = iteration.variable::<((T::Loan, T::Point), ())>("invalidates"); // we need `region_live_at` in both variable and relation forms, // (respectively, for join and antijoin). - let region_live_at_rel: Relation<(T::Origin, T::Point)> = region_live_at.into(); let region_live_at_var = iteration.variable::<((T::Origin, T::Point), ())>("region_live_at"); @@ -408,15 +380,7 @@ pub(super) fn compute(dump_enabled: bool, all_facts: AllFacts) }); } - if dump_enabled { - for &(origin, location) in region_live_at_rel.iter() { - result - .region_live_at - .entry(location) - .or_default() - .push(origin); - } - + if result.dump_enabled { let subset_o1p = subset_o1p.complete(); assert!( subset_o1p @@ -460,7 +424,7 @@ pub(super) fn compute(dump_enabled: bool, all_facts: AllFacts) errors.complete() }; - if dump_enabled { + if result.dump_enabled { info!( "errors is complete: {} tuples, {:?}", errors.len(), @@ -468,9 +432,5 @@ pub(super) fn compute(dump_enabled: bool, all_facts: AllFacts) ); } - for &(loan, location) in errors.iter() { - result.errors.entry(location).or_default().push(loan); - } - - result + errors } diff --git a/polonius-engine/src/output/hybrid.rs b/polonius-engine/src/output/hybrid.rs deleted file mode 100644 index 91ef4fede1b..00000000000 --- a/polonius-engine/src/output/hybrid.rs +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2019 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -//! A hybrid version combining the optimized Datafrog model with the -//! location-insensitive version. - -use crate::output::datafrog_opt; -use crate::output::location_insensitive; -use crate::output::Output; -use facts::{AllFacts, FactTypes}; - -pub(super) fn compute(dump_enabled: bool, all_facts: AllFacts) -> Output { - let lins_output = location_insensitive::compute(dump_enabled, &all_facts); - if lins_output.errors.is_empty() { - lins_output - } else { - datafrog_opt::compute(dump_enabled, all_facts) - } -} diff --git a/polonius-engine/src/output/initialization.rs b/polonius-engine/src/output/initialization.rs index ca711ee516e..8866b3cb368 100644 --- a/polonius-engine/src/output/initialization.rs +++ b/polonius-engine/src/output/initialization.rs @@ -11,9 +11,9 @@ pub(super) fn init_var_maybe_initialized_on_exit( initialized_at: Vec<(T::Path, T::Point)>, moved_out_at: Vec<(T::Path, T::Point)>, path_accessed_at: Vec<(T::Path, T::Point)>, - cfg_edge: &[(T::Point, T::Point)], + cfg_edge: &Relation<(T::Point, T::Point)>, output: &mut Output, -) -> Vec<(T::Variable, T::Point)> { +) -> Relation<(T::Variable, T::Point)> { let computation_start = Instant::now(); let mut iteration = Iteration::new(); @@ -23,7 +23,6 @@ pub(super) fn init_var_maybe_initialized_on_exit( 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 cfg_edge: Relation<(T::Point, T::Point)> = cfg_edge.iter().cloned().collect(); let _path_accessed_at: Relation<(T::Path, T::Point)> = path_accessed_at.into(); // Variables @@ -130,7 +129,4 @@ pub(super) fn init_var_maybe_initialized_on_exit( } var_maybe_initialized_on_exit - .iter() - .map(|&(var, point)| (var, point)) - .collect() } diff --git a/polonius-engine/src/output/liveness.rs b/polonius-engine/src/output/liveness.rs index c28a8a65c1a..13d03ba0636 100644 --- a/polonius-engine/src/output/liveness.rs +++ b/polonius-engine/src/output/liveness.rs @@ -24,8 +24,8 @@ pub(super) fn compute_live_regions( var_defined: Vec<(T::Variable, T::Point)>, var_uses_region: Vec<(T::Variable, T::Origin)>, var_drops_region: Vec<(T::Variable, T::Origin)>, - cfg_edge: &[(T::Point, T::Point)], - var_maybe_initialized_on_exit: Vec<(T::Variable, T::Point)>, + cfg_edge_rel: &Relation<(T::Point, T::Point)>, + var_maybe_initialized_on_exit_rel: Relation<(T::Variable, T::Point)>, output: &mut Output, ) -> Vec<(T::Origin, T::Point)> { let computation_start = Instant::now(); @@ -33,18 +33,12 @@ pub(super) fn compute_live_regions( // Relations let var_defined_rel: Relation<(T::Variable, T::Point)> = var_defined.into(); - let cfg_edge_rel: Relation<(T::Point, T::Point)> = cfg_edge - .iter() - .map(|&(point1, point2)| (point1, point2)) - .collect(); - let cfg_edge_reverse_rel: Relation<(T::Point, T::Point)> = cfg_edge + 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_maybe_initialized_on_exit_rel: Relation<(T::Variable, T::Point)> = - var_maybe_initialized_on_exit.into(); let var_drop_used_rel: Relation<((T::Variable, T::Point), ())> = var_drop_used .into_iter() .map(|(var, point)| ((var, point), ())) @@ -58,7 +52,7 @@ pub(super) fn compute_live_regions( let var_drop_live_var = iteration.variable::<(T::Variable, T::Point)>("var_drop_live_at"); // This is what we are actually calculating: - let region_live_at_var = iteration.variable::<((T::Origin, T::Point), ())>("region_live_at"); + 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()); @@ -88,7 +82,7 @@ pub(super) fn compute_live_regions( region_live_at_var.from_join( &var_drop_live_var, &var_drops_region_rel, - |_var, &point, &origin| ((origin, point), ()), + |_var, &point, &origin| (origin, point), ); // region_live_at(origin, point) :- @@ -97,7 +91,7 @@ pub(super) fn compute_live_regions( region_live_at_var.from_join( &var_live_var, &var_uses_region_rel, - |_var, &point, &origin| ((origin, point), ()), + |_var, &point, &origin| (origin, point), ); // var_live(var, point1) :- @@ -132,12 +126,12 @@ pub(super) fn compute_live_regions( ); } - let region_live_at_rel = region_live_at_var.complete(); + let region_live_at = region_live_at_var.complete(); info!( "compute_live_regions() completed: {} tuples, {:?}", - region_live_at_rel.len(), - computation_start.elapsed() + region_live_at.len(), + computation_start.elapsed(), ); if output.dump_enabled { @@ -156,10 +150,7 @@ pub(super) fn compute_live_regions( } } - region_live_at_rel - .iter() - .map(|&((origin, point), _)| (origin, point)) - .collect() + region_live_at.elements } pub(super) fn make_universal_region_live( @@ -189,8 +180,8 @@ pub(super) fn init_region_live_at( 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: Vec<(T::Variable, T::Point)>, - cfg_edge: &[(T::Point, T::Point)], + var_maybe_initialized_on_exit: Relation<(T::Variable, T::Point)>, + cfg_edge: &Relation<(T::Point, T::Point)>, universal_region: Vec, output: &mut Output, ) -> Vec<(T::Origin, T::Point)> { diff --git a/polonius-engine/src/output/location_insensitive.rs b/polonius-engine/src/output/location_insensitive.rs index 80cc5b482dd..cee0c0affb6 100644 --- a/polonius-engine/src/output/location_insensitive.rs +++ b/polonius-engine/src/output/location_insensitive.rs @@ -8,53 +8,28 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use datafrog::{Iteration, Relation, RelationLeaper}; use std::time::Instant; -use crate::output::initialization; -use crate::output::liveness; -use crate::output::Output; +use crate::facts::FactTypes; +use crate::output::{Context, Output}; -use datafrog::{Iteration, Relation, RelationLeaper}; -use facts::{AllFacts, FactTypes}; - -pub(super) fn compute(dump_enabled: bool, all_facts: &AllFacts) -> Output { - let mut result = Output::new(dump_enabled); - let var_maybe_initialized_on_exit = initialization::init_var_maybe_initialized_on_exit( - all_facts.child.clone(), - all_facts.path_belongs_to_var.clone(), - all_facts.initialized_at.clone(), - all_facts.moved_out_at.clone(), - all_facts.path_accessed_at.clone(), - &all_facts.cfg_edge, - &mut result, - ); - let region_live_at = liveness::init_region_live_at( - all_facts.var_used.clone(), - all_facts.var_drop_used.clone(), - all_facts.var_defined.clone(), - all_facts.var_uses_region.clone(), - all_facts.var_drops_region.clone(), - var_maybe_initialized_on_exit.clone(), - &all_facts.cfg_edge, - all_facts.universal_region.clone(), - &mut result, - ); - - let potential_errors_start = Instant::now(); +pub(super) fn compute( + ctx: &Context, + result: &mut Output, +) -> Relation<(T::Loan, T::Point)> { + 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; + // Create a new iteration context, ... let mut iteration = Iteration::new(); - // static inputs - let region_live_at: Relation<(T::Origin, T::Point)> = region_live_at.into(); - let invalidates = Relation::from_iter( - all_facts - .invalidates - .iter() - .map(|&(loan, point)| (point, loan)), - ); - // .. some variables, .. let subset = iteration.variable::<(T::Origin, T::Origin)>("subset"); let requires = iteration.variable::<(T::Origin, T::Loan)>("requires"); @@ -112,7 +87,7 @@ pub(super) fn compute(dump_enabled: bool, all_facts: &AllFacts) ); } - if dump_enabled { + if result.dump_enabled { let subset = subset.complete(); for &(origin1, origin2) in subset.iter() { result @@ -130,30 +105,18 @@ pub(super) fn compute(dump_enabled: bool, all_facts: &AllFacts) .or_default() .insert(loan); } - - for &(origin, location) in region_live_at.iter() { - result - .region_live_at - .entry(location) - .or_default() - .push(origin); - } } potential_errors.complete() }; - if dump_enabled { + if result.dump_enabled { info!( "potential_errors is complete: {} tuples, {:?}", potential_errors.len(), - potential_errors_start.elapsed() + timer.elapsed() ); } - for &(loan, location) in potential_errors.iter() { - result.errors.entry(location).or_default().push(loan); - } - - result + potential_errors } diff --git a/polonius-engine/src/output/mod.rs b/polonius-engine/src/output/mod.rs index 8351512fde0..dbbe70b9409 100644 --- a/polonius-engine/src/output/mod.rs +++ b/polonius-engine/src/output/mod.rs @@ -8,17 +8,19 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use rustc_hash::FxHashMap; +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}; mod datafrog_opt; -mod hybrid; mod initialization; mod liveness; mod location_insensitive; mod naive; -use facts::{AllFacts, Atom, FactTypes}; #[derive(Debug, Clone, Copy)] pub enum Algorithm { @@ -117,18 +119,136 @@ fn compare_errors( differ } +struct Context { + all_facts: AllFacts, + + // `Relation`s used by multiple variants as static inputs + region_live_at: Relation<(T::Origin, T::Point)>, + invalidates: Relation<(T::Loan, T::Point)>, + cfg_edge: Relation<(T::Point, T::Point)>, + killed: Relation<(T::Loan, T::Point)>, + + // Partial results possibly used by other variants as input + potential_errors: FxHashSet, +} + impl Output { pub fn compute(all_facts: &AllFacts, algorithm: Algorithm, dump_enabled: bool) -> Self { - match algorithm { - Algorithm::Naive => naive::compute(dump_enabled, all_facts.clone()), - Algorithm::DatafrogOpt => datafrog_opt::compute(dump_enabled, all_facts.clone()), - Algorithm::LocationInsensitive => { - location_insensitive::compute(dump_enabled, &all_facts) + // 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(); + + // 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()), + &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, + &cfg_edge, + mem::replace(&mut all_facts.universal_region, Vec::new()), + &mut result, + ); + + // Prepare data as datafrog relations, ready to join. + // + // Note: if rustc and polonius had more interaction, we could also delay or avoid + // generating some of the facts that are now always present here. For example, + // the `LocationInsensitive` variant doesn't use the `killed` or `invalidates` + // relations, so we could technically delay passing them from rustc, when + // using this or the `Hybrid` variant, to after the pre-pass has made sure + // we actually need to compute the full analysis. If these facts happened to + // 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)`. + // to avoid this allocation. + let invalidates = Relation::from_iter( + all_facts + .invalidates + .iter() + .map(|&(loan, point)| (point, loan)), + ); + + // Ask the variants to compute errors in their own way + let mut ctx = Context { + all_facts, + region_live_at, + cfg_edge, + invalidates, + killed, + potential_errors: FxHashSet::default(), + }; + + let errors = match algorithm { + Algorithm::LocationInsensitive => location_insensitive::compute(&ctx, &mut result), + Algorithm::Naive => naive::compute(&ctx, &mut result), + Algorithm::DatafrogOpt => datafrog_opt::compute(&ctx, &mut result), + Algorithm::Hybrid => { + // Execute the fast `LocationInsensitive` computation as a pre-pass: + // if it finds no possible errors, we don't need to do the more complex + // computations as they won't find errors either, and we can return early. + let potential_errors = location_insensitive::compute(&ctx, &mut result); + if potential_errors.is_empty() { + potential_errors + } 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)); + + datafrog_opt::compute(&ctx, &mut result) + } } Algorithm::Compare => { - let naive_output = naive::compute(dump_enabled, all_facts.clone()); - let opt_output = datafrog_opt::compute(dump_enabled, all_facts.clone()); - if compare_errors(&naive_output.errors, &opt_output.errors) { + // Ensure the `Naive` and `DatafrogOpt` errors are the same + let naive_errors = naive::compute(&ctx, &mut result); + let opt_errors = datafrog_opt::compute(&ctx, &mut result); + + let mut naive_errors_by_point = FxHashMap::default(); + for &(loan, point) in naive_errors.iter() { + naive_errors_by_point + .entry(point) + .or_insert(Vec::new()) + .push(loan); + } + + let mut opt_errors_by_point = FxHashMap::default(); + for &(loan, point) in opt_errors.iter() { + opt_errors_by_point + .entry(point) + .or_insert(Vec::new()) + .push(loan); + } + + if compare_errors(&naive_errors_by_point, &opt_errors_by_point) { panic!(concat!( "The errors reported by the naive algorithm differ from ", "the errors reported by the optimized algorithm. ", @@ -137,10 +257,27 @@ impl Output { } else { debug!("Naive and optimized algorithms reported the same errors."); } - opt_output + + naive_errors } - Algorithm::Hybrid => hybrid::compute(dump_enabled, all_facts.clone()), + }; + + for &(loan, location) in errors.iter() { + result.errors.entry(location).or_default().push(loan); } + + // Record more debugging info when necessary + if dump_enabled { + for &(origin, location) in ctx.region_live_at.iter() { + result + .region_live_at + .entry(location) + .or_default() + .push(origin); + } + } + + result } fn new(dump_enabled: bool) -> Self { diff --git a/polonius-engine/src/output/naive.rs b/polonius-engine/src/output/naive.rs index 73922109625..29171b8cd26 100644 --- a/polonius-engine/src/output/naive.rs +++ b/polonius-engine/src/output/naive.rs @@ -10,51 +10,29 @@ //! A version of the Naive datalog analysis using Datafrog. -use std::time::Instant; - -use crate::output::initialization; -use crate::output::liveness; -use crate::output::Output; -use facts::{AllFacts, FactTypes}; - use datafrog::{Iteration, Relation, RelationLeaper}; +use std::time::Instant; -pub(super) fn compute(dump_enabled: bool, all_facts: AllFacts) -> Output { - let mut result = Output::new(dump_enabled); +use crate::facts::FactTypes; +use crate::output::{Context, Output}; - let var_maybe_initialized_on_exit = initialization::init_var_maybe_initialized_on_exit( - all_facts.child, - all_facts.path_belongs_to_var, - all_facts.initialized_at, - all_facts.moved_out_at, - all_facts.path_accessed_at, - &all_facts.cfg_edge, - &mut result, - ); +pub(super) fn compute( + ctx: &Context, + result: &mut Output, +) -> Relation<(T::Loan, T::Point)> { + let timer = Instant::now(); - let region_live_at = liveness::init_region_live_at( - all_facts.var_used, - all_facts.var_drop_used, - all_facts.var_defined, - all_facts.var_uses_region, - all_facts.var_drops_region, - var_maybe_initialized_on_exit, - &all_facts.cfg_edge, - all_facts.universal_region, - &mut result, - ); + let errors = { + let all_facts = &ctx.all_facts; - let computation_start = Instant::now(); + // Static inputs + let region_live_at_rel = &ctx.region_live_at; + let cfg_edge_rel = &ctx.cfg_edge; + let killed_rel = &ctx.killed; - let errors = { // Create a new iteration context, ... let mut iteration = Iteration::new(); - // static inputs - let cfg_edge_rel: Relation<(T::Point, T::Point)> = all_facts.cfg_edge.into(); - let killed_rel: Relation<(T::Loan, T::Point)> = all_facts.killed.into(); - let region_live_at_rel: Relation<(T::Origin, T::Point)> = region_live_at.into(); - // .. some variables, .. let subset = iteration.variable::<(T::Origin, T::Origin, T::Point)>("subset"); let requires = iteration.variable::<(T::Origin, T::Loan, T::Point)>("requires"); @@ -79,8 +57,8 @@ pub(super) fn compute(dump_enabled: bool, all_facts: AllFacts) let errors = iteration.variable("errors"); // load initial facts. - subset.insert(all_facts.outlives.into()); - requires.insert(all_facts.borrow_region.into()); + subset.extend(all_facts.outlives.iter()); + requires.extend(all_facts.borrow_region.iter()); invalidates.extend( all_facts .invalidates @@ -190,7 +168,7 @@ pub(super) fn compute(dump_enabled: bool, all_facts: AllFacts) }); } - if dump_enabled { + if result.dump_enabled { let subset = subset.complete(); assert!( subset @@ -221,14 +199,6 @@ pub(super) fn compute(dump_enabled: bool, all_facts: AllFacts) .insert(loan); } - for &(origin, location) in region_live_at_rel.iter() { - result - .region_live_at - .entry(location) - .or_default() - .push(origin); - } - let borrow_live_at = borrow_live_at.complete(); for &((loan, location), _) in borrow_live_at.iter() { result @@ -242,17 +212,13 @@ pub(super) fn compute(dump_enabled: bool, all_facts: AllFacts) errors.complete() }; - if dump_enabled { + if result.dump_enabled { info!( "errors is complete: {} tuples, {:?}", errors.len(), - computation_start.elapsed() + timer.elapsed() ); } - for &(loan, location) in errors.iter() { - result.errors.entry(location).or_default().push(loan); - } - - result + errors } From 704612e70dc536e62526c782abc644b8be08d2de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Re=CC=81my=20Rakic?= Date: Sat, 12 Oct 2019 15:14:18 +0200 Subject: [PATCH 2/8] cleanup some names --- polonius-engine/src/output/initialization.rs | 4 ++-- polonius-engine/src/output/liveness.rs | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/polonius-engine/src/output/initialization.rs b/polonius-engine/src/output/initialization.rs index 8866b3cb368..0debfa6b13b 100644 --- a/polonius-engine/src/output/initialization.rs +++ b/polonius-engine/src/output/initialization.rs @@ -14,7 +14,7 @@ pub(super) fn init_var_maybe_initialized_on_exit( cfg_edge: &Relation<(T::Point, T::Point)>, output: &mut Output, ) -> Relation<(T::Variable, T::Point)> { - let computation_start = Instant::now(); + let timer = Instant::now(); let mut iteration = Iteration::new(); // Relations @@ -106,7 +106,7 @@ pub(super) fn init_var_maybe_initialized_on_exit( info!( "init_var_maybe_initialized_on_exit() completed: {} tuples, {:?}", var_maybe_initialized_on_exit.len(), - computation_start.elapsed() + timer.elapsed() ); if output.dump_enabled { diff --git a/polonius-engine/src/output/liveness.rs b/polonius-engine/src/output/liveness.rs index 13d03ba0636..50546637545 100644 --- a/polonius-engine/src/output/liveness.rs +++ b/polonius-engine/src/output/liveness.rs @@ -28,7 +28,7 @@ pub(super) fn compute_live_regions( var_maybe_initialized_on_exit_rel: Relation<(T::Variable, T::Point)>, output: &mut Output, ) -> Vec<(T::Origin, T::Point)> { - let computation_start = Instant::now(); + let timer = Instant::now(); let mut iteration = Iteration::new(); // Relations @@ -131,7 +131,7 @@ pub(super) fn compute_live_regions( info!( "compute_live_regions() completed: {} tuples, {:?}", region_live_at.len(), - computation_start.elapsed(), + timer.elapsed(), ); if output.dump_enabled { @@ -153,10 +153,10 @@ pub(super) fn compute_live_regions( region_live_at.elements } -pub(super) fn make_universal_region_live( +pub(super) fn make_universal_regions_live( region_live_at: &mut Vec<(T::Origin, T::Point)>, cfg_edge: &[(T::Point, T::Point)], - universal_region: Vec, + universal_regions: Vec, ) { debug!("make_universal_regions_live()"); @@ -166,9 +166,9 @@ pub(super) fn make_universal_region_live( .chain(cfg_edge.iter().map(|&(_, point2)| point2)) .collect(); - region_live_at.reserve(universal_region.len() * all_points.len()); - for &origin in &universal_region { - for &point in &all_points { + region_live_at.reserve(universal_regions.len() * all_points.len()); + for origin in universal_regions { + for &point in all_points.iter() { region_live_at.push((origin, point)); } } @@ -182,7 +182,7 @@ pub(super) fn init_region_live_at( 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_region: Vec, + universal_regions: Vec, output: &mut Output, ) -> Vec<(T::Origin, T::Point)> { debug!("init_region_live_at()"); @@ -197,7 +197,7 @@ pub(super) fn init_region_live_at( output, ); - make_universal_region_live::(&mut region_live_at, cfg_edge, universal_region); + make_universal_regions_live::(&mut region_live_at, cfg_edge, universal_regions); region_live_at } From be1773cd7a8c6a57f25a89649f0c49636365787d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Re=CC=81my=20Rakic?= Date: Sat, 12 Oct 2019 14:51:48 +0200 Subject: [PATCH 3/8] document the variants --- polonius-engine/src/output/mod.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/polonius-engine/src/output/mod.rs b/polonius-engine/src/output/mod.rs index dbbe70b9409..17d8c37c57f 100644 --- a/polonius-engine/src/output/mod.rs +++ b/polonius-engine/src/output/mod.rs @@ -24,11 +24,22 @@ mod naive; #[derive(Debug, Clone, Copy)] pub enum Algorithm { + /// Simple rules, but slower to execute Naive, + + /// Optimized variant of the rules DatafrogOpt, + + /// Fast to compute, but imprecise: there can be false-positives + /// but no false-negatives. Tailored for quick "early return" situations. LocationInsensitive, - /// Compare Naive and DatafrogOpt. + + /// Compares the `Naive` and `DatafrogOpt` variants to ensure they indeed + /// compute the same errors. Compare, + + /// Combination of the fast `LocationInsensitive` pre-pass, followed by + /// the more expensive `DatafrogOpt` variant. Hybrid, } From 8fcd951afde6326da70a2a40b0275475b13415f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Re=CC=81my=20Rakic?= Date: Sat, 12 Oct 2019 14:53:19 +0200 Subject: [PATCH 4/8] cleanup and move a function --- polonius-engine/src/output/mod.rs | 76 ++++++++++++++++--------------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/polonius-engine/src/output/mod.rs b/polonius-engine/src/output/mod.rs index 17d8c37c57f..5019dd0d12c 100644 --- a/polonius-engine/src/output/mod.rs +++ b/polonius-engine/src/output/mod.rs @@ -94,42 +94,6 @@ pub struct Output { pub var_maybe_initialized_on_exit: FxHashMap>, } -/// Compares errors reported by Naive implementation with the errors -/// reported by the optimized implementation. -fn compare_errors( - all_naive_errors: &FxHashMap>, - all_opt_errors: &FxHashMap>, -) -> bool { - let points = all_naive_errors.keys().chain(all_opt_errors.keys()); - - let mut differ = false; - for point in points { - let mut naive_errors = all_naive_errors.get(&point).cloned().unwrap_or(Vec::new()); - naive_errors.sort(); - let mut opt_errors = all_opt_errors.get(&point).cloned().unwrap_or(Vec::new()); - opt_errors.sort(); - for err in naive_errors.iter() { - if !opt_errors.contains(err) { - error!( - "Error {0:?} at {1:?} reported by naive, but not opt.", - err, point - ); - differ = true; - } - } - for err in opt_errors.iter() { - if !naive_errors.contains(err) { - error!( - "Error {0:?} at {1:?} reported by opt, but not naive.", - err, point - ); - differ = true; - } - } - } - differ -} - struct Context { all_facts: AllFacts, @@ -354,6 +318,46 @@ impl Output { } } +/// Compares errors reported by Naive implementation with the errors +/// reported by the optimized implementation. +fn compare_errors( + all_naive_errors: &FxHashMap>, + all_opt_errors: &FxHashMap>, +) -> bool { + let points = all_naive_errors.keys().chain(all_opt_errors.keys()); + + let mut differ = false; + for point in points { + let mut naive_errors = all_naive_errors.get(&point).cloned().unwrap_or_default(); + naive_errors.sort(); + + let mut opt_errors = all_opt_errors.get(&point).cloned().unwrap_or_default(); + opt_errors.sort(); + + for err in naive_errors.iter() { + if !opt_errors.contains(err) { + error!( + "Error {0:?} at {1:?} reported by naive, but not opt.", + err, point + ); + differ = true; + } + } + + for err in opt_errors.iter() { + if !naive_errors.contains(err) { + error!( + "Error {0:?} at {1:?} reported by opt, but not naive.", + err, point + ); + differ = true; + } + } + } + + differ +} + #[cfg(test)] mod tests { use super::*; From 58dcef089f77dc11eed1ba4f6abc7061631e71dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Re=CC=81my=20Rakic?= Date: Sat, 12 Oct 2019 15:25:54 +0200 Subject: [PATCH 5/8] log variant results all the time --- polonius-engine/src/output/datafrog_opt.rs | 12 +++++------- polonius-engine/src/output/location_insensitive.rs | 12 +++++------- polonius-engine/src/output/naive.rs | 12 +++++------- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/polonius-engine/src/output/datafrog_opt.rs b/polonius-engine/src/output/datafrog_opt.rs index 156b3f66a80..253a0ffea93 100644 --- a/polonius-engine/src/output/datafrog_opt.rs +++ b/polonius-engine/src/output/datafrog_opt.rs @@ -424,13 +424,11 @@ pub(super) fn compute( errors.complete() }; - if result.dump_enabled { - info!( - "errors is complete: {} tuples, {:?}", - errors.len(), - timer.elapsed() - ); - } + info!( + "errors is complete: {} tuples, {:?}", + errors.len(), + timer.elapsed() + ); errors } diff --git a/polonius-engine/src/output/location_insensitive.rs b/polonius-engine/src/output/location_insensitive.rs index cee0c0affb6..20e0e3be530 100644 --- a/polonius-engine/src/output/location_insensitive.rs +++ b/polonius-engine/src/output/location_insensitive.rs @@ -110,13 +110,11 @@ pub(super) fn compute( potential_errors.complete() }; - if result.dump_enabled { - info!( - "potential_errors is complete: {} tuples, {:?}", - potential_errors.len(), - timer.elapsed() - ); - } + info!( + "potential_errors is complete: {} tuples, {:?}", + potential_errors.len(), + timer.elapsed() + ); potential_errors } diff --git a/polonius-engine/src/output/naive.rs b/polonius-engine/src/output/naive.rs index 29171b8cd26..e7bcb8f1632 100644 --- a/polonius-engine/src/output/naive.rs +++ b/polonius-engine/src/output/naive.rs @@ -212,13 +212,11 @@ pub(super) fn compute( errors.complete() }; - if result.dump_enabled { - info!( - "errors is complete: {} tuples, {:?}", - errors.len(), - timer.elapsed() - ); - } + info!( + "errors is complete: {} tuples, {:?}", + errors.len(), + timer.elapsed() + ); errors } From 7ba70b65f43bee2e4b80b7a2379c26b5663483ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Re=CC=81my=20Rakic?= Date: Sat, 12 Oct 2019 15:46:34 +0200 Subject: [PATCH 6/8] Configure logging in the `polonius` CLI --- src/cli.rs | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 50ad4ec0b9f..f7c79fef2bd 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,11 +1,7 @@ -use crate::dump; -use crate::dump::Output; -use crate::facts::AllFacts; -use crate::intern; -use crate::tab_delim; -use log::error; +use log::{error, Level, LevelFilter, Metadata, Record, SetLoggerError}; use pico_args as pico; use polonius_engine::Algorithm; +use std::env; use std::error; use std::fmt; use std::path::Path; @@ -13,6 +9,12 @@ use std::process::exit; use std::str::FromStr; use std::time::{Duration, Instant}; +use crate::dump; +use crate::dump::Output; +use crate::facts::AllFacts; +use crate::intern; +use crate::tab_delim; + const PKG_NAME: &'static str = env!("CARGO_PKG_NAME"); const PKG_VERSION: &'static str = env!("CARGO_PKG_VERSION"); const PKG_DESCRIPTION: &'static str = env!("CARGO_PKG_DESCRIPTION"); @@ -184,6 +186,11 @@ For more information try --help"# exit(1); } + // 4) setup logging at the default `Info` level when necessary + if env::var("RUST_LOG").is_ok() { + start_logging().expect("Initializing logger failed"); + } + Ok(options) } @@ -217,3 +224,25 @@ fn readable_pico_error(error: pico::Error) -> Error { Error::NonUtf8Argument => "not a valid utf8 value".to_string(), }) } + +struct Logger; + +impl log::Log for Logger { + fn enabled(&self, metadata: &Metadata) -> bool { + metadata.level() <= Level::Info + } + + fn log(&self, record: &Record) { + if self.enabled(record.metadata()) { + eprintln!("{} {} - {}", record.level(), record.target(), record.args()); + } + } + + fn flush(&self) {} +} + +static LOGGER: Logger = Logger; + +fn start_logging() -> Result<(), SetLoggerError> { + log::set_logger(&LOGGER).map(|()| log::set_max_level(LevelFilter::Info)) +} From 61d763e590d2b6e60af4be6a3156832d34dab685 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Re=CC=81my=20Rakic?= Date: Sat, 12 Oct 2019 22:04:21 +0200 Subject: [PATCH 7/8] micro optimization: regular joins can be used in initialization code instead of leapjoins that's 1% of free benchmark real estate --- polonius-engine/src/output/initialization.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/polonius-engine/src/output/initialization.rs b/polonius-engine/src/output/initialization.rs index 0debfa6b13b..16034edeba6 100644 --- a/polonius-engine/src/output/initialization.rs +++ b/polonius-engine/src/output/initialization.rs @@ -59,10 +59,10 @@ pub(super) fn init_var_maybe_initialized_on_exit( // path_maybe_initialized_on_exit(Mother, point) :- // path_maybe_initialized_on_exit(Daughter, point), // child(Daughter, Mother). - path_maybe_initialized_on_exit.from_leapjoin( + path_maybe_initialized_on_exit.from_join( &path_maybe_initialized_on_exit, - child.extend_with(|&(daughter, _point)| daughter), - |&(_daughter, point), &mother| (mother, point), + &child, + |&_daughter, &point, &mother| (mother, point), ); // TODO: the following lines contain things left to implement for move @@ -92,12 +92,12 @@ pub(super) fn init_var_maybe_initialized_on_exit( // END TODO // var_maybe_initialized_on_exit(var, point) :- - // path_belongs_to_var(path, var), - // path_maybe_initialized_at(path, point). - var_maybe_initialized_on_exit.from_leapjoin( + // path_maybe_initialized_on_exit(path, point), + // path_belongs_to_var(path, var). + var_maybe_initialized_on_exit.from_join( &path_maybe_initialized_on_exit, - path_belongs_to_var.extend_with(|&(path, _point)| path), - |&(_path, point), &var| (var, point), + &path_belongs_to_var, + |&_path, &point, &var| (var, point), ); } From d580ea4de8eb9aca1c145db438bf890b4e30c432 Mon Sep 17 00:00:00 2001 From: lqd Date: Mon, 14 Oct 2019 13:43:44 +0200 Subject: [PATCH 8/8] 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