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

NLL fixes #46984

Merged
merged 3 commits into from
Jan 3, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
97 changes: 75 additions & 22 deletions src/librustc_mir/borrow_check/nll/constraint_generation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,38 +130,91 @@ impl<'cx, 'cg, 'gcx, 'tcx> ConstraintGeneration<'cx, 'cg, 'gcx, 'tcx> {
});
}

// Add the reborrow constraint at `location` so that `borrowed_place`
// is valid for `borrow_region`.
fn add_reborrow_constraint(
&mut self,
location: Location,
borrow_region: ty::Region<'tcx>,
borrowed_place: &Place<'tcx>,
) {
if let Projection(ref proj) = *borrowed_place {
let PlaceProjection { ref base, ref elem } = **proj;

if let ProjectionElem::Deref = *elem {
let tcx = self.infcx.tcx;
let base_ty = base.ty(self.mir, tcx).to_ty(tcx);
let base_sty = &base_ty.sty;

if let ty::TyRef(base_region, ty::TypeAndMut { ty: _, mutbl }) = *base_sty {
match mutbl {
hir::Mutability::MutImmutable => {}

hir::Mutability::MutMutable => {
self.add_reborrow_constraint(location, borrow_region, base);
let mut borrowed_place = borrowed_place;

debug!("add_reborrow_constraint({:?}, {:?}, {:?})",
location, borrow_region, borrowed_place);
while let Projection(box PlaceProjection { base, elem }) = borrowed_place {
debug!("add_reborrow_constraint - iteration {:?}", borrowed_place);

match *elem {
ProjectionElem::Deref => {
let tcx = self.infcx.tcx;
let base_ty = base.ty(self.mir, tcx).to_ty(tcx);

debug!("add_reborrow_constraint - base_ty = {:?}", base_ty);
match base_ty.sty {
ty::TyRef(ref_region, ty::TypeAndMut { ty: _, mutbl }) => {
let span = self.mir.source_info(location).span;
self.regioncx.add_outlives(
span,
ref_region.to_region_vid(),
borrow_region.to_region_vid(),
location.successor_within_block(),
);

match mutbl {
hir::Mutability::MutImmutable => {
// Immutable reference. We don't need the base
// to be valid for the entire lifetime of
// the borrow.
break
}
hir::Mutability::MutMutable => {
// Mutable reference. We *do* need the base
// to be valid, because after the base becomes
// invalid, someone else can use our mutable deref.

// This is in order to make the following function
// illegal:
// ```
// fn unsafe_deref<'a, 'b>(x: &'a &'b mut T) -> &'b mut T {
// &mut *x
// }
// ```
//
// As otherwise you could clone `&mut T` using the
// following function:
// ```
// fn bad(x: &mut T) -> (&mut T, &mut T) {
// let my_clone = unsafe_deref(&'a x);
// ENDREGION 'a;
// (my_clone, x)
// }
// ```
}
}
}
ty::TyRawPtr(..) => {
// deref of raw pointer, guaranteed to be valid
break
}
ty::TyAdt(def, _) if def.is_box() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

is the change here the inclusion of Box? (i.e., the behavior for other cases is the same?)

// deref of `Box`, need the base to be valid - propagate
}
_ => bug!("unexpected deref ty {:?} in {:?}", base_ty, borrowed_place)
}

let span = self.mir.source_info(location).span;
self.regioncx.add_outlives(
span,
base_region.to_region_vid(),
borrow_region.to_region_vid(),
location.successor_within_block(),
);
}
ProjectionElem::Field(..) |
ProjectionElem::Downcast(..) |
ProjectionElem::Index(..) |
ProjectionElem::ConstantIndex { .. } |
ProjectionElem::Subslice { .. } => {
// other field access
}
}

// The "propagate" case. We need to check that our base is valid
// for the borrow's lifetime.
borrowed_place = base;
}
}
}
31 changes: 31 additions & 0 deletions src/test/ui/nll/guarantor-issue-46974.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Test that NLL analysis propagates lifetimes correctly through
// field accesses, Box accesses, etc.

#![feature(nll)]

fn foo(s: &mut (i32,)) -> i32 {
let t = &mut *s; // this borrow should last for the entire function
let x = &t.0;
*s = (2,); //~ ERROR cannot assign to `*s`
*x
}

fn bar(s: &Box<(i32,)>) -> &'static i32 {
// FIXME(#46983): error message should be better
&s.0 //~ ERROR free region `` does not outlive free region `'static`
}

fn main() {
foo(&mut (0,));
bar(&Box::new((1,)));
}
17 changes: 17 additions & 0 deletions src/test/ui/nll/guarantor-issue-46974.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error[E0506]: cannot assign to `*s` because it is borrowed
--> $DIR/guarantor-issue-46974.rs:19:5
|
17 | let t = &mut *s; // this borrow should last for the entire function
| ------- borrow of `*s` occurs here
18 | let x = &t.0;
19 | *s = (2,); //~ ERROR cannot assign to `*s`
| ^^^^^^^^^ assignment to borrowed `*s` occurs here

error: free region `` does not outlive free region `'static`
--> $DIR/guarantor-issue-46974.rs:25:5
|
25 | &s.0 //~ ERROR free region `` does not outlive free region `'static`
| ^^^^

error: aborting due to 2 previous errors