Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2324,9 +2324,9 @@ checksum = "676e8eb2b1b4c9043511a9b7bea0915320d7e502b0a079fb03f9635a5252b18c"

[[package]]
name = "polonius-engine"
version = "0.9.0"
version = "0.10.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f6b8a5defa2aef9ba4999aaa745fbc01c622ecea35964a306adc3e44be4f3b5b"
checksum = "50fa9dbfd0d3d60594da338cfe6f94028433eecae4b11b7e83fd99759227bbfe"
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 @@ -21,7 +21,7 @@ scoped-tls = "1.0"
log = { version = "0.4", features = ["release_max_level_info", "std"] }
rustc-rayon = "0.2.0"
rustc-rayon-core = "0.2.0"
polonius-engine = "0.9.0"
polonius-engine = "0.10.0"
rustc_apfloat = { path = "../librustc_apfloat" }
rustc_target = { path = "../librustc_target" }
rustc_macros = { path = "../librustc_macros" }
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 @@ -15,7 +15,7 @@ either = "1.5.0"
dot = { path = "../libgraphviz", package = "graphviz" }
log = "0.4"
log_settings = "0.1.1"
polonius-engine = "0.9.0"
polonius-engine = "0.10.0"
rustc = { path = "../librustc" }
rustc_target = { path = "../librustc_target" }
rustc_data_structures = { path = "../librustc_data_structures" }
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/borrow_check/flows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::borrow_check::location::LocationIndex;
use polonius_engine::Output;

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

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

// (forced to be `pub` due to its use as an associated type below.)
crate struct Flows<'b, 'tcx> {
Expand Down
16 changes: 13 additions & 3 deletions src/librustc_mir/borrow_check/nll/facts.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::borrow_check::location::{LocationIndex, LocationTable};
use crate::dataflow::indexes::BorrowIndex;
use crate::dataflow::indexes::{BorrowIndex, MovePathIndex};
use polonius_engine::AllFacts as PoloniusAllFacts;
use polonius_engine::Atom;
use rustc::mir::Local;
Expand All @@ -11,7 +11,7 @@ use std::fs::{self, File};
use std::io::Write;
use std::path::Path;

crate type AllFacts = PoloniusAllFacts<RegionVid, BorrowIndex, LocationIndex, Local>;
crate type AllFacts = PoloniusAllFacts<RegionVid, BorrowIndex, LocationIndex, Local, MovePathIndex>;

crate trait AllFactsExt {
/// Returns `true` if there is a need to gather `AllFacts` given the
Expand Down Expand Up @@ -65,7 +65,11 @@ impl AllFactsExt for AllFacts {
var_drop_used,
var_uses_region,
var_drops_region,
var_initialized_on_exit,
var_maybe_initialized_on_exit,
parent,
var_starts_path,
initialized_at,
moved_out_at,
])
}
Ok(())
Expand All @@ -84,6 +88,12 @@ impl Atom for LocationIndex {
}
}

impl Atom for MovePathIndex {
fn index(self) -> usize {
Idx::index(self)
}
}

struct FactWriter<'w> {
location_table: &'w LocationTable,
dir: &'w Path,
Expand Down
62 changes: 59 additions & 3 deletions src/librustc_mir/borrow_check/nll/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ 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::MoveData;
use crate::dataflow::move_paths::{InitLocation, MoveData, MovePathIndex, InitKind};
use crate::dataflow::FlowAtLocation;
use crate::dataflow::MaybeInitializedPlaces;
use crate::transform::MirSource;
use crate::borrow_check::Upvar;
use rustc::hir::def_id::DefId;
use rustc::infer::InferCtxt;
use rustc::mir::{ClosureOutlivesSubject, ClosureRegionRequirements, Local, Body, Promoted};
use rustc::mir::{ClosureOutlivesSubject, ClosureRegionRequirements, Local, Location, Body, LocalKind, BasicBlock, Promoted};
use rustc::ty::{self, RegionKind, RegionVid};
use rustc_data_structures::indexed_vec::IndexVec;
use rustc_errors::Diagnostic;
Expand Down Expand Up @@ -69,6 +69,61 @@ pub(in crate::borrow_check) fn replace_regions_in_mir<'cx, 'tcx>(
universal_regions
}


// This function populates an AllFacts instance with base facts related to
// MovePaths and needed for the move analysis.
fn populate_polonius_move_facts(all_facts: &mut AllFacts, move_data: &MoveData<'_>, location_table: &LocationTable, body: &Body<'_>) {
all_facts.var_starts_path.extend(move_data.rev_lookup.iter_locals_enumerated().map(|(v, &m)| (v, m)));

for (idx, move_path) in move_data.move_paths.iter_enumerated() {
all_facts.parent.extend(move_path.parents(&move_data.move_paths).iter().map(|&parent| (parent, idx)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as noted in the polonius PR, ultimately I think we should leave the transitive computation here to polonius. But I guess I don't wnat to do that in this PR.

}

// initialized_at
for init in move_data.inits.iter() {

match init.location {
InitLocation::Statement(location) => {
let block_data = &body[location.block];
let is_terminator = location.statement_index == block_data.statements.len();

if is_terminator && init.kind == InitKind::NonPanicPathOnly {
// We are at the terminator of an init that has a panic path,
// and where the init should not happen on panic

for &successor in block_data.terminator().successors() {
if body[successor].is_cleanup {
continue;
}

// The initialization happened in (or rather, when arriving at)
// the successors, but not in the unwind block.
let first_statement = Location { block: successor, statement_index: 0};
all_facts.initialized_at.push((init.path, location_table.start_index(first_statement)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting quirk. I guess this makes sense.

Copy link
Contributor Author

@amandasystems amandasystems Sep 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just say I scratched my head for quite some time before I figured out why this happened. The results were completely wrong otherwise. If you are curious about the details, there's a ton of logs on Zulip where I figure this out in the initialization topic.

}

} else {
// In all other cases, the initialization just happens at the
// midpoint, like any other effect.
all_facts.initialized_at.push((init.path, location_table.mid_index(location)));
}
},
// Arguments are initialized on function entry
InitLocation::Argument(local) => {
assert!(body.local_kind(local) == LocalKind::Arg);
let fn_entry = Location {block: BasicBlock::from_u32(0u32), statement_index: 0 };
all_facts.initialized_at.push((init.path, location_table.start_index(fn_entry)));

}
}
}


// moved_out_at
// deinitialisation is assumed to always happen!
all_facts.moved_out_at.extend(move_data.moves.iter().map(|mo| (mo.path, location_table.mid_index(mo.source))));
}

/// Computes the (non-lexical) regions from the input MIR.
///
/// This may result in errors being reported.
Expand All @@ -87,7 +142,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>>>,
Option<Rc<Output<RegionVid, BorrowIndex, LocationIndex, Local, MovePathIndex>>>,
Option<ClosureRegionRequirements<'tcx>>,
) {
let mut all_facts = if AllFacts::enabled(infcx.tcx) {
Expand Down Expand Up @@ -123,6 +178,7 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>(
all_facts
.universal_region
.extend(universal_regions.universal_regions());
populate_polonius_move_facts(all_facts, move_data, location_table, body);
}

// Create the region inference context, taking ownership of the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ struct LivenessPointFactsExtractor<'me> {
var_defined: &'me mut VarPointRelations,
var_used: &'me mut VarPointRelations,
location_table: &'me LocationTable,
var_drop_used: &'me mut VarPointRelations,
}

// A Visitor to walk through the MIR and extract point-wise facts
Expand All @@ -30,15 +31,20 @@ impl LivenessPointFactsExtractor<'_> {
debug!("LivenessFactsExtractor::insert_use()");
self.var_used.push((local, self.location_to_index(location)));
}

fn insert_drop_use(&mut self, local: Local, location: Location) {
debug!("LivenessFactsExtractor::insert_drop_use()");
self.var_drop_used.push((local, self.location_to_index(location)));
}
}

impl Visitor<'tcx> for LivenessPointFactsExtractor<'_> {
fn visit_local(&mut self, &local: &Local, context: PlaceContext, location: Location) {
match categorize(context) {
Some(DefUse::Def) => self.insert_def(local, location),
Some(DefUse::Use) => self.insert_use(local, location),
Some(DefUse::Drop) => self.insert_drop_use(local, location),
_ => (),
// NOTE: Drop handling is now done in trace()
}
}
}
Expand All @@ -65,6 +71,7 @@ pub(super) fn populate_var_liveness_facts(
LivenessPointFactsExtractor {
var_defined: &mut facts.var_defined,
var_used: &mut facts.var_used,
var_drop_used: &mut facts.var_drop_used,
location_table,
}
.visit_body(mir);
Expand Down
19 changes: 7 additions & 12 deletions src/librustc_mir/borrow_check/nll/type_check/liveness/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ impl LivenessResults<'me, 'typeck, 'flow, 'tcx> {

// FIXME: this is temporary until we can generate our own initialization
if self.cx.typeck.borrowck_context.all_facts.is_some() {
self.add_polonius_var_initialized_on_exit_for(local)
self.add_polonius_var_maybe_initialized_on_exit_for(local)
}

self.compute_use_live_points_for(local);
Expand All @@ -161,14 +161,14 @@ impl LivenessResults<'me, 'typeck, 'flow, 'tcx> {
//
// FIXME: this analysis (the initialization tracking) should be
// done in Polonius, but isn't yet.
fn add_polonius_var_initialized_on_exit_for(&mut self, local: Local) {
fn add_polonius_var_maybe_initialized_on_exit_for(&mut self, local: Local) {
let move_path = self.cx.move_data.rev_lookup.find_local(local);
let facts = self.cx.typeck.borrowck_context.all_facts.as_mut().unwrap();
for block in self.cx.body.basic_blocks().indices() {
debug!("polonius: generating initialization facts for {:?} in {:?}", local, block);

// iterate through the block, applying the effects of each statement
// up to and including location, and populate `var_initialized_on_exit`
// up to and including location, and populate `var_maybe_initialized_on_exit`
self.cx.flow_inits.reset_to_entry_of(block);
let start_location = Location { block, statement_index: 0 };
self.cx.flow_inits.apply_local_effect(start_location);
Expand All @@ -181,7 +181,7 @@ impl LivenessResults<'me, 'typeck, 'flow, 'tcx> {
// statement has not yet taken effect:
if self.cx.flow_inits.has_any_child_of(move_path).is_some() {
facts
.var_initialized_on_exit
.var_maybe_initialized_on_exit
.push((local, self.cx.location_table.start_index(current_location)));
}

Expand All @@ -190,7 +190,7 @@ impl LivenessResults<'me, 'typeck, 'flow, 'tcx> {

if self.cx.flow_inits.has_any_child_of(move_path).is_some() {
facts
.var_initialized_on_exit
.var_maybe_initialized_on_exit
.push((local, self.cx.location_table.mid_index(current_location)));
}
}
Expand All @@ -199,7 +199,7 @@ impl LivenessResults<'me, 'typeck, 'flow, 'tcx> {

if self.cx.flow_inits.has_any_child_of(move_path).is_some() {
facts
.var_initialized_on_exit
.var_maybe_initialized_on_exit
.push((local, self.cx.location_table.start_index(terminator_location)));
}

Expand All @@ -208,7 +208,7 @@ impl LivenessResults<'me, 'typeck, 'flow, 'tcx> {

if self.cx.flow_inits.has_any_child_of(move_path).is_some() {
facts
.var_initialized_on_exit
.var_maybe_initialized_on_exit
.push((local, self.cx.location_table.mid_index(terminator_location)));
}
}
Expand Down Expand Up @@ -273,11 +273,6 @@ impl LivenessResults<'me, 'typeck, 'flow, 'tcx> {
debug_assert_eq!(self.cx.body.terminator_loc(location.block), location,);

if self.cx.initialized_at_terminator(location.block, mpi) {
// FIXME: this analysis (the initialization tracking) should be
// done in Polonius, but isn't yet.
if let Some(facts) = self.cx.typeck.borrowck_context.all_facts {
facts.var_drop_used.push((local, self.cx.location_table.mid_index(location)));
}
if self.drop_live_at.insert(drop_point) {
self.drop_locations.push(location);
self.stack.push(drop_point);
Expand Down
9 changes: 8 additions & 1 deletion src/librustc_mir/dataflow/move_paths/mod.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use rustc::ty::{Ty, TyCtxt};
use rustc::mir::*;
use rustc::util::nodemap::FxHashMap;
use rustc_data_structures::indexed_vec::{Idx, IndexVec};
use rustc_data_structures::indexed_vec::{Idx, IndexVec, Enumerated};
use smallvec::SmallVec;
use syntax_pos::{Span};
use core::slice::Iter;

use std::fmt;
use std::ops::{Index, IndexMut};
Expand Down Expand Up @@ -262,6 +263,12 @@ impl MovePathLookup {
pub fn find_local(&self, local: Local) -> MovePathIndex {
self.locals[local]
}

/// An enumerated iterator of `local`s and their associated
/// `MovePathIndex`es.
pub fn iter_locals_enumerated(&self) -> Enumerated<Local, Iter<'_, MovePathIndex>> {
self.locals.iter_enumerated()
}
}

#[derive(Debug)]
Expand Down