From bd1bd76cd83d0a75917842966c52140a44753ed3 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sun, 24 Dec 2017 14:45:51 +0200 Subject: [PATCH] fix linking of place projections projections other than dereferences of `&mut` used to do no linking. Fix that. Fixes #46974. --- .../borrow_check/nll/constraint_generation.rs | 91 +++++++++++++++---- src/test/ui/nll/guarantor-issue-46974.rs | 31 +++++++ src/test/ui/nll/guarantor-issue-46974.stderr | 17 ++++ 3 files changed, 120 insertions(+), 19 deletions(-) create mode 100644 src/test/ui/nll/guarantor-issue-46974.rs create mode 100644 src/test/ui/nll/guarantor-issue-46974.stderr diff --git a/src/librustc_mir/borrow_check/nll/constraint_generation.rs b/src/librustc_mir/borrow_check/nll/constraint_generation.rs index bdacd831cb65..7ca4ebd1cb29 100644 --- a/src/librustc_mir/borrow_check/nll/constraint_generation.rs +++ b/src/librustc_mir/borrow_check/nll/constraint_generation.rs @@ -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; + let mut borrowed_place = borrowed_place; - 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; + 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); - if let ty::TyRef(base_region, ty::TypeAndMut { ty: _, mutbl }) = *base_sty { - match mutbl { - hir::Mutability::MutImmutable => {} + match *elem { + ProjectionElem::Deref => { + let tcx = self.infcx.tcx; + let base_ty = base.ty(self.mir, tcx).to_ty(tcx); - hir::Mutability::MutMutable => { - self.add_reborrow_constraint(location, borrow_region, base); + 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() => { + // 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; } } } diff --git a/src/test/ui/nll/guarantor-issue-46974.rs b/src/test/ui/nll/guarantor-issue-46974.rs new file mode 100644 index 000000000000..57ecddb80ab3 --- /dev/null +++ b/src/test/ui/nll/guarantor-issue-46974.rs @@ -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 or the MIT license +// , 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,))); +} diff --git a/src/test/ui/nll/guarantor-issue-46974.stderr b/src/test/ui/nll/guarantor-issue-46974.stderr new file mode 100644 index 000000000000..68cc87ef4073 --- /dev/null +++ b/src/test/ui/nll/guarantor-issue-46974.stderr @@ -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 +