From cb9f9b107c981261925e38c2d454ea7f706d9f84 Mon Sep 17 00:00:00 2001 From: Albin Stjerna Date: Mon, 11 Mar 2019 16:22:47 +1100 Subject: [PATCH 1/6] =?UTF-8?q?Add=20a=20na=C3=AFve=20hybrid=20algorithm.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #100. --- polonius-engine/src/output/hybrid.rs | 29 ++++++++++++++++++++++++++++ polonius-engine/src/output/mod.rs | 10 +++++++--- src/test.rs | 4 ++++ 3 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 polonius-engine/src/output/hybrid.rs diff --git a/polonius-engine/src/output/hybrid.rs b/polonius-engine/src/output/hybrid.rs new file mode 100644 index 00000000000..75ece833bc1 --- /dev/null +++ b/polonius-engine/src/output/hybrid.rs @@ -0,0 +1,29 @@ +// 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, Atom}; + +pub(super) fn compute( + dump_enabled: bool, + all_facts: AllFacts, +) -> Output { + let lins_output = location_insensitive::compute(dump_enabled, + all_facts.clone()); + return match lins_output.errors.is_empty() { + true => lins_output, + false => datafrog_opt::compute(dump_enabled, all_facts) + }; +} diff --git a/polonius-engine/src/output/mod.rs b/polonius-engine/src/output/mod.rs index 19d225a67c3..0113fd6e3db 100644 --- a/polonius-engine/src/output/mod.rs +++ b/polonius-engine/src/output/mod.rs @@ -15,6 +15,7 @@ use std::collections::{BTreeMap, BTreeSet}; mod datafrog_opt; mod location_insensitive; mod naive; +mod hybrid; use facts::{AllFacts, Atom}; #[derive(Debug, Clone, Copy)] @@ -24,14 +25,15 @@ pub enum Algorithm { LocationInsensitive, /// Compare Naive and DatafrogOpt. Compare, + Hybrid, } impl Algorithm { /// Optimized variants that ought to be equivalent to "naive" pub const OPTIMIZED: &'static [Algorithm] = &[Algorithm::DatafrogOpt]; - pub fn variants() -> [&'static str; 4] { - ["Naive", "DatafrogOpt", "LocationInsensitive", "Compare"] + pub fn variants() -> [&'static str; 5] { + ["Naive", "DatafrogOpt", "LocationInsensitive", "Compare", "Hybrid"] } } @@ -43,8 +45,9 @@ impl ::std::str::FromStr for Algorithm { "datafrogopt" => Ok(Algorithm::DatafrogOpt), "locationinsensitive" => Ok(Algorithm::LocationInsensitive), "compare" => Ok(Algorithm::Compare), + "hybrid" => Ok(Algorithm::Hybrid), _ => Err(String::from( - "valid values: Naive, DatafrogOpt, LocationInsensitive, Compare", + "valid values: Naive, DatafrogOpt, LocationInsensitive, Compare, Hybrid", )), } } @@ -133,6 +136,7 @@ where } opt_output } + Algorithm::Hybrid => hybrid::compute(dump_enabled, all_facts.clone()) } } diff --git a/src/test.rs b/src/test.rs index f9798eae730..6f0c658506b 100644 --- a/src/test.rs +++ b/src/test.rs @@ -48,6 +48,10 @@ fn test_facts(all_facts: &AllFacts, algorithms: &[Algorithm]) { assert_equal(&naive.borrow_live_at, &opt.borrow_live_at); assert_equal(&naive.errors, &opt.errors); } + + // The hybrid algorithm gets the same errors as the naive version + let opt = Output::compute(all_facts, Algorithm::Hybrid, true); + assert_equal(&naive.errors, &opt.errors); } fn test_fn(dir_name: &str, fn_name: &str, algorithm: Algorithm) -> Result<(), Error> { From e7729e09df975a211164f9bfd4f9bbc31a5b5d85 Mon Sep 17 00:00:00 2001 From: Albin Stjerna Date: Mon, 11 Mar 2019 16:33:55 +1100 Subject: [PATCH 2/6] Fix rustfmt errors --- polonius-engine/src/output/hybrid.rs | 5 ++--- polonius-engine/src/output/mod.rs | 12 +++++++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/polonius-engine/src/output/hybrid.rs b/polonius-engine/src/output/hybrid.rs index 75ece833bc1..c7a4279fdff 100644 --- a/polonius-engine/src/output/hybrid.rs +++ b/polonius-engine/src/output/hybrid.rs @@ -20,10 +20,9 @@ pub(super) fn compute( dump_enabled: bool, all_facts: AllFacts, ) -> Output { - let lins_output = location_insensitive::compute(dump_enabled, - all_facts.clone()); + let lins_output = location_insensitive::compute(dump_enabled, all_facts.clone()); return match lins_output.errors.is_empty() { true => lins_output, - false => datafrog_opt::compute(dump_enabled, all_facts) + false => datafrog_opt::compute(dump_enabled, all_facts), }; } diff --git a/polonius-engine/src/output/mod.rs b/polonius-engine/src/output/mod.rs index 0113fd6e3db..68410b1e3c9 100644 --- a/polonius-engine/src/output/mod.rs +++ b/polonius-engine/src/output/mod.rs @@ -13,9 +13,9 @@ use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet}; mod datafrog_opt; +mod hybrid; mod location_insensitive; mod naive; -mod hybrid; use facts::{AllFacts, Atom}; #[derive(Debug, Clone, Copy)] @@ -33,7 +33,13 @@ impl Algorithm { pub const OPTIMIZED: &'static [Algorithm] = &[Algorithm::DatafrogOpt]; pub fn variants() -> [&'static str; 5] { - ["Naive", "DatafrogOpt", "LocationInsensitive", "Compare", "Hybrid"] + [ + "Naive", + "DatafrogOpt", + "LocationInsensitive", + "Compare", + "Hybrid", + ] } } @@ -136,7 +142,7 @@ where } opt_output } - Algorithm::Hybrid => hybrid::compute(dump_enabled, all_facts.clone()) + Algorithm::Hybrid => hybrid::compute(dump_enabled, all_facts.clone()), } } From 814b720bd76a816212abebc18bb2c671f3450622 Mon Sep 17 00:00:00 2001 From: Albin Stjerna Date: Wed, 13 Mar 2019 21:03:29 +1100 Subject: [PATCH 3/6] Fix Clippy warning --- polonius-engine/src/output/hybrid.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/polonius-engine/src/output/hybrid.rs b/polonius-engine/src/output/hybrid.rs index c7a4279fdff..689df26c1c9 100644 --- a/polonius-engine/src/output/hybrid.rs +++ b/polonius-engine/src/output/hybrid.rs @@ -21,8 +21,9 @@ pub(super) fn compute( all_facts: AllFacts, ) -> Output { let lins_output = location_insensitive::compute(dump_enabled, all_facts.clone()); - return match lins_output.errors.is_empty() { - true => lins_output, - false => datafrog_opt::compute(dump_enabled, all_facts), - }; + if lins_output.errors.is_empty() { + lins_output + } else { + datafrog_opt::compute(dump_enabled, all_facts) + } } From 9bcbae6be2b275009a1fdd462854a92081955430 Mon Sep 17 00:00:00 2001 From: Albin Stjerna Date: Thu, 14 Mar 2019 20:10:56 +0100 Subject: [PATCH 4/6] Make `all_facts` a reference for location insensitive --- polonius-engine/src/output/hybrid.rs | 2 +- polonius-engine/src/output/location_insensitive.rs | 4 ++-- polonius-engine/src/output/mod.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/polonius-engine/src/output/hybrid.rs b/polonius-engine/src/output/hybrid.rs index 689df26c1c9..f1912a09a70 100644 --- a/polonius-engine/src/output/hybrid.rs +++ b/polonius-engine/src/output/hybrid.rs @@ -20,7 +20,7 @@ pub(super) fn compute( dump_enabled: bool, all_facts: AllFacts, ) -> Output { - let lins_output = location_insensitive::compute(dump_enabled, all_facts.clone()); + let lins_output = location_insensitive::compute(dump_enabled, &all_facts); if lins_output.errors.is_empty() { lins_output } else { diff --git a/polonius-engine/src/output/location_insensitive.rs b/polonius-engine/src/output/location_insensitive.rs index 40f0bda9f03..c7943b2897f 100644 --- a/polonius-engine/src/output/location_insensitive.rs +++ b/polonius-engine/src/output/location_insensitive.rs @@ -18,7 +18,7 @@ use facts::{AllFacts, Atom}; pub(super) fn compute( dump_enabled: bool, - mut all_facts: AllFacts, + all_facts: &AllFacts, ) -> Output { let all_points: BTreeSet = all_facts .cfg_edge @@ -45,7 +45,7 @@ pub(super) fn compute( let mut iteration = Iteration::new(); // static inputs - let region_live_at: Relation<(Region, Point)> = all_facts.region_live_at.into(); + let region_live_at: Relation<(Region, Point)> = my_region_live_at.into(); let invalidates = Relation::from_iter(all_facts.invalidates.iter().map(|&(b, p)| (p, b))); // .. some variables, .. diff --git a/polonius-engine/src/output/mod.rs b/polonius-engine/src/output/mod.rs index 68410b1e3c9..78d409f66c5 100644 --- a/polonius-engine/src/output/mod.rs +++ b/polonius-engine/src/output/mod.rs @@ -126,7 +126,7 @@ where 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.clone()) + location_insensitive::compute(dump_enabled, &all_facts) } Algorithm::Compare => { let naive_output = naive::compute(dump_enabled, all_facts.clone()); From bb083f22f2ba8718e328f9d30008705a6bfddb98 Mon Sep 17 00:00:00 2001 From: Albin Stjerna Date: Thu, 14 Mar 2019 20:14:57 +0100 Subject: [PATCH 5/6] Mistake: restore my_region declaration Apparently my jet-lagged brain had discarded a hunk of changes by mistake from my commit when I was clearing out an unnecessary whitespace change. --- polonius-engine/src/output/location_insensitive.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/polonius-engine/src/output/location_insensitive.rs b/polonius-engine/src/output/location_insensitive.rs index c7943b2897f..97d2d0dbdbe 100644 --- a/polonius-engine/src/output/location_insensitive.rs +++ b/polonius-engine/src/output/location_insensitive.rs @@ -27,12 +27,11 @@ pub(super) fn compute( .chain(all_facts.cfg_edge.iter().map(|&(_, q)| q)) .collect(); - all_facts - .region_live_at - .reserve(all_facts.universal_region.len() * all_points.len()); + let mut my_region_live_at = all_facts.region_live_at.clone(); + my_region_live_at.reserve(all_facts.universal_region.len() * all_points.len()); for &r in &all_facts.universal_region { for &p in &all_points { - all_facts.region_live_at.push((r, p)); + my_region_live_at.push((r, p)); } } From 833ba373baff68404b5c52d0fcf34b6bbea1ca50 Mon Sep 17 00:00:00 2001 From: Albin Stjerna Date: Thu, 14 Mar 2019 22:09:31 +0100 Subject: [PATCH 6/6] Drop the my_ prefix for region_live_at Apparently the shadowing variable isn't a problem, and the preparations done to the first version of it are basically preparations for creating that variable anyway. --- polonius-engine/src/output/location_insensitive.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/polonius-engine/src/output/location_insensitive.rs b/polonius-engine/src/output/location_insensitive.rs index 97d2d0dbdbe..d9e7bc4057c 100644 --- a/polonius-engine/src/output/location_insensitive.rs +++ b/polonius-engine/src/output/location_insensitive.rs @@ -27,11 +27,11 @@ pub(super) fn compute( .chain(all_facts.cfg_edge.iter().map(|&(_, q)| q)) .collect(); - let mut my_region_live_at = all_facts.region_live_at.clone(); - my_region_live_at.reserve(all_facts.universal_region.len() * all_points.len()); + let mut region_live_at = all_facts.region_live_at.clone(); + region_live_at.reserve(all_facts.universal_region.len() * all_points.len()); for &r in &all_facts.universal_region { for &p in &all_points { - my_region_live_at.push((r, p)); + region_live_at.push((r, p)); } } @@ -44,7 +44,7 @@ pub(super) fn compute( let mut iteration = Iteration::new(); // static inputs - let region_live_at: Relation<(Region, Point)> = my_region_live_at.into(); + let region_live_at: Relation<(Region, Point)> = region_live_at.into(); let invalidates = Relation::from_iter(all_facts.invalidates.iter().map(|&(b, p)| (p, b))); // .. some variables, ..