Skip to content

Commit bd1bd76

Browse files
committed
fix linking of place projections
projections other than dereferences of `&mut` used to do no linking. Fix that. Fixes #46974.
1 parent 17d4e9b commit bd1bd76

File tree

3 files changed

+123
-22
lines changed

3 files changed

+123
-22
lines changed

Diff for: src/librustc_mir/borrow_check/nll/constraint_generation.rs

+75-22
Original file line numberDiff line numberDiff line change
@@ -130,38 +130,91 @@ impl<'cx, 'cg, 'gcx, 'tcx> ConstraintGeneration<'cx, 'cg, 'gcx, 'tcx> {
130130
});
131131
}
132132

133+
// Add the reborrow constraint at `location` so that `borrowed_place`
134+
// is valid for `borrow_region`.
133135
fn add_reborrow_constraint(
134136
&mut self,
135137
location: Location,
136138
borrow_region: ty::Region<'tcx>,
137139
borrowed_place: &Place<'tcx>,
138140
) {
139-
if let Projection(ref proj) = *borrowed_place {
140-
let PlaceProjection { ref base, ref elem } = **proj;
141-
142-
if let ProjectionElem::Deref = *elem {
143-
let tcx = self.infcx.tcx;
144-
let base_ty = base.ty(self.mir, tcx).to_ty(tcx);
145-
let base_sty = &base_ty.sty;
146-
147-
if let ty::TyRef(base_region, ty::TypeAndMut { ty: _, mutbl }) = *base_sty {
148-
match mutbl {
149-
hir::Mutability::MutImmutable => {}
150-
151-
hir::Mutability::MutMutable => {
152-
self.add_reborrow_constraint(location, borrow_region, base);
141+
let mut borrowed_place = borrowed_place;
142+
143+
debug!("add_reborrow_constraint({:?}, {:?}, {:?})",
144+
location, borrow_region, borrowed_place);
145+
while let Projection(box PlaceProjection { base, elem }) = borrowed_place {
146+
debug!("add_reborrow_constraint - iteration {:?}", borrowed_place);
147+
148+
match *elem {
149+
ProjectionElem::Deref => {
150+
let tcx = self.infcx.tcx;
151+
let base_ty = base.ty(self.mir, tcx).to_ty(tcx);
152+
153+
debug!("add_reborrow_constraint - base_ty = {:?}", base_ty);
154+
match base_ty.sty {
155+
ty::TyRef(ref_region, ty::TypeAndMut { ty: _, mutbl }) => {
156+
let span = self.mir.source_info(location).span;
157+
self.regioncx.add_outlives(
158+
span,
159+
ref_region.to_region_vid(),
160+
borrow_region.to_region_vid(),
161+
location.successor_within_block(),
162+
);
163+
164+
match mutbl {
165+
hir::Mutability::MutImmutable => {
166+
// Immutable reference. We don't need the base
167+
// to be valid for the entire lifetime of
168+
// the borrow.
169+
break
170+
}
171+
hir::Mutability::MutMutable => {
172+
// Mutable reference. We *do* need the base
173+
// to be valid, because after the base becomes
174+
// invalid, someone else can use our mutable deref.
175+
176+
// This is in order to make the following function
177+
// illegal:
178+
// ```
179+
// fn unsafe_deref<'a, 'b>(x: &'a &'b mut T) -> &'b mut T {
180+
// &mut *x
181+
// }
182+
// ```
183+
//
184+
// As otherwise you could clone `&mut T` using the
185+
// following function:
186+
// ```
187+
// fn bad(x: &mut T) -> (&mut T, &mut T) {
188+
// let my_clone = unsafe_deref(&'a x);
189+
// ENDREGION 'a;
190+
// (my_clone, x)
191+
// }
192+
// ```
193+
}
194+
}
195+
}
196+
ty::TyRawPtr(..) => {
197+
// deref of raw pointer, guaranteed to be valid
198+
break
153199
}
200+
ty::TyAdt(def, _) if def.is_box() => {
201+
// deref of `Box`, need the base to be valid - propagate
202+
}
203+
_ => bug!("unexpected deref ty {:?} in {:?}", base_ty, borrowed_place)
154204
}
155-
156-
let span = self.mir.source_info(location).span;
157-
self.regioncx.add_outlives(
158-
span,
159-
base_region.to_region_vid(),
160-
borrow_region.to_region_vid(),
161-
location.successor_within_block(),
162-
);
205+
}
206+
ProjectionElem::Field(..) |
207+
ProjectionElem::Downcast(..) |
208+
ProjectionElem::Index(..) |
209+
ProjectionElem::ConstantIndex { .. } |
210+
ProjectionElem::Subslice { .. } => {
211+
// other field access
163212
}
164213
}
214+
215+
// The "propagate" case. We need to check that our base is valid
216+
// for the borrow's lifetime.
217+
borrowed_place = base;
165218
}
166219
}
167220
}

Diff for: src/test/ui/nll/guarantor-issue-46974.rs

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Test that NLL analysis propagates lifetimes correctly through
12+
// field accesses, Box accesses, etc.
13+
14+
#![feature(nll)]
15+
16+
fn foo(s: &mut (i32,)) -> i32 {
17+
let t = &mut *s; // this borrow should last for the entire function
18+
let x = &t.0;
19+
*s = (2,); //~ ERROR cannot assign to `*s`
20+
*x
21+
}
22+
23+
fn bar(s: &Box<(i32,)>) -> &'static i32 {
24+
// FIXME(#46983): error message should be better
25+
&s.0 //~ ERROR free region `` does not outlive free region `'static`
26+
}
27+
28+
fn main() {
29+
foo(&mut (0,));
30+
bar(&Box::new((1,)));
31+
}

Diff for: src/test/ui/nll/guarantor-issue-46974.stderr

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
error[E0506]: cannot assign to `*s` because it is borrowed
2+
--> $DIR/guarantor-issue-46974.rs:19:5
3+
|
4+
17 | let t = &mut *s; // this borrow should last for the entire function
5+
| ------- borrow of `*s` occurs here
6+
18 | let x = &t.0;
7+
19 | *s = (2,); //~ ERROR cannot assign to `*s`
8+
| ^^^^^^^^^ assignment to borrowed `*s` occurs here
9+
10+
error: free region `` does not outlive free region `'static`
11+
--> $DIR/guarantor-issue-46974.rs:25:5
12+
|
13+
25 | &s.0 //~ ERROR free region `` does not outlive free region `'static`
14+
| ^^^^
15+
16+
error: aborting due to 2 previous errors
17+

0 commit comments

Comments
 (0)