diff options
| -rw-r--r-- | src/librustc_mir/borrow_check/flows.rs | 3 | ||||
| -rw-r--r-- | src/librustc_mir/borrow_check/mod.rs | 2 | ||||
| -rw-r--r-- | src/librustc_mir/borrow_check/nll/constraint_generation.rs | 97 | ||||
| -rw-r--r-- | src/librustc_mir/dataflow/at_location.rs | 24 | ||||
| -rw-r--r-- | src/librustc_mir/dataflow/impls/borrows.rs | 28 | ||||
| -rw-r--r-- | src/librustc_mir/dataflow/mod.rs | 47 | ||||
| -rw-r--r-- | src/librustc_mir/transform/generator.rs | 2 | ||||
| -rw-r--r-- | src/librustc_mir/transform/rustc_peek.rs | 9 | ||||
| -rw-r--r-- | src/test/compile-fail/borrowck/two-phase-activation-sharing-interference.rs | 1 | ||||
| -rw-r--r-- | src/test/ui/nll/borrow-use-issue-46875.rs | 30 | ||||
| -rw-r--r-- | src/test/ui/nll/guarantor-issue-46974.rs | 31 | ||||
| -rw-r--r-- | src/test/ui/nll/guarantor-issue-46974.stderr | 17 |
12 files changed, 262 insertions, 29 deletions
diff --git a/src/librustc_mir/borrow_check/flows.rs b/src/librustc_mir/borrow_check/flows.rs index 69a08c7a30d..61d6c14d627 100644 --- a/src/librustc_mir/borrow_check/flows.rs +++ b/src/librustc_mir/borrow_check/flows.rs @@ -88,7 +88,8 @@ impl<'b, 'gcx, 'tcx> fmt::Display for Flows<'b, 'gcx, 'tcx> { }; saw_one = true; let borrow_data = &self.borrows.operator().borrows()[borrow.borrow_index()]; - s.push_str(&format!("{}", borrow_data)); + s.push_str(&format!("{}{}", borrow_data, + if borrow.is_activation() { "@active" } else { "" })); }); s.push_str("] "); diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 4eb664e4856..b5836c65d67 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -2012,6 +2012,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let borrowed = &data[i.borrow_index()]; if self.places_conflict(&borrowed.borrowed_place, place, access) { + debug!("each_borrow_involving_path: {:?} @ {:?} vs. {:?}/{:?}", + i, borrowed, place, access); let ctrl = op(self, i, borrowed); if ctrl == Control::Break { return; diff --git a/src/librustc_mir/borrow_check/nll/constraint_generation.rs b/src/librustc_mir/borrow_check/nll/constraint_generation.rs index bdacd831cb6..7ca4ebd1cb2 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; - - 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() => { + // 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/librustc_mir/dataflow/at_location.rs b/src/librustc_mir/dataflow/at_location.rs index 7f243ad6e26..b1f73bfbe22 100644 --- a/src/librustc_mir/dataflow/at_location.rs +++ b/src/librustc_mir/dataflow/at_location.rs @@ -149,6 +149,18 @@ impl<BD> FlowsAtLocation for FlowAtLocation<BD> fn reconstruct_statement_effect(&mut self, loc: Location) { self.stmt_gen.reset_to_empty(); self.stmt_kill.reset_to_empty(); + { + let mut sets = BlockSets { + on_entry: &mut self.curr_state, + gen_set: &mut self.stmt_gen, + kill_set: &mut self.stmt_kill, + }; + self.base_results + .operator() + .before_statement_effect(&mut sets, loc); + } + self.apply_local_effect(loc); + let mut sets = BlockSets { on_entry: &mut self.curr_state, gen_set: &mut self.stmt_gen, @@ -162,6 +174,18 @@ impl<BD> FlowsAtLocation for FlowAtLocation<BD> fn reconstruct_terminator_effect(&mut self, loc: Location) { self.stmt_gen.reset_to_empty(); self.stmt_kill.reset_to_empty(); + { + let mut sets = BlockSets { + on_entry: &mut self.curr_state, + gen_set: &mut self.stmt_gen, + kill_set: &mut self.stmt_kill, + }; + self.base_results + .operator() + .before_terminator_effect(&mut sets, loc); + } + self.apply_local_effect(loc); + let mut sets = BlockSets { on_entry: &mut self.curr_state, gen_set: &mut self.stmt_gen, diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index 6c1b8675764..f76aea19677 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -649,6 +649,13 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Reservations<'a, 'gcx, 'tcx> { // `_sets`. } + fn before_statement_effect(&self, + sets: &mut BlockSets<ReserveOrActivateIndex>, + location: Location) { + debug!("Reservations::before_statement_effect sets: {:?} location: {:?}", sets, location); + self.0.kill_loans_out_of_scope_at_location(sets, location, false); + } + fn statement_effect(&self, sets: &mut BlockSets<ReserveOrActivateIndex>, location: Location) { @@ -656,6 +663,13 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Reservations<'a, 'gcx, 'tcx> { self.0.statement_effect_on_borrows(sets, location, false); } + fn before_terminator_effect(&self, + sets: &mut BlockSets<ReserveOrActivateIndex>, + location: Location) { + debug!("Reservations::before_terminator_effect sets: {:?} location: {:?}", sets, location); + self.0.kill_loans_out_of_scope_at_location(sets, location, false); + } + fn terminator_effect(&self, sets: &mut BlockSets<ReserveOrActivateIndex>, location: Location) { @@ -696,6 +710,13 @@ impl<'a, 'gcx, 'tcx> BitDenotation for ActiveBorrows<'a, 'gcx, 'tcx> { // `_sets`. } + fn before_statement_effect(&self, + sets: &mut BlockSets<ReserveOrActivateIndex>, + location: Location) { + debug!("ActiveBorrows::before_statement_effect sets: {:?} location: {:?}", sets, location); + self.0.kill_loans_out_of_scope_at_location(sets, location, true); + } + fn statement_effect(&self, sets: &mut BlockSets<ReserveOrActivateIndex>, location: Location) { @@ -703,6 +724,13 @@ impl<'a, 'gcx, 'tcx> BitDenotation for ActiveBorrows<'a, 'gcx, 'tcx> { self.0.statement_effect_on_borrows(sets, location, true); } + fn before_terminator_effect(&self, + sets: &mut BlockSets<ReserveOrActivateIndex>, + location: Location) { + debug!("ActiveBorrows::before_terminator_effect sets: {:?} location: {:?}", sets, location); + self.0.kill_loans_out_of_scope_at_location(sets, location, true); + } + fn terminator_effect(&self, sets: &mut BlockSets<ReserveOrActivateIndex>, location: Location) { diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index 1597ba8aaa0..b18fb7c7b9c 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -214,6 +214,7 @@ impl<'a, 'tcx: 'a, BD> DataflowAnalysis<'a, 'tcx, BD> where BD: BitDenotation } for j_stmt in 0..statements.len() { let location = Location { block: bb, statement_index: j_stmt }; + self.flow_state.operator.before_statement_effect(sets, location); self.flow_state.operator.statement_effect(sets, location); if track_intrablock { sets.apply_local_effect(); @@ -222,6 +223,7 @@ impl<'a, 'tcx: 'a, BD> DataflowAnalysis<'a, 'tcx, BD> where BD: BitDenotation if terminator.is_some() { let location = Location { block: bb, statement_index: statements.len() }; + self.flow_state.operator.before_terminator_effect(sets, location); self.flow_state.operator.terminator_effect(sets, location); if track_intrablock { sets.apply_local_effect(); @@ -365,9 +367,10 @@ pub(crate) trait DataflowResultsConsumer<'a, 'tcx: 'a> { fn mir(&self) -> &'a Mir<'tcx>; } -pub fn state_for_location<T: BitDenotation>(loc: Location, - analysis: &T, - result: &DataflowResults<T>) +pub fn state_for_location<'tcx, T: BitDenotation>(loc: Location, + analysis: &T, + result: &DataflowResults<T>, + mir: &Mir<'tcx>) -> IdxSetBuf<T::Idx> { let mut entry = result.sets().on_entry_set_for(loc.block.index()).to_owned(); @@ -381,8 +384,16 @@ pub fn state_for_location<T: BitDenotation>(loc: Location, for stmt in 0..loc.statement_index { let mut stmt_loc = loc; stmt_loc.statement_index = stmt; + analysis.before_statement_effect(&mut sets, stmt_loc); analysis.statement_effect(&mut sets, stmt_loc); } + + // Apply the pre-statement effect of the statement we're evaluating. + if loc.statement_index == mir[loc.block].statements.len() { + analysis.before_terminator_effect(&mut sets, loc); + } else { + analysis.before_statement_effect(&mut sets, loc); + } } entry @@ -637,6 +648,21 @@ pub trait BitDenotation: BitwiseOperator { /// (For example, establishing the call arguments.) fn start_block_effect(&self, entry_set: &mut IdxSet<Self::Idx>); + /// Similar to `statement_effect`, except it applies + /// *just before* the statement rather than *just after* it. + /// + /// This matters for "dataflow at location" APIs, because the + /// before-statement effect is visible while visiting the + /// statement, while the after-statement effect only becomes + /// visible at the next statement. + /// + /// Both the before-statement and after-statement effects are + /// applied, in that order, before moving for the next + /// statement. + fn before_statement_effect(&self, + _sets: &mut BlockSets<Self::Idx>, + _location: Location) {} + /// Mutates the block-sets (the flow sets for the given /// basic block) according to the effects of evaluating statement. /// @@ -651,6 +677,21 @@ pub trait BitDenotation: BitwiseOperator { sets: &mut BlockSets<Self::Idx>, location: Location); + /// Similar to `terminator_effect`, except it applies + /// *just before* the terminator rather than *just after* it. + /// + /// This matters for "dataflow at location" APIs, because the + /// before-terminator effect is visible while visiting the + /// terminator, while the after-terminator effect only becomes + /// visible at the terminator's successors. + /// + /// Both the before-terminator and after-terminator effects are + /// applied, in that order, before moving for the next + /// terminator. + fn before_terminator_effect(&self, + _sets: &mut BlockSets<Self::Idx>, + _location: Location) {} + /// Mutates the block-sets (the flow sets for the given /// basic block) according to the effects of evaluating /// the terminator. diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 6be5b21cae6..9734edbe193 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -363,7 +363,7 @@ fn locals_live_across_suspend_points<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, statement_index: data.statements.len(), }; - let storage_liveness = state_for_location(loc, &analysis, &storage_live); + let storage_liveness = state_for_location(loc, &analysis, &storage_live, mir); storage_liveness_map.insert(block, storage_liveness.clone()); diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs index 6b8e2b073cc..b6153ea1fdb 100644 --- a/src/librustc_mir/transform/rustc_peek.rs +++ b/src/librustc_mir/transform/rustc_peek.rs @@ -203,11 +203,18 @@ fn each_block<'a, 'tcx, O>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // reset GEN and KILL sets before emulating their effect. for e in sets.gen_set.words_mut() { *e = 0; } for e in sets.kill_set.words_mut() { *e = 0; } - results.0.operator.statement_effect(&mut sets, Location { block: bb, statement_index: j }); + results.0.operator.before_statement_effect( + &mut sets, Location { block: bb, statement_index: j }); + results.0.operator.statement_effect( + &mut sets, Location { block: bb, statement_index: j }); sets.on_entry.union(sets.gen_set); sets.on_entry.subtract(sets.kill_set); } + results.0.operator.before_terminator_effect( + &mut sets, + Location { block: bb, statement_index: statements.len() }); + tcx.sess.span_err(span, &format!("rustc_peek: MIR did not match \ anticipated pattern; note that \ rustc_peek expects input of \ diff --git a/src/test/compile-fail/borrowck/two-phase-activation-sharing-interference.rs b/src/test/compile-fail/borrowck/two-phase-activation-sharing-interference.rs index b6f5e17f1f6..a9797e4d215 100644 --- a/src/test/compile-fail/borrowck/two-phase-activation-sharing-interference.rs +++ b/src/test/compile-fail/borrowck/two-phase-activation-sharing-interference.rs @@ -56,7 +56,6 @@ fn should_also_eventually_be_ok_with_nll() { let _z = &x; *y += 1; //[lxl]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable - //[nll]~^^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable } fn main() { } diff --git a/src/test/ui/nll/borrow-use-issue-46875.rs b/src/test/ui/nll/borrow-use-issue-46875.rs new file mode 100644 index 00000000000..47d69fe8e97 --- /dev/null +++ b/src/test/ui/nll/borrow-use-issue-46875.rs @@ -0,0 +1,30 @@ +// 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. + +#![feature(nll)] + +// run-pass + +fn vec() { + let mut _x = vec!['c']; + let _y = &_x; + _x = Vec::new(); +} + +fn int() { + let mut _x = 5; + let _y = &_x; + _x = 7; +} + +fn main() { + vec(); + int(); +} 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 00000000000..57ecddb80ab --- /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 <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,))); +} 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 00000000000..68cc87ef407 --- /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 + |