From 60eeed34af9b8a934ed02c7da36764710e466554 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 17 Jan 2019 20:03:58 +0000 Subject: [PATCH 1/3] Include bounds from promoted constants in NLL Previously, a promoted that contains a function item wouldn't have the function items bounds propagated to the main function body. --- .../borrow_check/nll/region_infer/values.rs | 4 +- src/librustc_mir/borrow_check/nll/renumber.rs | 8 ++ .../borrow_check/nll/type_check/mod.rs | 114 +++++++++++++++--- src/test/ui/nll/issue-48697.rs | 4 +- src/test/ui/nll/issue-48697.stderr | 11 ++ src/test/ui/nll/promoted-bounds.rs | 27 +++++ src/test/ui/nll/promoted-bounds.stderr | 15 +++ src/test/ui/nll/promoted-closure-pair.rs | 12 ++ src/test/ui/nll/promoted-closure-pair.stderr | 12 ++ 9 files changed, 185 insertions(+), 22 deletions(-) create mode 100644 src/test/ui/nll/issue-48697.stderr create mode 100644 src/test/ui/nll/promoted-bounds.rs create mode 100644 src/test/ui/nll/promoted-bounds.stderr create mode 100644 src/test/ui/nll/promoted-closure-pair.rs create mode 100644 src/test/ui/nll/promoted-closure-pair.stderr diff --git a/src/librustc_mir/borrow_check/nll/region_infer/values.rs b/src/librustc_mir/borrow_check/nll/region_infer/values.rs index ef27fdbde387f..c4491778162f4 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/values.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/values.rs @@ -154,10 +154,10 @@ impl LivenessValues { /// Creates a new set of "region values" that tracks causal information. /// Each of the regions in num_region_variables will be initialized with an /// empty set of points and no causal information. - crate fn new(elements: &Rc) -> Self { + crate fn new(elements: Rc) -> Self { Self { - elements: elements.clone(), points: SparseBitMatrix::new(elements.num_points), + elements: elements, } } diff --git a/src/librustc_mir/borrow_check/nll/renumber.rs b/src/librustc_mir/borrow_check/nll/renumber.rs index eab9e0ae171ed..0a0a88e694260 100644 --- a/src/librustc_mir/borrow_check/nll/renumber.rs +++ b/src/librustc_mir/borrow_check/nll/renumber.rs @@ -47,6 +47,14 @@ impl<'a, 'gcx, 'tcx> NLLVisitor<'a, 'gcx, 'tcx> { } impl<'a, 'gcx, 'tcx> MutVisitor<'tcx> for NLLVisitor<'a, 'gcx, 'tcx> { + fn visit_mir(&mut self, mir: &mut Mir<'tcx>) { + for promoted in mir.promoted.iter_mut() { + self.visit_mir(promoted); + } + + self.super_mir(mir); + } + fn visit_ty(&mut self, ty: &mut Ty<'tcx>, ty_context: TyContext) { debug!("visit_ty(ty={:?}, ty_context={:?})", ty, ty_context); 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 cf054c2d05505..4929c69d873b4 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -45,7 +45,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::indexed_vec::{IndexVec, Idx}; use rustc::ty::layout::VariantIdx; use std::rc::Rc; -use std::{fmt, iter}; +use std::{fmt, iter, mem}; use syntax_pos::{Span, DUMMY_SP}; macro_rules! span_mirbug { @@ -124,7 +124,7 @@ pub(crate) fn type_check<'gcx, 'tcx>( let mut constraints = MirTypeckRegionConstraints { placeholder_indices: PlaceholderIndices::default(), placeholder_index_to_region: IndexVec::default(), - liveness_constraints: LivenessValues::new(elements), + liveness_constraints: LivenessValues::new(elements.clone()), outlives_constraints: ConstraintSet::default(), closure_bounds_mapping: Default::default(), type_tests: Vec::default(), @@ -253,7 +253,7 @@ enum FieldAccessError { /// is a problem. struct TypeVerifier<'a, 'b: 'a, 'gcx: 'tcx, 'tcx: 'b> { cx: &'a mut TypeChecker<'b, 'gcx, 'tcx>, - mir: &'a Mir<'tcx>, + mir: &'b Mir<'tcx>, last_span: Span, mir_def_id: DefId, errors_reported: bool, @@ -385,7 +385,7 @@ impl<'a, 'b, 'gcx, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'gcx, 'tcx> { } impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { - fn new(cx: &'a mut TypeChecker<'b, 'gcx, 'tcx>, mir: &'a Mir<'tcx>) -> Self { + fn new(cx: &'a mut TypeChecker<'b, 'gcx, 'tcx>, mir: &'b Mir<'tcx>) -> Self { TypeVerifier { mir, mir_def_id: cx.mir_def_id, @@ -454,19 +454,31 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { Place::Base(PlaceBase::Local(index)) => PlaceTy::Ty { ty: self.mir.local_decls[index].ty, }, - Place::Base(PlaceBase::Promoted(box (_index, sty))) => { + Place::Base(PlaceBase::Promoted(box (index, sty))) => { let sty = self.sanitize_type(place, sty); - // FIXME -- promoted MIR return types reference - // various "free regions" (e.g., scopes and things) - // that they ought not to do. We have to figure out - // how best to handle that -- probably we want treat - // promoted MIR much like closures, renumbering all - // their free regions and propagating constraints - // upwards. We have the same acyclic guarantees, so - // that should be possible. But for now, ignore them. - // - // let promoted_mir = &self.mir.promoted[index]; - // promoted_mir.return_ty() + + if !self.errors_reported { + let promoted_mir = &self.mir.promoted[index]; + self.sanitize_promoted(promoted_mir, location); + + let promoted_ty = promoted_mir.return_ty(); + + if let Err(terr) = self.cx.eq_types( + sty, + promoted_ty, + location.to_locations(), + ConstraintCategory::Boring, + ) { + span_mirbug!( + self, + place, + "bad promoted type ({:?}: {:?}): {:?}", + promoted_ty, + sty, + terr + ); + }; + } PlaceTy::Ty { ty: sty } } Place::Base(PlaceBase::Static(box Static { def_id, ty: sty })) => { @@ -533,6 +545,74 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { place_ty } + fn sanitize_promoted(&mut self, promoted_mir: &'b Mir<'tcx>, location: Location) { + // Determine the constraints from the promoted MIR by running the type + // checker on the promoted MIR, then transfer the constraints back to + // the main MIR, changing the locations to the provided location. + + let main_mir = mem::replace(&mut self.mir, promoted_mir); + self.cx.mir = promoted_mir; + + let all_facts = &mut None; + let mut constraints = Default::default(); + let mut closure_bounds = Default::default(); + if let Some(ref mut bcx) = self.cx.borrowck_context { + // Don't try to add borrow_region facts for the promoted MIR + mem::swap(bcx.all_facts, all_facts); + + // Use a new sets of constraints and closure bounds so that we can + // modify their locations. + mem::swap(&mut bcx.constraints.outlives_constraints, &mut constraints); + mem::swap(&mut bcx.constraints.closure_bounds_mapping, &mut closure_bounds); + }; + + self.visit_mir(promoted_mir); + + if !self.errors_reported { + // if verifier failed, don't do further checks to avoid ICEs + self.cx.typeck_mir(promoted_mir); + } + + self.mir = main_mir; + self.cx.mir = main_mir; + // Merge the outlives constraints back in, at the given location. + if let Some(ref mut base_bcx) = self.cx.borrowck_context { + mem::swap(base_bcx.all_facts, all_facts); + mem::swap(&mut base_bcx.constraints.outlives_constraints, &mut constraints); + mem::swap(&mut base_bcx.constraints.closure_bounds_mapping, &mut closure_bounds); + + let locations = location.to_locations(); + for constraint in constraints.iter() { + let mut constraint = *constraint; + constraint.locations = locations; + if let ConstraintCategory::Return + | ConstraintCategory::UseAsConst + | ConstraintCategory::UseAsStatic = constraint.category + { + // "Returning" from a promoted is an assigment to a + // temporary from the user's point of view. + constraint.category = ConstraintCategory::Boring; + } + base_bcx.constraints.outlives_constraints.push(constraint) + } + + if !closure_bounds.is_empty() { + let combined_bounds_mapping = closure_bounds + .into_iter() + .flat_map(|(_, value)| value) + .collect(); + let existing = base_bcx + .constraints + .closure_bounds_mapping + .insert(location, combined_bounds_mapping); + assert!( + existing.is_none(), + "Multiple promoteds/closures at the same location." + ); + } + } + } + fn sanitize_projection( &mut self, base: PlaceTy<'tcx>, @@ -2275,7 +2355,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { ) -> ty::InstantiatedPredicates<'tcx> { if let Some(closure_region_requirements) = tcx.mir_borrowck(def_id).closure_requirements { let closure_constraints = - closure_region_requirements.apply_requirements(tcx, location, def_id, substs); + closure_region_requirements.apply_requirements(tcx, def_id, substs); if let Some(ref mut borrowck_context) = self.borrowck_context { let bounds_mapping = closure_constraints diff --git a/src/test/ui/nll/issue-48697.rs b/src/test/ui/nll/issue-48697.rs index c60c265b813b8..ececd6fccd84b 100644 --- a/src/test/ui/nll/issue-48697.rs +++ b/src/test/ui/nll/issue-48697.rs @@ -1,14 +1,12 @@ // Regression test for #48697 -// compile-pass - #![feature(nll)] fn foo(x: &i32) -> &i32 { let z = 4; let f = &|y| y; let k = f(&z); - f(x) + f(x) //~ cannot return value referencing local variable } fn main() {} diff --git a/src/test/ui/nll/issue-48697.stderr b/src/test/ui/nll/issue-48697.stderr new file mode 100644 index 0000000000000..16b46c15b90b6 --- /dev/null +++ b/src/test/ui/nll/issue-48697.stderr @@ -0,0 +1,11 @@ +error[E0515]: cannot return value referencing local variable `z` + --> $DIR/issue-48697.rs:9:5 + | +LL | let k = f(&z); + | -- `z` is borrowed here +LL | f(x) //~ cannot return value referencing local variable + | ^^^^ returns a value referencing data owned by the current function + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0515`. diff --git a/src/test/ui/nll/promoted-bounds.rs b/src/test/ui/nll/promoted-bounds.rs new file mode 100644 index 0000000000000..59b21cf9ac2a9 --- /dev/null +++ b/src/test/ui/nll/promoted-bounds.rs @@ -0,0 +1,27 @@ +#![feature(nll)] + +fn shorten_lifetime<'a, 'b, 'min>(a: &'a i32, b: &'b i32) -> &'min i32 +where + 'a: 'min, + 'b: 'min, +{ + if *a < *b { + &a + } else { + &b + } +} + +fn main() { + let promoted_fn_item_ref = &shorten_lifetime; + + let a = &5; + let ptr = { + let l = 3; + let b = &l; //~ ERROR does not live long enough + let c = promoted_fn_item_ref(a, b); + c + }; + + println!("ptr = {:?}", ptr); +} diff --git a/src/test/ui/nll/promoted-bounds.stderr b/src/test/ui/nll/promoted-bounds.stderr new file mode 100644 index 0000000000000..9798f238fc4d0 --- /dev/null +++ b/src/test/ui/nll/promoted-bounds.stderr @@ -0,0 +1,15 @@ +error[E0597]: `l` does not live long enough + --> $DIR/promoted-bounds.rs:21:17 + | +LL | let ptr = { + | --- borrow later stored here +LL | let l = 3; +LL | let b = &l; //~ ERROR does not live long enough + | ^^ borrowed value does not live long enough +... +LL | }; + | - `l` dropped here while still borrowed + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/promoted-closure-pair.rs b/src/test/ui/nll/promoted-closure-pair.rs new file mode 100644 index 0000000000000..7b3bbad4c1e0c --- /dev/null +++ b/src/test/ui/nll/promoted-closure-pair.rs @@ -0,0 +1,12 @@ +// Check that we handle multiple closures in the same promoted constant. + +#![feature(nll)] + +fn foo() -> &'static i32 { + let z = 0; + let p = &(|y| y, |y| y); + p.0(&z); + p.1(&z) //~ ERROR cannot return +} + +fn main() {} diff --git a/src/test/ui/nll/promoted-closure-pair.stderr b/src/test/ui/nll/promoted-closure-pair.stderr new file mode 100644 index 0000000000000..5f4a6037668b5 --- /dev/null +++ b/src/test/ui/nll/promoted-closure-pair.stderr @@ -0,0 +1,12 @@ +error[E0515]: cannot return value referencing local variable `z` + --> $DIR/promoted-closure-pair.rs:9:5 + | +LL | p.1(&z) //~ ERROR cannot return + | ^^^^--^ + | | | + | | `z` is borrowed here + | returns a value referencing data owned by the current function + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0515`. From 848c25207208d3ad71e6d4fd3719cc7dc725bdf4 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 17 Jan 2019 20:04:09 +0000 Subject: [PATCH 2/3] Remove unnecessary parameter --- src/librustc_mir/borrow_check/nll/region_infer/mod.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index 785f810d94159..f0d3a0d2986e5 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -1357,7 +1357,6 @@ pub trait ClosureRegionRequirementsExt<'gcx, 'tcx> { fn apply_requirements( &self, tcx: TyCtxt<'_, 'gcx, 'tcx>, - location: Location, closure_def_id: DefId, closure_substs: SubstsRef<'tcx>, ) -> Vec>; @@ -1388,13 +1387,12 @@ impl<'gcx, 'tcx> ClosureRegionRequirementsExt<'gcx, 'tcx> for ClosureRegionRequi fn apply_requirements( &self, tcx: TyCtxt<'_, 'gcx, 'tcx>, - location: Location, closure_def_id: DefId, closure_substs: SubstsRef<'tcx>, ) -> Vec> { debug!( - "apply_requirements(location={:?}, closure_def_id={:?}, closure_substs={:?})", - location, closure_def_id, closure_substs + "apply_requirements(closure_def_id={:?}, closure_substs={:?})", + closure_def_id, closure_substs ); // Extract the values of the free regions in `closure_substs` From 3b93d71fd200d5f2f39f1c4a7a4b09cb19ff64be Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Wed, 27 Feb 2019 22:21:49 +0000 Subject: [PATCH 3/3] Handle type annotations in promoted MIR correctly Type annotations are shared between the MIR of a function and the promoted constants for that function, so keep them in the type checker when we check the promoted MIR. --- .../borrow_check/nll/type_check/mod.rs | 32 ++++++++++--------- .../user-annotations/promoted-annotation.rs | 12 +++++++ .../promoted-annotation.stderr | 17 ++++++++++ 3 files changed, 46 insertions(+), 15 deletions(-) create mode 100644 src/test/ui/nll/user-annotations/promoted-annotation.rs create mode 100644 src/test/ui/nll/user-annotations/promoted-annotation.stderr 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 4929c69d873b4..9fa3bf5524739 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -39,7 +39,8 @@ use rustc::ty::fold::TypeFoldable; use rustc::ty::subst::{Subst, SubstsRef, UnpackedKind, UserSubsts}; use rustc::ty::{ self, RegionVid, ToPolyTraitRef, Ty, TyCtxt, TyKind, UserType, - CanonicalUserTypeAnnotation, UserTypeAnnotationIndex, + CanonicalUserTypeAnnotation, CanonicalUserTypeAnnotations, + UserTypeAnnotationIndex, }; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::indexed_vec::{IndexVec, Idx}; @@ -283,7 +284,7 @@ impl<'a, 'b, 'gcx, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'gcx, 'tcx> { location.to_locations(), ConstraintCategory::Boring, ) { - let annotation = &self.mir.user_type_annotations[annotation_index]; + let annotation = &self.cx.user_type_annotations[annotation_index]; span_mirbug!( self, constant, @@ -550,8 +551,7 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { // checker on the promoted MIR, then transfer the constraints back to // the main MIR, changing the locations to the provided location. - let main_mir = mem::replace(&mut self.mir, promoted_mir); - self.cx.mir = promoted_mir; + let parent_mir = mem::replace(&mut self.mir, promoted_mir); let all_facts = &mut None; let mut constraints = Default::default(); @@ -573,8 +573,7 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { self.cx.typeck_mir(promoted_mir); } - self.mir = main_mir; - self.cx.mir = main_mir; + self.mir = parent_mir; // Merge the outlives constraints back in, at the given location. if let Some(ref mut base_bcx) = self.cx.borrowck_context { mem::swap(base_bcx.all_facts, all_facts); @@ -818,7 +817,9 @@ struct TypeChecker<'a, 'gcx: 'tcx, 'tcx: 'a> { infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, param_env: ty::ParamEnv<'gcx>, last_span: Span, - mir: &'a Mir<'tcx>, + /// User type annotations are shared between the main MIR and the MIR of + /// all of the promoted items. + user_type_annotations: &'a CanonicalUserTypeAnnotations<'tcx>, mir_def_id: DefId, region_bound_pairs: &'a RegionBoundPairs<'tcx>, implicit_region_bound: Option>, @@ -973,8 +974,8 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { let mut checker = Self { infcx, last_span: DUMMY_SP, - mir, mir_def_id, + user_type_annotations: &mir.user_type_annotations, param_env, region_bound_pairs, implicit_region_bound, @@ -990,9 +991,9 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { fn check_user_type_annotations(&mut self) { debug!( "check_user_type_annotations: user_type_annotations={:?}", - self.mir.user_type_annotations + self.user_type_annotations ); - for user_annotation in &self.mir.user_type_annotations { + for user_annotation in self.user_type_annotations { let CanonicalUserTypeAnnotation { span, ref user_ty, inferred_ty } = *user_annotation; let (annotation, _) = self.infcx.instantiate_canonical_with_fresh_inference_vars( span, user_ty @@ -1175,7 +1176,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { a, v, user_ty, locations, ); - let annotated_type = self.mir.user_type_annotations[user_ty.base].inferred_ty; + let annotated_type = self.user_type_annotations[user_ty.base].inferred_ty; let mut curr_projected_ty = PlaceTy::from_ty(annotated_type); let tcx = self.infcx.tcx; @@ -1361,7 +1362,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { location.to_locations(), ConstraintCategory::Boring, ) { - let annotation = &mir.user_type_annotations[annotation_index]; + let annotation = &self.user_type_annotations[annotation_index]; span_mirbug!( self, stmt, @@ -1420,7 +1421,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { Locations::All(stmt.source_info.span), ConstraintCategory::TypeAnnotation, ) { - let annotation = &mir.user_type_annotations[projection.base]; + let annotation = &self.user_type_annotations[projection.base]; span_mirbug!( self, stmt, @@ -2078,7 +2079,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } Rvalue::Ref(region, _borrow_kind, borrowed_place) => { - self.add_reborrow_constraint(location, region, borrowed_place); + self.add_reborrow_constraint(mir, location, region, borrowed_place); } // FIXME: These other cases have to be implemented in future PRs @@ -2177,6 +2178,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { /// - `borrowed_place`: the place `P` being borrowed fn add_reborrow_constraint( &mut self, + mir: &Mir<'tcx>, location: Location, borrow_region: ty::Region<'tcx>, borrowed_place: &Place<'tcx>, @@ -2226,7 +2228,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { match *elem { ProjectionElem::Deref => { let tcx = self.infcx.tcx; - let base_ty = base.ty(self.mir, tcx).to_ty(tcx); + let base_ty = base.ty(mir, tcx).to_ty(tcx); debug!("add_reborrow_constraint - base_ty = {:?}", base_ty); match base_ty.sty { diff --git a/src/test/ui/nll/user-annotations/promoted-annotation.rs b/src/test/ui/nll/user-annotations/promoted-annotation.rs new file mode 100644 index 0000000000000..fa2d2fb81183d --- /dev/null +++ b/src/test/ui/nll/user-annotations/promoted-annotation.rs @@ -0,0 +1,12 @@ +// Test that type annotations are checked in promoted constants correctly. + +#![feature(nll)] + +fn foo<'a>() { + let x = 0; + let f = &drop::<&'a i32>; + f(&x); + //~^ ERROR `x` does not live long enough +} + +fn main() {} diff --git a/src/test/ui/nll/user-annotations/promoted-annotation.stderr b/src/test/ui/nll/user-annotations/promoted-annotation.stderr new file mode 100644 index 0000000000000..144af1e0ec120 --- /dev/null +++ b/src/test/ui/nll/user-annotations/promoted-annotation.stderr @@ -0,0 +1,17 @@ +error[E0597]: `x` does not live long enough + --> $DIR/promoted-annotation.rs:8:7 + | +LL | fn foo<'a>() { + | -- lifetime `'a` defined here +LL | let x = 0; +LL | let f = &drop::<&'a i32>; + | ---------------- assignment requires that `x` is borrowed for `'a` +LL | f(&x); + | ^^ borrowed value does not live long enough +LL | //~^ ERROR `x` does not live long enough +LL | } + | - `x` dropped here while still borrowed + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`.