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

Include bounds from promoted constants in NLL #57202

Merged
merged 3 commits into from
Mar 2, 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
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
Copy link
Member

Choose a reason for hiding this comment

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

uh.... uh oh?

Is this because we aren't inferring a sufficiently general type for the closure?

Copy link
Member

Choose a reason for hiding this comment

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

but I guess the "good" news is that AST-borrowck also rejects this code, so the impact of this "regression" will be limited... maybe...?

}

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