Auto merge of #54468 - matthewjasper:fix-polonius, r=nikomatsakis

[NLL] 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 (previously these cases have compiled, and more recently ICEd).

Closes #54212
cc #53142 (will need test)

### Known limitations

* Two phase borrows aren't implemented for Polonius yet
* Invalidation facts haven't been updated for some of the recent changes to make `Drop` terminators access fewer things.
* Fact generation is not as optimized as it could be.
* Around 30 tests fail in compare mode, often tests that are ignored in nll compare mode

r? @nikomatsakis

src/librustc_mir/borrow_check/nll/invalidation.rs

@@ -479,7 +479,7 @@ fn check_access_for_conflict(
 
     /// 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));
     }
 }

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 @@
 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<ty::Region<'tcx>>,
     param_env: ty::ParamEnv<'tcx>,
@@ -34,14 +31,12 @@
     category: ConstraintCategory,
     outlives_constraints: &'a mut ConstraintSet,
     type_tests: &'a mut Vec<TypeTest<'tcx>>,
-    all_facts: &'a mut Option<AllFacts>,
 }
 
 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<ty::Region<'tcx>>,
         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<TypeTest<'tcx>>,
-        all_facts: &'a mut Option<AllFacts>,
     ) -> 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 @@ pub(super) fn convert(&mut self, query_constraint: &QueryRegionConstraint<'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) => {

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 fn create(
     infcx: &InferCtxt<'_, '_, 'tcx>,
     param_env: ty::ParamEnv<'tcx>,
-    location_table: &LocationTable,
     implicit_region_bound: Option<ty::Region<'tcx>>,
     universal_regions: &Rc<UniversalRegions<'tcx>>,
     constraints: &mut MirTypeckRegionConstraints<'tcx>,
-    all_facts: &mut Option<AllFacts>,
 ) -> 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 @@ fn non_local_bound(
 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<UniversalRegions<'tcx>>,
     implicit_region_bound: Option<ty::Region<'tcx>>,
     constraints: &'this mut MirTypeckRegionConstraints<'tcx>,
-    all_facts: &'this mut Option<AllFacts>,
 
     // 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);
         }
 

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<Local, Option<_>> = 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.

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<RegionValueElements>,
     flow_inits: &mut FlowAtLocation<MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
     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

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<MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
     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 @@ struct LivenessContext<'me, 'typeck, 'flow, 'gcx, 'tcx>
 
     /// 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 @@ fn add_use_live_facts_for(
     ) {
         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 @@ fn add_drop_live_facts_for(
         // 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 @@ fn make_all_regions_live(
         typeck: &mut TypeChecker<'_, '_, 'tcx>,
         value: impl TypeFoldable<'tcx>,
         live_at: &HybridBitSet<PointIndex>,
+        location_table: &LocationTable,
     ) {
         debug!("make_all_regions_live(value={:?})", value);
         debug!(
@@ -532,8 +551,12 @@ fn make_all_regions_live(
                 .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)));
+                }
             }
         });
     }

src/librustc_mir/borrow_check/nll/type_check/mod.rs

@@ -42,12 +42,13 @@
 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,
-            &region_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,
+        &region_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 @@ fn push_region_constraints(
             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 @@ fn push_region_constraints(
                 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 @@ fn add_reborrow_constraint(
                                 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

src/librustc_mir/borrow_check/nll/type_check/relate_tys.rs

@@ -227,8 +227,6 @@ fn push_outlives(&mut self, sup: ty::Region<'tcx>, sub: ty::Region<'tcx>) {
                     locations: self.locations,
                     category: self.category,
                 });
-
-            // FIXME all facts!
         }
     }
 }

src/test/ui/nll/polonius-smoke-test.rs

+// 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() {}

src/test/ui/nll/polonius-smoke-test.stderr

+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`.