From 610903fb118cb7cbf1474bd6cf11ae8afa380c4e Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Wed, 26 Sep 2018 20:41:14 +0100 Subject: [PATCH] Get Polonius borrow check to work in simple cases * Restores the generation of outlives facts from subtyping. * Restore liveness facts. * Generate invalidates facts at the start point of each location, where we check for errors. * Add a small test for simple cases. --- .../borrow_check/nll/invalidation.rs | 2 +- .../nll/type_check/constraint_conversion.rs | 25 ------ .../nll/type_check/free_region_relations.rs | 10 --- .../nll/type_check/liveness/liveness_map.rs | 4 +- .../nll/type_check/liveness/mod.rs | 4 +- .../nll/type_check/liveness/trace.rs | 31 ++++++- .../borrow_check/nll/type_check/mod.rs | 82 +++++++++++-------- .../borrow_check/nll/type_check/relate_tys.rs | 2 - src/test/ui/nll/polonius-smoke-test.rs | 47 +++++++++++ src/test/ui/nll/polonius-smoke-test.stderr | 45 ++++++++++ 10 files changed, 172 insertions(+), 80 deletions(-) create mode 100644 src/test/ui/nll/polonius-smoke-test.rs create mode 100644 src/test/ui/nll/polonius-smoke-test.stderr diff --git a/src/librustc_mir/borrow_check/nll/invalidation.rs b/src/librustc_mir/borrow_check/nll/invalidation.rs index 1246f7120c4fb..fff0f70924ac8 100644 --- a/src/librustc_mir/borrow_check/nll/invalidation.rs +++ b/src/librustc_mir/borrow_check/nll/invalidation.rs @@ -479,7 +479,7 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cx, 'tcx, 'gcx> { /// Generate a new invalidates(L, B) fact fn generate_invalidates(&mut self, b: BorrowIndex, l: Location) { - let lidx = self.location_table.mid_index(l); + let lidx = self.location_table.start_index(l); self.all_facts.invalidates.push((lidx, b)); } } diff --git a/src/librustc_mir/borrow_check/nll/type_check/constraint_conversion.rs b/src/librustc_mir/borrow_check/nll/type_check/constraint_conversion.rs index 0ad6183960da9..dabf669eca65f 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/constraint_conversion.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/constraint_conversion.rs @@ -8,9 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use borrow_check::location::LocationTable; use borrow_check::nll::constraints::{ConstraintCategory, ConstraintSet, OutlivesConstraint}; -use borrow_check::nll::facts::AllFacts; use borrow_check::nll::region_infer::TypeTest; use borrow_check::nll::type_check::Locations; use borrow_check::nll::universal_regions::UniversalRegions; @@ -26,7 +24,6 @@ use syntax_pos::DUMMY_SP; crate struct ConstraintConversion<'a, 'gcx: 'tcx, 'tcx: 'a> { tcx: TyCtxt<'a, 'gcx, 'tcx>, universal_regions: &'a UniversalRegions<'tcx>, - location_table: &'a LocationTable, region_bound_pairs: &'a RegionBoundPairs<'tcx>, implicit_region_bound: Option>, param_env: ty::ParamEnv<'tcx>, @@ -34,14 +31,12 @@ crate struct ConstraintConversion<'a, 'gcx: 'tcx, 'tcx: 'a> { category: ConstraintCategory, outlives_constraints: &'a mut ConstraintSet, type_tests: &'a mut Vec>, - all_facts: &'a mut Option, } impl<'a, 'gcx, 'tcx> ConstraintConversion<'a, 'gcx, 'tcx> { crate fn new( tcx: TyCtxt<'a, 'gcx, 'tcx>, universal_regions: &'a UniversalRegions<'tcx>, - location_table: &'a LocationTable, region_bound_pairs: &'a RegionBoundPairs<'tcx>, implicit_region_bound: Option>, param_env: ty::ParamEnv<'tcx>, @@ -49,12 +44,10 @@ impl<'a, 'gcx, 'tcx> ConstraintConversion<'a, 'gcx, 'tcx> { category: ConstraintCategory, outlives_constraints: &'a mut ConstraintSet, type_tests: &'a mut Vec>, - all_facts: &'a mut Option, ) -> Self { Self { tcx, universal_regions, - location_table, region_bound_pairs, implicit_region_bound, param_env, @@ -62,7 +55,6 @@ impl<'a, 'gcx, 'tcx> ConstraintConversion<'a, 'gcx, 'tcx> { category, outlives_constraints, type_tests, - all_facts, } } @@ -101,23 +93,6 @@ impl<'a, 'gcx, 'tcx> ConstraintConversion<'a, 'gcx, 'tcx> { let r1_vid = self.to_region_vid(r1); let r2_vid = self.to_region_vid(r2); self.add_outlives(r1_vid, r2_vid); - - // In the new analysis, all outlives relations etc - // "take effect" at the mid point of the statement - // that requires them, so ignore the `at_location`. - if let Some(all_facts) = &mut self.all_facts { - if let Some(from_location) = self.locations.from_location() { - all_facts.outlives.push(( - r1_vid, - r2_vid, - self.location_table.mid_index(from_location), - )); - } else { - for location in self.location_table.all_points() { - all_facts.outlives.push((r1_vid, r2_vid, location)); - } - } - } } UnpackedKind::Type(t1) => { diff --git a/src/librustc_mir/borrow_check/nll/type_check/free_region_relations.rs b/src/librustc_mir/borrow_check/nll/type_check/free_region_relations.rs index 61c612b3c0118..f33909db78f9f 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/free_region_relations.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/free_region_relations.rs @@ -8,8 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use borrow_check::location::LocationTable; -use borrow_check::nll::facts::AllFacts; use borrow_check::nll::type_check::constraint_conversion; use borrow_check::nll::type_check::{Locations, MirTypeckRegionConstraints}; use borrow_check::nll::universal_regions::UniversalRegions; @@ -69,19 +67,15 @@ crate struct CreateResult<'tcx> { crate fn create( infcx: &InferCtxt<'_, '_, 'tcx>, param_env: ty::ParamEnv<'tcx>, - location_table: &LocationTable, implicit_region_bound: Option>, universal_regions: &Rc>, constraints: &mut MirTypeckRegionConstraints<'tcx>, - all_facts: &mut Option, ) -> CreateResult<'tcx> { UniversalRegionRelationsBuilder { infcx, param_env, implicit_region_bound, constraints, - location_table, - all_facts, universal_regions: universal_regions.clone(), region_bound_pairs: Vec::new(), relations: UniversalRegionRelations { @@ -210,11 +204,9 @@ impl UniversalRegionRelations<'tcx> { struct UniversalRegionRelationsBuilder<'this, 'gcx: 'tcx, 'tcx: 'this> { infcx: &'this InferCtxt<'this, 'gcx, 'tcx>, param_env: ty::ParamEnv<'tcx>, - location_table: &'this LocationTable, universal_regions: Rc>, implicit_region_bound: Option>, constraints: &'this mut MirTypeckRegionConstraints<'tcx>, - all_facts: &'this mut Option, // outputs: relations: UniversalRegionRelations<'tcx>, @@ -281,7 +273,6 @@ impl UniversalRegionRelationsBuilder<'cx, 'gcx, 'tcx> { constraint_conversion::ConstraintConversion::new( self.infcx.tcx, &self.universal_regions, - &self.location_table, &self.region_bound_pairs, self.implicit_region_bound, self.param_env, @@ -289,7 +280,6 @@ impl UniversalRegionRelationsBuilder<'cx, 'gcx, 'tcx> { ConstraintCategory::Internal, &mut self.constraints.outlives_constraints, &mut self.constraints.type_tests, - &mut self.all_facts, ).convert_all(&data); } diff --git a/src/librustc_mir/borrow_check/nll/type_check/liveness/liveness_map.rs b/src/librustc_mir/borrow_check/nll/type_check/liveness/liveness_map.rs index 467554dc38a67..cc176cbc40392 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/liveness/liveness_map.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/liveness/liveness_map.rs @@ -17,6 +17,7 @@ //! types, instead of all variables. use borrow_check::nll::ToRegionVid; +use borrow_check::nll::facts::{AllFacts, AllFactsExt}; use rustc::mir::{Local, Mir}; use rustc::ty::{RegionVid, TyCtxt}; use rustc_data_structures::fx::FxHashSet; @@ -61,12 +62,13 @@ impl NllLivenessMap { mir: &Mir<'tcx>, ) -> Self { let mut to_local = IndexVec::default(); + let facts_enabled = AllFacts::enabled(tcx); let from_local: IndexVec> = mir.local_decls .iter_enumerated() .map(|(local, local_decl)| { if tcx.all_free_regions_meet(&local_decl.ty, |r| { free_regions.contains(&r.to_region_vid()) - }) { + }) && !facts_enabled { // If all the regions in the type are free regions // (or there are no regions), then we don't need // to track liveness for this variable. diff --git a/src/librustc_mir/borrow_check/nll/type_check/liveness/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/liveness/mod.rs index 357e9ee72102a..9ccdc84db1561 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/liveness/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/liveness/mod.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use borrow_check::location::LocationTable; use borrow_check::nll::region_infer::values::RegionValueElements; use borrow_check::nll::constraints::ConstraintSet; use borrow_check::nll::NllLivenessMap; @@ -40,6 +41,7 @@ pub(super) fn generate<'gcx, 'tcx>( elements: &Rc, flow_inits: &mut FlowAtLocation>, move_data: &MoveData<'tcx>, + location_table: &LocationTable, ) { debug!("liveness::generate"); let free_regions = { @@ -51,7 +53,7 @@ pub(super) fn generate<'gcx, 'tcx>( ) }; let liveness_map = NllLivenessMap::compute(typeck.tcx(), &free_regions, mir); - trace::trace(typeck, mir, elements, flow_inits, move_data, &liveness_map); + trace::trace(typeck, mir, elements, flow_inits, move_data, &liveness_map, location_table); } /// Compute all regions that are (currently) known to outlive free diff --git a/src/librustc_mir/borrow_check/nll/type_check/liveness/trace.rs b/src/librustc_mir/borrow_check/nll/type_check/liveness/trace.rs index e706c1adaddfb..6c1252fc73d8b 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/liveness/trace.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/liveness/trace.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use borrow_check::location::LocationTable; use borrow_check::nll::constraints::ConstraintCategory; use borrow_check::nll::region_infer::values::{self, PointIndex, RegionValueElements}; use borrow_check::nll::type_check::liveness::liveness_map::{LiveVar, NllLivenessMap}; @@ -49,6 +50,7 @@ pub(super) fn trace( flow_inits: &mut FlowAtLocation>, move_data: &MoveData<'tcx>, liveness_map: &NllLivenessMap, + location_table: &LocationTable, ) { debug!("trace()"); @@ -67,6 +69,7 @@ pub(super) fn trace( move_data, liveness_map, drop_data: FxHashMap::default(), + location_table, }; LivenessResults::new(cx).compute_for_all_locals(); @@ -105,6 +108,9 @@ where /// Map tracking which variables need liveness computation. liveness_map: &'me NllLivenessMap, + + /// Maps between a MIR Location and a LocationIndex + location_table: &'me LocationTable, } struct DropData<'tcx> { @@ -453,7 +459,13 @@ impl LivenessContext<'_, '_, '_, '_, 'tcx> { ) { debug!("add_use_live_facts_for(value={:?})", value); - Self::make_all_regions_live(self.elements, &mut self.typeck, value, live_at) + Self::make_all_regions_live( + self.elements, + &mut self.typeck, + value, + live_at, + self.location_table, + ) } /// Some variable with type `live_ty` is "drop live" at `location` @@ -505,7 +517,13 @@ impl LivenessContext<'_, '_, '_, '_, 'tcx> { // All things in the `outlives` array may be touched by // the destructor and must be live at this point. for &kind in &drop_data.dropck_result.kinds { - Self::make_all_regions_live(self.elements, &mut self.typeck, kind, live_at); + Self::make_all_regions_live( + self.elements, + &mut self.typeck, + kind, + live_at, + self.location_table, + ); } } @@ -514,6 +532,7 @@ impl LivenessContext<'_, '_, '_, '_, 'tcx> { typeck: &mut TypeChecker<'_, '_, 'tcx>, value: impl TypeFoldable<'tcx>, live_at: &HybridBitSet, + location_table: &LocationTable, ) { debug!("make_all_regions_live(value={:?})", value); debug!( @@ -532,8 +551,12 @@ impl LivenessContext<'_, '_, '_, '_, 'tcx> { .liveness_constraints .add_elements(live_region_vid, live_at); - if let Some(_) = borrowck_context.all_facts { - bug!("polonius liveness facts not implemented yet") + if let Some(facts) = borrowck_context.all_facts { + for point in live_at.iter() { + let loc = elements.to_location(point); + facts.region_live_at.push((live_region_vid, location_table.start_index(loc))); + facts.region_live_at.push((live_region_vid, location_table.mid_index(loc))); + } } }); } diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index 5ebaec1874aef..4c2ef58a3a783 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -42,12 +42,13 @@ use rustc::traits::{ObligationCause, PredicateObligations}; use rustc::ty::fold::TypeFoldable; use rustc::ty::subst::Subst; use rustc::ty::{self, CanonicalTy, RegionVid, ToPolyTraitRef, Ty, TyCtxt, TyKind}; -use std::fmt; +use std::{fmt, iter}; use std::rc::Rc; use syntax_pos::{Span, DUMMY_SP}; use transform::{MirPass, MirSource}; use rustc_data_structures::fx::FxHashSet; +use either::Either; macro_rules! span_mirbug { ($context:expr, $elem:expr, $($message:tt)*) => ({ @@ -135,37 +136,35 @@ pub(crate) fn type_check<'gcx, 'tcx>( } = free_region_relations::create( infcx, param_env, - location_table, Some(implicit_region_bound), universal_regions, &mut constraints, - all_facts, ); - { - let mut borrowck_context = BorrowCheckContext { - universal_regions, - location_table, - borrow_set, - all_facts, - constraints: &mut constraints, - }; + let mut borrowck_context = BorrowCheckContext { + universal_regions, + location_table, + borrow_set, + all_facts, + constraints: &mut constraints, + }; - type_check_internal( - infcx, - mir_def_id, - param_env, - mir, - ®ion_bound_pairs, - Some(implicit_region_bound), - Some(&mut borrowck_context), - Some(&universal_region_relations), - |cx| { - cx.equate_inputs_and_outputs(mir, universal_regions, &normalized_inputs_and_output); - liveness::generate(cx, mir, elements, flow_inits, move_data); - }, - ); - } + type_check_internal( + infcx, + mir_def_id, + param_env, + mir, + ®ion_bound_pairs, + Some(implicit_region_bound), + Some(&mut borrowck_context), + Some(&universal_region_relations), + |cx| { + cx.equate_inputs_and_outputs(mir, universal_regions, &normalized_inputs_and_output); + liveness::generate(cx, mir, elements, flow_inits, move_data, location_table); + + cx.borrowck_context.as_mut().map(|bcx| translate_outlives_facts(bcx)); + }, + ); MirTypeckResults { constraints, @@ -208,6 +207,27 @@ fn type_check_internal<'a, 'gcx, 'tcx, R>( extra(&mut checker) } +fn translate_outlives_facts(cx: &mut BorrowCheckContext) { + if let Some(facts) = cx.all_facts { + let location_table = cx.location_table; + facts.outlives.extend( + cx.constraints.outlives_constraints.iter().flat_map(|constraint: &OutlivesConstraint| { + if let Some(from_location) = constraint.locations.from_location() { + Either::Left(iter::once(( + constraint.sup, + constraint.sub, + location_table.mid_index(from_location), + ))) + } else { + Either::Right(location_table.all_points().map(move |location| { + (constraint.sup, constraint.sub, location) + })) + } + }) + ); + } +} + fn mirbug(tcx: TyCtxt, span: Span, msg: &str) { // We sometimes see MIR failures (notably predicate failures) due to // the fact that we check rvalue sized predicates here. So use `delay_span_bug` @@ -853,7 +873,6 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { constraint_conversion::ConstraintConversion::new( self.infcx.tcx, borrowck_context.universal_regions, - borrowck_context.location_table, self.region_bound_pairs, self.implicit_region_bound, self.param_env, @@ -861,7 +880,6 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { category, &mut borrowck_context.constraints.outlives_constraints, &mut borrowck_context.constraints.type_tests, - &mut borrowck_context.all_facts, ).convert_all(&data); } } @@ -1921,14 +1939,6 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { category: ConstraintCategory::Boring, }); - if let Some(all_facts) = all_facts { - all_facts.outlives.push(( - ref_region.to_region_vid(), - borrow_region.to_region_vid(), - location_table.mid_index(location), - )); - } - match mutbl { hir::Mutability::MutImmutable => { // Immutable reference. We don't need the base diff --git a/src/librustc_mir/borrow_check/nll/type_check/relate_tys.rs b/src/librustc_mir/borrow_check/nll/type_check/relate_tys.rs index 39e3403136360..13d59c3ba29c6 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/relate_tys.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/relate_tys.rs @@ -227,8 +227,6 @@ impl TypeRelatingDelegate<'tcx> for NllTypeRelatingDelegate<'_, '_, '_, 'tcx> { locations: self.locations, category: self.category, }); - - // FIXME all facts! } } } diff --git a/src/test/ui/nll/polonius-smoke-test.rs b/src/test/ui/nll/polonius-smoke-test.rs new file mode 100644 index 0000000000000..bea5e4559988e --- /dev/null +++ b/src/test/ui/nll/polonius-smoke-test.rs @@ -0,0 +1,47 @@ +// Check that Polonius borrow check works for simple cases. +// ignore-compare-mode-nll +// compile-flags: -Z borrowck=mir -Zpolonius + +pub fn return_ref_to_local() -> &'static i32 { + let x = 0; + &x //~ ERROR +} + +pub fn use_while_mut() { + let mut x = 0; + let y = &mut x; + let z = x; //~ ERROR + let w = y; +} + +pub fn use_while_mut_fr(x: &mut i32) -> &mut i32 { + let y = &mut *x; + let z = x; //~ ERROR + y +} + +// Cases like this are why we have Polonius. +pub fn position_dependent_outlives(x: &mut i32, cond: bool) -> &mut i32 { + let y = &mut *x; + if cond { + return y; + } else { + *x = 0; + return x; + } +} + +fn foo<'a, 'b>(p: &'b &'a mut usize) -> &'b usize { + p +} + +// Check that we create constraints for well-formedness of function arguments +fn well_formed_function_inputs() { + let s = &mut 1; + let r = &mut *s; + let tmp = foo(&r); + s; //~ ERROR + tmp; +} + +fn main() {} diff --git a/src/test/ui/nll/polonius-smoke-test.stderr b/src/test/ui/nll/polonius-smoke-test.stderr new file mode 100644 index 0000000000000..a30d522f3ff07 --- /dev/null +++ b/src/test/ui/nll/polonius-smoke-test.stderr @@ -0,0 +1,45 @@ +error[E0597]: `x` does not live long enough + --> $DIR/polonius-smoke-test.rs:7:5 + | +LL | &x //~ ERROR + | ^^ borrowed value does not live long enough +LL | } + | - `x` dropped here while still borrowed + | + = note: borrowed value must be valid for the static lifetime... + +error[E0503]: cannot use `x` because it was mutably borrowed + --> $DIR/polonius-smoke-test.rs:13:13 + | +LL | let y = &mut x; + | ------ borrow of `x` occurs here +LL | let z = x; //~ ERROR + | ^ use of borrowed `x` +LL | let w = y; + | - borrow later used here + +error[E0505]: cannot move out of `x` because it is borrowed + --> $DIR/polonius-smoke-test.rs:19:13 + | +LL | let y = &mut *x; + | ------- borrow of `*x` occurs here +LL | let z = x; //~ ERROR + | ^ move out of `x` occurs here +LL | y + | - borrow later used here + +error[E0505]: cannot move out of `s` because it is borrowed + --> $DIR/polonius-smoke-test.rs:43:5 + | +LL | let r = &mut *s; + | ------- borrow of `*s` occurs here +LL | let tmp = foo(&r); +LL | s; //~ ERROR + | ^ move out of `s` occurs here +LL | tmp; + | --- borrow later used here + +error: aborting due to 4 previous errors + +Some errors occurred: E0503, E0505, E0597. +For more information about an error, try `rustc --explain E0503`.