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

Extend Polonius fact generation for (some) move tracking #62800

Merged
merged 8 commits into from
Sep 5, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
17 changes: 13 additions & 4 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 @@ -58,14 +58,17 @@ impl AllFactsExt for AllFacts {
cfg_edge,
killed,
outlives,
region_live_at,
invalidates,
var_used,
var_defined,
var_drop_used,
var_uses_region,
var_drops_region,
var_initialized_on_exit,
child,
path_belongs_to_var,
initialized_at,
moved_out_at,
path_accessed_at,
])
}
Ok(())
Expand All @@ -84,6 +87,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
87 changes: 84 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,15 @@ 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 +70,85 @@ 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
.path_belongs_to_var
.extend(
move_data
.rev_lookup
.iter_locals_enumerated()
.map(|(v, &m)| (m, v)));

for (child, move_path) in move_data.move_paths.iter_enumerated() {
all_facts
.child
.extend(
move_path
.parents(&move_data.move_paths)
.iter()
.map(|&parent| (child, parent)));
}

// 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)));
}

} 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 +167,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 +203,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
@@ -1,7 +1,7 @@
use crate::borrow_check::nll::region_infer::values::{PointIndex, RegionValueElements};
use crate::util::liveness::{categorize, DefUse};
use rustc::mir::visit::{PlaceContext, Visitor};
use rustc::mir::{Local, Location, Body};
use rustc::mir::{Body, Local, Location};
use rustc_data_structures::indexed_vec::{Idx, IndexVec};
use rustc_data_structures::vec_linked_list as vll;

Expand Down Expand Up @@ -72,16 +72,10 @@ impl LocalUseMap {

let mut locals_with_use_data: IndexVec<Local, bool> =
IndexVec::from_elem_n(false, body.local_decls.len());
live_locals
.iter()
.for_each(|&local| locals_with_use_data[local] = true);

LocalUseMapBuild {
local_use_map: &mut local_use_map,
elements,
locals_with_use_data,
}
.visit_body(body);
live_locals.iter().for_each(|&local| locals_with_use_data[local] = true);

LocalUseMapBuild { local_use_map: &mut local_use_map, elements, locals_with_use_data }
.visit_body(body);

local_use_map
}
Expand Down Expand Up @@ -151,10 +145,8 @@ impl LocalUseMapBuild<'_> {
location: Location,
) {
let point_index = elements.point_from_location(location);
let appearance_index = appearances.push(Appearance {
point_index,
next: *first_appearance,
});
let appearance_index =
appearances.push(Appearance { point_index, next: *first_appearance });
*first_appearance = Some(appearance_index);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/borrow_check/nll/type_check/liveness/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ pub(super) fn generate<'tcx>(
};

if !live_locals.is_empty() {
trace::trace(typeck, body, elements, flow_inits, move_data, live_locals, location_table);
trace::trace(typeck, body, elements, flow_inits, move_data, live_locals);

polonius::populate_var_liveness_facts(typeck, body, location_table);
polonius::populate_access_facts(typeck, body, location_table, move_data);
}
}

Expand Down
67 changes: 56 additions & 11 deletions src/librustc_mir/borrow_check/nll/type_check/liveness/polonius.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,28 @@
use crate::borrow_check::location::{LocationIndex, LocationTable};
use crate::dataflow::indexes::MovePathIndex;
use crate::dataflow::move_paths::{LookupResult, MoveData};
use crate::util::liveness::{categorize, DefUse};
use rustc::mir::visit::{PlaceContext, Visitor};
use rustc::mir::{Body, Local, Location};
use rustc::mir::visit::{MutatingUseContext, PlaceContext, Visitor};
use rustc::mir::{Body, Local, Location, Place};
use rustc::ty::subst::Kind;
use rustc::ty::Ty;

use super::TypeChecker;

type VarPointRelations = Vec<(Local, LocationIndex)>;
type MovePathPointRelations = Vec<(MovePathIndex, LocationIndex)>;

struct LivenessPointFactsExtractor<'me> {
struct UseFactsExtractor<'me> {
var_defined: &'me mut VarPointRelations,
var_used: &'me mut VarPointRelations,
location_table: &'me LocationTable,
var_drop_used: &'me mut VarPointRelations,
move_data: &'me MoveData<'me>,
path_accessed_at: &'me mut MovePathPointRelations,
}

// A Visitor to walk through the MIR and extract point-wise facts
impl LivenessPointFactsExtractor<'_> {
impl UseFactsExtractor<'_> {
fn location_to_index(&self, location: Location) -> LocationIndex {
self.location_table.mid_index(location)
}
Expand All @@ -30,15 +36,50 @@ 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)));
}

fn insert_path_access(&mut self, path: MovePathIndex, location: Location) {
debug!("LivenessFactsExtractor::insert_path_access({:?}, {:?})", path, location);
self.path_accessed_at.push((path, self.location_to_index(location)));
}

fn place_to_mpi(&self, place: &Place<'_>) -> Option<MovePathIndex> {
Copy link
Contributor

Choose a reason for hiding this comment

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

might be nice to have some docs here -- when does this return None?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it is when the place is rooted in a static, though

match self.move_data.rev_lookup.find(place.as_ref()) {
LookupResult::Exact(mpi) => Some(mpi),
LookupResult::Parent(mmpi) => mmpi,
}
}
}

impl Visitor<'tcx> for LivenessPointFactsExtractor<'_> {
impl Visitor<'tcx> for UseFactsExtractor<'_> {
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),
_ => (),
}
}

fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, location: Location) {
self.super_place(place, context, location);
match context {
PlaceContext::NonMutatingUse(_) => {
if let Some(mpi) = self.place_to_mpi(place) {
self.insert_path_access(mpi, location);
}
}

PlaceContext::MutatingUse(MutatingUseContext::Borrow) => {
if let Some(mpi) = self.place_to_mpi(place) {
self.insert_path_access(mpi, location);
}
}
_ => (),
// NOTE: Drop handling is now done in trace()
}
}
}
Expand All @@ -54,23 +95,27 @@ fn add_var_uses_regions(typeck: &mut TypeChecker<'_, 'tcx>, local: Local, ty: Ty
});
}

pub(super) fn populate_var_liveness_facts(
pub(super) fn populate_access_facts(
typeck: &mut TypeChecker<'_, 'tcx>,
mir: &Body<'tcx>,
body: &Body<'tcx>,
location_table: &LocationTable,
move_data: &MoveData<'_>,
) {
debug!("populate_var_liveness_facts()");

if let Some(facts) = typeck.borrowck_context.all_facts.as_mut() {
LivenessPointFactsExtractor {
UseFactsExtractor {
var_defined: &mut facts.var_defined,
var_used: &mut facts.var_used,
var_drop_used: &mut facts.var_drop_used,
path_accessed_at: &mut facts.path_accessed_at,
location_table,
move_data,
}
.visit_body(mir);
.visit_body(body);
}

for (local, local_decl) in mir.local_decls.iter_enumerated() {
for (local, local_decl) in body.local_decls.iter_enumerated() {
add_var_uses_regions(typeck, local, local_decl.ty);
}
}
Expand Down
Loading