Auto merge of #46984 - arielb1:pre-statement-effect, r=nikomatsakis

NLL fixes

First, introduce pre-statement effects to dataflow to fix #46875. Edge dataflow effects might make that redundant, but I'm not sure of the best way to integrate them with liveness etc., and if this is a hack, this is one of the cleanest hacks I've seen.

And I want a small fix to avoid the torrent of bug reports.

Second, fix linking of projections to fix #46974

r? @pnkfelix

Author: bors

Diff for: 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 for: 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 for: 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 for: 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 for: src/librustc_mir/dataflow/impls/borrows.rs

@@ -649,13 +649,27 @@ 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) {
         debug!("Reservations::statement_effect sets: {:?} location: {:?}", sets, location);
         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,13 +710,27 @@ 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) {
         debug!("ActiveBorrows::statement_effect sets: {:?} location: {:?}", sets, location);
         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 for: 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 for: 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 for: 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 for: 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 for: 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();
+}