Skip to content

Commit

Permalink
Auto merge of #57202 - matthewjasper:nll-typeck-promoteds, r=pnkfelix
Browse files Browse the repository at this point in the history
Include bounds from promoted constants in NLL

Previously a promoted function wouldn't have its bound propagated out to
the main function body.

When we visit a promoted, we now type check the MIR of the promoted
and transfer any lifetime constraints to back to the main function's MIR.

Fixes #57170

r? @nikomatsakis
  • Loading branch information
bors committed Mar 2, 2019
2 parents d987b46 + 3b93d71 commit a21d824
Show file tree
Hide file tree
Showing 12 changed files with 229 additions and 37 deletions.
6 changes: 2 additions & 4 deletions src/librustc_mir/borrow_check/nll/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<QueryRegionConstraint<'tcx>>;
Expand Down Expand Up @@ -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<QueryRegionConstraint<'tcx>> {
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`
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/borrow_check/nll/region_infer/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,10 @@ impl<N: Idx> LivenessValues<N> {
/// 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<RegionValueElements>) -> Self {
crate fn new(elements: Rc<RegionValueElements>) -> Self {
Self {
elements: elements.clone(),
points: SparseBitMatrix::new(elements.num_points),
elements: elements,
}
}

Expand Down
8 changes: 8 additions & 0 deletions src/librustc_mir/borrow_check/nll/renumber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
138 changes: 110 additions & 28 deletions src/librustc_mir/borrow_check/nll/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,14 @@ 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};
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 {
Expand Down Expand Up @@ -124,7 +125,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(),
Expand Down Expand Up @@ -253,7 +254,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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -385,7 +386,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,
Expand Down Expand Up @@ -454,19 +455,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 })) => {
Expand Down Expand Up @@ -533,6 +546,72 @@ 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 parent_mir = mem::replace(&mut self.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 = 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);
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>,
Expand Down Expand Up @@ -738,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<ty::Region<'tcx>>,
Expand Down Expand Up @@ -893,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,
Expand All @@ -910,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
Expand Down Expand Up @@ -1095,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;
Expand Down Expand Up @@ -1281,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,
Expand Down Expand Up @@ -1340,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,
Expand Down Expand Up @@ -1998,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
Expand Down Expand Up @@ -2097,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>,
Expand Down Expand Up @@ -2146,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 {
Expand Down Expand Up @@ -2275,7 +2357,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
Expand Down
4 changes: 1 addition & 3 deletions src/test/ui/nll/issue-48697.rs
Original file line number Diff line number Diff line change
@@ -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() {}
11 changes: 11 additions & 0 deletions src/test/ui/nll/issue-48697.stderr
Original file line number Diff line number Diff line change
@@ -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`.
27 changes: 27 additions & 0 deletions src/test/ui/nll/promoted-bounds.rs
Original file line number Diff line number Diff line change
@@ -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);
}
15 changes: 15 additions & 0 deletions src/test/ui/nll/promoted-bounds.stderr
Original file line number Diff line number Diff line change
@@ -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`.
Loading

0 comments on commit a21d824

Please sign in to comment.