Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In which we implement illegal subset relations errors using Polonius #67016

Merged
merged 10 commits into from
Dec 9, 2019
4 changes: 2 additions & 2 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2575,9 +2575,9 @@ checksum = "676e8eb2b1b4c9043511a9b7bea0915320d7e502b0a079fb03f9635a5252b18c"

[[package]]
name = "polonius-engine"
version = "0.10.0"
version = "0.11.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "50fa9dbfd0d3d60594da338cfe6f94028433eecae4b11b7e83fd99759227bbfe"
checksum = "1e478d7c38eb785c6416cbe58df12aa55d7aefa3759b6d3e044b2ed03f423cec"
dependencies = [
"datafrog",
"log",
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ scoped-tls = "1.0"
log = { version = "0.4", features = ["release_max_level_info", "std"] }
rustc-rayon = "0.3.0"
rustc-rayon-core = "0.3.0"
polonius-engine = "0.10.0"
polonius-engine = "0.11.0"
rustc_apfloat = { path = "../librustc_apfloat" }
rustc_feature = { path = "../librustc_feature" }
rustc_target = { path = "../librustc_target" }
Expand Down
8 changes: 8 additions & 0 deletions src/librustc_data_structures/transitive_relation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,14 @@ impl<T: Clone + Debug + Eq + Hash> TransitiveRelation<T> {
}
matrix
}

/// Lists all the base edges in the graph: the initial _non-transitive_ set of element
/// relations, which will be later used as the basis for the transitive closure computation.
pub fn base_edges(&self) -> impl Iterator<Item=(&T, &T)> {
self.edges
.iter()
.map(move |edge| (&self.elements[edge.source.0], &self.elements[edge.target.0]))
}
}

/// Pare down is used as a step in the LUB computation. It edits the
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ dot = { path = "../libgraphviz", package = "graphviz" }
itertools = "0.8"
log = "0.4"
log_settings = "0.1.1"
polonius-engine = "0.10.0"
polonius-engine = "0.11.0"
rustc = { path = "../librustc" }
rustc_target = { path = "../librustc_target" }
rustc_data_structures = { path = "../librustc_data_structures" }
Expand Down
9 changes: 3 additions & 6 deletions src/librustc_mir/borrow_check/flows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,15 @@
//! FIXME: this might be better as a "generic" fixed-point combinator,
//! but is not as ugly as it is right now.

use rustc::mir::{BasicBlock, Local, Location};
use rustc::ty::RegionVid;
use rustc::mir::{BasicBlock, Location};
use rustc_index::bit_set::BitIter;

use crate::borrow_check::location::LocationIndex;

use polonius_engine::Output;
use crate::borrow_check::nll::PoloniusOutput;

use crate::dataflow::indexes::BorrowIndex;
use crate::dataflow::move_paths::{HasMoveData, MovePathIndex};
use crate::dataflow::move_paths::HasMoveData;
use crate::dataflow::Borrows;
use crate::dataflow::EverInitializedPlaces;
use crate::dataflow::MaybeUninitializedPlaces;
Expand All @@ -21,8 +20,6 @@ use either::Either;
use std::fmt;
use std::rc::Rc;

crate type PoloniusOutput = Output<RegionVid, BorrowIndex, LocationIndex, Local, MovePathIndex>;

crate struct Flows<'b, 'tcx> {
borrows: FlowAtLocation<'tcx, Borrows<'b, 'tcx>>,
pub uninits: FlowAtLocation<'tcx, MaybeUninitializedPlaces<'b, 'tcx>>,
Expand Down
17 changes: 15 additions & 2 deletions src/librustc_mir/borrow_check/nll/facts.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::borrow_check::location::{LocationIndex, LocationTable};
use crate::dataflow::indexes::{BorrowIndex, MovePathIndex};
use polonius_engine::AllFacts as PoloniusAllFacts;
use polonius_engine::AllFacts as PoloniusFacts;
use polonius_engine::Atom;
use rustc::mir::Local;
use rustc::ty::{RegionVid, TyCtxt};
Expand All @@ -11,7 +11,18 @@ use std::fs::{self, File};
use std::io::Write;
use std::path::Path;

crate type AllFacts = PoloniusAllFacts<RegionVid, BorrowIndex, LocationIndex, Local, MovePathIndex>;
#[derive(Copy, Clone, Debug)]
crate struct RustcFacts;

impl polonius_engine::FactTypes for RustcFacts {
type Origin = RegionVid;
type Loan = BorrowIndex;
type Point = LocationIndex;
type Variable = Local;
type Path = MovePathIndex;
}

crate type AllFacts = PoloniusFacts<RustcFacts>;

crate trait AllFactsExt {
/// Returns `true` if there is a need to gather `AllFacts` given the
Expand Down Expand Up @@ -55,6 +66,7 @@ impl AllFactsExt for AllFacts {
wr.write_facts_to_path(self.[
borrow_region,
universal_region,
placeholder,
cfg_edge,
killed,
outlives,
Expand All @@ -69,6 +81,7 @@ impl AllFactsExt for AllFacts {
initialized_at,
moved_out_at,
path_accessed_at,
known_subset,
])
}
Ok(())
Expand Down
57 changes: 49 additions & 8 deletions src/librustc_mir/borrow_check/nll/mod.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use crate::borrow_check::borrow_set::BorrowSet;
use crate::borrow_check::location::{LocationIndex, LocationTable};
use crate::borrow_check::location::LocationTable;
use crate::borrow_check::nll::facts::AllFactsExt;
use crate::borrow_check::nll::type_check::{MirTypeckResults, MirTypeckRegionConstraints};
use crate::borrow_check::nll::region_infer::values::RegionValueElements;
use crate::dataflow::indexes::BorrowIndex;
use crate::dataflow::move_paths::{InitLocation, MoveData, MovePathIndex, InitKind};
use crate::dataflow::move_paths::{InitLocation, MoveData, InitKind};
use crate::dataflow::FlowAtLocation;
use crate::dataflow::MaybeInitializedPlaces;
use crate::transform::MirSource;
Expand Down Expand Up @@ -43,10 +42,12 @@ crate mod universal_regions;
crate mod type_check;
crate mod region_infer;

use self::facts::AllFacts;
use self::facts::{AllFacts, RustcFacts};
use self::region_infer::RegionInferenceContext;
use self::universal_regions::UniversalRegions;

crate type PoloniusOutput = Output<RustcFacts>;

/// Rewrites the regions in the MIR to use NLL variables, also
/// scraping out the set of universal regions (e.g., region parameters)
/// declared on the function. That set will need to be given to
Expand Down Expand Up @@ -170,7 +171,7 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>(
errors_buffer: &mut Vec<Diagnostic>,
) -> (
RegionInferenceContext<'tcx>,
Option<Rc<Output<RegionVid, BorrowIndex, LocationIndex, Local, MovePathIndex>>>,
Option<Rc<PoloniusOutput>>,
Option<ClosureRegionRequirements<'tcx>>,
) {
let mut all_facts = if AllFacts::enabled(infcx.tcx) {
Expand Down Expand Up @@ -208,6 +209,39 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>(
.universal_region
.extend(universal_regions.universal_regions());
populate_polonius_move_facts(all_facts, move_data, location_table, &body);

// Emit universal regions facts, and their relations, for Polonius.
//
// 1: universal regions are modeled in Polonius as a pair:
// - the universal region vid itself.
// - a "placeholder loan" associated to this universal region. Since they don't exist in
// the `borrow_set`, their `BorrowIndex` are synthesized as the universal region index
// added to the existing number of loans, as if they succeeded them in the set.
//
let borrow_count = borrow_set.borrows.len();
debug!(
"compute_regions: polonius placeholders, num_universals={}, borrow_count={}",
universal_regions.len(),
borrow_count
);

for universal_region in universal_regions.universal_regions() {
let universal_region_idx = universal_region.index();
let placeholder_loan_idx = borrow_count + universal_region_idx;
all_facts.placeholder.push((universal_region, placeholder_loan_idx.into()));
}

// 2: the universal region relations `outlives` constraints are emitted as
// `known_subset` facts.
for (fr1, fr2) in universal_region_relations.known_outlives() {
if fr1 != fr2 {
debug!(
"compute_regions: emitting polonius `known_subset` fr1={:?}, fr2={:?}",
fr1, fr2
);
all_facts.known_subset.push((*fr1, *fr2));
}
}
}

// Create the region inference context, taking ownership of the
Expand Down Expand Up @@ -269,7 +303,7 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>(

if infcx.tcx.sess.opts.debugging_opts.polonius {
let algorithm = env::var("POLONIUS_ALGORITHM")
.unwrap_or_else(|_| String::from("Hybrid"));
.unwrap_or_else(|_| String::from("Naive"));
let algorithm = Algorithm::from_str(&algorithm).unwrap();
debug!("compute_regions: using polonius algorithm {:?}", algorithm);
Some(Rc::new(Output::compute(
Expand All @@ -283,8 +317,15 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>(
});

// Solve the region constraints.
let closure_region_requirements =
regioncx.solve(infcx, &body, local_names, upvars, def_id, errors_buffer);
let closure_region_requirements = regioncx.solve(
infcx,
&body,
local_names,
upvars,
def_id,
errors_buffer,
polonius_output.clone(),
);

// Dump MIR results into a file, if that is enabled. This let us
// write unit-tests, as well as helping with debugging.
Expand Down
Loading