Skip a shared borrow of a immutable local variables

issue #53643

src/librustc/mir/mod.rs

@@ -252,11 +252,6 @@ impl<'tcx> Mir<'tcx> {
         } else if self.local_decls[local].name.is_some() {
             LocalKind::Var
         } else {
-            debug_assert!(
-                self.local_decls[local].mutability == Mutability::Mut,
-                "temp should be mutable"
-            );
-
             LocalKind::Temp
         }
     }
@@ -784,25 +779,30 @@ impl<'tcx> LocalDecl<'tcx> {
     /// Create a new `LocalDecl` for a temporary.
     #[inline]
     pub fn new_temp(ty: Ty<'tcx>, span: Span) -> Self {
-        LocalDecl {
+        Self::new_local(ty, Mutability::Mut, false, span)
-            mutability: Mutability::Mut,
+    }
-            ty,
+
-            name: None,
+    /// Create a new immutable `LocalDecl` for a temporary.
-            source_info: SourceInfo {
+    #[inline]
-                span,
+    pub fn new_immutable_temp(ty: Ty<'tcx>, span: Span) -> Self {
-                scope: OUTERMOST_SOURCE_SCOPE,
+        Self::new_local(ty, Mutability::Not, false, span)
-            },
-            visibility_scope: OUTERMOST_SOURCE_SCOPE,
-            internal: false,
-            is_user_variable: None,
-        }
     }
 
     /// Create a new `LocalDecl` for a internal temporary.
     #[inline]
     pub fn new_internal(ty: Ty<'tcx>, span: Span) -> Self {
+        Self::new_local(ty, Mutability::Mut, true, span)
+    }
+
+    #[inline]
+    fn new_local(
+        ty: Ty<'tcx>,
+        mutability: Mutability,
+        internal: bool,
+        span: Span,
+    ) -> Self {
         LocalDecl {
-            mutability: Mutability::Mut,
+            mutability,
             ty,
             name: None,
             source_info: SourceInfo {
@@ -810,7 +810,7 @@ impl<'tcx> LocalDecl<'tcx> {
                 scope: OUTERMOST_SOURCE_SCOPE,
             },
             visibility_scope: OUTERMOST_SOURCE_SCOPE,
-            internal: true,
+            internal,
             is_user_variable: None,
         }
     }

src/librustc_mir/borrow_check/borrow_set.rs

@@ -10,12 +10,14 @@
 
 use borrow_check::place_ext::PlaceExt;
 use dataflow::indexes::BorrowIndex;
+use dataflow::move_paths::MoveData;
 use rustc::mir::traversal;
 use rustc::mir::visit::{PlaceContext, Visitor};
-use rustc::mir::{self, Location, Mir, Place};
+use rustc::mir::{self, Location, Mir, Place, Local};
 use rustc::ty::{Region, TyCtxt};
 use rustc::util::nodemap::{FxHashMap, FxHashSet};
 use rustc_data_structures::indexed_vec::IndexVec;
+use rustc_data_structures::bitvec::BitArray;
 use std::fmt;
 use std::hash::Hash;
 use std::ops::Index;
@@ -43,6 +45,8 @@ crate struct BorrowSet<'tcx> {
 
     /// Map from local to all the borrows on that local
     crate local_map: FxHashMap<mir::Local, FxHashSet<BorrowIndex>>,
+
+    crate locals_state_at_exit: LocalsStateAtExit,
 }
 
 impl<'tcx> Index<BorrowIndex> for BorrowSet<'tcx> {
@@ -96,8 +100,52 @@ impl<'tcx> fmt::Display for BorrowData<'tcx> {
     }
 }
 
+crate enum LocalsStateAtExit {
+    AllAreInvalidated,
+    SomeAreInvalidated { has_storage_dead_or_moved: BitArray<Local> }
+}
+
+impl LocalsStateAtExit {
+    fn build(
+        locals_are_invalidated_at_exit: bool,
+        mir: &Mir<'tcx>,
+        move_data: &MoveData<'tcx>
+    ) -> Self {
+        struct HasStorageDead(BitArray<Local>);
+
+        impl<'tcx> Visitor<'tcx> for HasStorageDead {
+            fn visit_local(&mut self, local: &Local, ctx: PlaceContext<'tcx>, _: Location) {
+                if ctx == PlaceContext::StorageDead {
+                    self.0.insert(*local);
+                }
+            }
+        }
+
+        if locals_are_invalidated_at_exit {
+            LocalsStateAtExit::AllAreInvalidated
+        } else {
+            let mut has_storage_dead = HasStorageDead(BitArray::new(mir.local_decls.len()));
+            has_storage_dead.visit_mir(mir);
+            let mut has_storage_dead_or_moved = has_storage_dead.0;
+            for move_out in &move_data.moves {
+                if let Some(index) = move_data.base_local(move_out.path) {
+                    has_storage_dead_or_moved.insert(index);
+
+                }
+            }
+            LocalsStateAtExit::SomeAreInvalidated{ has_storage_dead_or_moved }
+        }
+    }
+}
+
 impl<'tcx> BorrowSet<'tcx> {
-    pub fn build(tcx: TyCtxt<'_, '_, 'tcx>, mir: &Mir<'tcx>) -> Self {
+    pub fn build(
+        tcx: TyCtxt<'_, '_, 'tcx>,
+        mir: &Mir<'tcx>,
+        locals_are_invalidated_at_exit: bool,
+        move_data: &MoveData<'tcx>
+    ) -> Self {
+
         let mut visitor = GatherBorrows {
             tcx,
             mir,
@@ -107,6 +155,8 @@ impl<'tcx> BorrowSet<'tcx> {
             region_map: FxHashMap(),
             local_map: FxHashMap(),
             pending_activations: FxHashMap(),
+            locals_state_at_exit:
+                LocalsStateAtExit::build(locals_are_invalidated_at_exit, mir, move_data),
         };
 
         for (block, block_data) in traversal::preorder(mir) {
@@ -119,6 +169,7 @@ impl<'tcx> BorrowSet<'tcx> {
             activation_map: visitor.activation_map,
             region_map: visitor.region_map,
             local_map: visitor.local_map,
+            locals_state_at_exit: visitor.locals_state_at_exit,
         }
     }
 
@@ -148,6 +199,8 @@ struct GatherBorrows<'a, 'gcx: 'tcx, 'tcx: 'a> {
     /// the borrow. When we find a later use of this activation, we
     /// remove from the map (and add to the "tombstone" set below).
     pending_activations: FxHashMap<mir::Local, BorrowIndex>,
+
+    locals_state_at_exit: LocalsStateAtExit,
 }
 
 impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
@@ -159,7 +212,8 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
         location: mir::Location,
     ) {
         if let mir::Rvalue::Ref(region, kind, ref borrowed_place) = *rvalue {
-            if borrowed_place.ignore_borrow(self.tcx, self.mir) {
+            if borrowed_place.ignore_borrow(
+                self.tcx, self.mir, &self.locals_state_at_exit) {
                 return;
             }
 

src/librustc_mir/borrow_check/mod.rs

@@ -196,7 +196,12 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
         |bd, i| DebugFormatted::new(&bd.move_data().inits[i]),
     ));
 
-    let borrow_set = Rc::new(BorrowSet::build(tcx, mir));
+    let locals_are_invalidated_at_exit = match tcx.hir.body_owner_kind(id) {
+            hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) => false,
+            hir::BodyOwnerKind::Fn => true,
+    };
+    let borrow_set = Rc::new(BorrowSet::build(
+            tcx, mir, locals_are_invalidated_at_exit, &mdpe.move_data));
 
     // If we are in non-lexical mode, compute the non-lexical lifetimes.
     let (regioncx, polonius_output, opt_closure_req) = nll::compute_regions(
@@ -241,10 +246,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
         param_env: param_env,
         location_table,
         movable_generator,
-        locals_are_invalidated_at_exit: match tcx.hir.body_owner_kind(id) {
+        locals_are_invalidated_at_exit,
-            hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) => false,
-            hir::BodyOwnerKind::Fn => true,
-        },
         access_place_error_reported: FxHashSet(),
         reservation_error_reported: FxHashSet(),
         moved_error_reported: FxHashSet(),

src/librustc_mir/borrow_check/place_ext.rs

@@ -10,8 +10,9 @@
 
 use rustc::hir;
 use rustc::mir::ProjectionElem;
-use rustc::mir::{Local, Mir, Place};
+use rustc::mir::{Local, Mir, Place, Mutability};
 use rustc::ty::{self, TyCtxt};
+use borrow_check::borrow_set::LocalsStateAtExit;
 
 /// Extension methods for the `Place` type.
 crate trait PlaceExt<'tcx> {
@@ -19,7 +20,12 @@ crate trait PlaceExt<'tcx> {
     /// This is true whenever there is no action that the user can do
     /// to the place `self` that would invalidate the borrow. This is true
     /// for borrows of raw pointer dereferents as well as shared references.
-    fn ignore_borrow(&self, tcx: TyCtxt<'_, '_, 'tcx>, mir: &Mir<'tcx>) -> bool;
+    fn ignore_borrow(
+        &self,
+        tcx: TyCtxt<'_, '_, 'tcx>,
+        mir: &Mir<'tcx>,
+        locals_state_at_exit: &LocalsStateAtExit,
+        ) -> bool;
 
     /// If this is a place like `x.f.g`, returns the local
     /// `x`. Returns `None` if this is based in a static.
@@ -27,10 +33,34 @@ crate trait PlaceExt<'tcx> {
 }
 
 impl<'tcx> PlaceExt<'tcx> for Place<'tcx> {
-    fn ignore_borrow(&self, tcx: TyCtxt<'_, '_, 'tcx>, mir: &Mir<'tcx>) -> bool {
+    fn ignore_borrow(
+        &self,
+        tcx: TyCtxt<'_, '_, 'tcx>,
+        mir: &Mir<'tcx>,
+        locals_state_at_exit: &LocalsStateAtExit,
+    ) -> bool {
         match self {
-            Place::Promoted(_) |
+            Place::Promoted(_) => false,
-            Place::Local(_) => false,
+
+            // If a local variable is immutable, then we only need to track borrows to guard
+            // against two kinds of errors:
+            // * The variable being dropped while still borrowed (e.g., because the fn returns
+            //   a reference to a local variable)
+            // * The variable being moved while still borrowed
+            //
+            // In particular, the variable cannot be mutated -- the "access checks" will fail --
+            // so we don't have to worry about mutation while borrowed.
+            Place::Local(index) => {
+                match locals_state_at_exit {
+                    LocalsStateAtExit::AllAreInvalidated => false,
+                    LocalsStateAtExit::SomeAreInvalidated { has_storage_dead_or_moved } => {
+                        let ignore = !has_storage_dead_or_moved.contains(*index) &&
+                            mir.local_decls[*index].mutability == Mutability::Not;
+                        debug!("ignore_borrow: local {:?} => {:?}", index, ignore);
+                        ignore
+                    }
+                }
+            }
             Place::Static(static_) => {
                 tcx.is_static(static_.def_id) == Some(hir::Mutability::MutMutable)
             }
@@ -39,7 +69,8 @@ impl<'tcx> PlaceExt<'tcx> for Place<'tcx> {
                 | ProjectionElem::Downcast(..)
                 | ProjectionElem::Subslice { .. }
                 | ProjectionElem::ConstantIndex { .. }
-                | ProjectionElem::Index(_) => proj.base.ignore_borrow(tcx, mir),
+                | ProjectionElem::Index(_) => proj.base.ignore_borrow(
+                    tcx, mir, locals_state_at_exit),
 
                 ProjectionElem::Deref => {
                     let ty = proj.base.ty(mir, tcx).to_ty(tcx);
@@ -55,7 +86,7 @@ impl<'tcx> PlaceExt<'tcx> for Place<'tcx> {
                         // borrowed *that* one, leaving the original
                         // path unborrowed.
                         ty::RawPtr(..) | ty::Ref(_, _, hir::MutImmutable) => true,
-                        _ => proj.base.ignore_borrow(tcx, mir),
+                        _ => proj.base.ignore_borrow(tcx, mir, locals_state_at_exit),
                     }
                 }
             },

src/librustc_mir/build/expr/as_operand.rs

@@ -73,7 +73,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             Category::Place |
             Category::Rvalue(..) => {
                 let operand =
-                    unpack!(block = this.as_temp(block, scope, expr));
+                    unpack!(block = this.as_temp(block, scope, expr, Mutability::Mut));
                 block.and(Operand::Move(Place::Local(operand)))
             }
         }

src/librustc_mir/build/expr/as_place.rs

@@ -28,14 +28,30 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
         where M: Mirror<'tcx, Output=Expr<'tcx>>
     {
         let expr = self.hir.mirror(expr);
-        self.expr_as_place(block, expr)
+        self.expr_as_place(block, expr, Mutability::Mut)
+    }
+
+    /// Compile `expr`, yielding a place that we can move from etc.
+    /// Mutability note: The caller of this method promises only to read from the resulting
+    /// place. The place itself may or may not be mutable:
+    /// * If this expr is a place expr like a.b, then we will return that place.
+    /// * Otherwise, a temporary is created: in that event, it will be an immutable temporary.
+    pub fn as_read_only_place<M>(&mut self,
+                        block: BasicBlock,
+                        expr: M)
+                        -> BlockAnd<Place<'tcx>>
+        where M: Mirror<'tcx, Output=Expr<'tcx>>
+    {
+        let expr = self.hir.mirror(expr);
+        self.expr_as_place(block, expr, Mutability::Not)
     }
 
     fn expr_as_place(&mut self,
                       mut block: BasicBlock,
-                      expr: Expr<'tcx>)
+                      expr: Expr<'tcx>,
+                      mutability: Mutability)
                       -> BlockAnd<Place<'tcx>> {
-        debug!("expr_as_place(block={:?}, expr={:?})", block, expr);
+        debug!("expr_as_place(block={:?}, expr={:?}, mutability={:?})", block, expr, mutability);
 
         let this = self;
         let expr_span = expr.span;
@@ -43,7 +59,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
         match expr.kind {
             ExprKind::Scope { region_scope, lint_level, value } => {
                 this.in_scope((region_scope, source_info), lint_level, block, |this| {
-                    this.as_place(block, value)
+                    if mutability == Mutability::Not {
+                        this.as_read_only_place(block, value)
+                    } else {
+                        this.as_place(block, value)
+                    }
                 })
             }
             ExprKind::Field { lhs, name } => {
@@ -63,7 +83,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                 // region_scope=None so place indexes live forever. They are scalars so they
                 // do not need storage annotations, and they are often copied between
                 // places.
-                let idx = unpack!(block = this.as_temp(block, None, index));
+                let idx = unpack!(block = this.as_temp(block, None, index, Mutability::Mut));
 
                 // bounds check:
                 let (len, lt) = (this.temp(usize_ty.clone(), expr_span),
@@ -137,7 +157,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                     Some(Category::Place) => false,
                     _ => true,
                 });
-                let temp = unpack!(block = this.as_temp(block, expr.temp_lifetime, expr));
+                let temp = unpack!(
+                    block = this.as_temp(block, expr.temp_lifetime, expr, mutability));
                 block.and(Place::Local(temp))
             }
         }

src/librustc_mir/build/expr/as_rvalue.rs

@@ -63,7 +63,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                 block.and(Rvalue::Repeat(value_operand, count))
             }
             ExprKind::Borrow { region, borrow_kind, arg } => {
-                let arg_place = unpack!(block = this.as_place(block, arg));
+                let arg_place = match borrow_kind {
+                    BorrowKind::Shared => unpack!(block = this.as_read_only_place(block, arg)),
+                    _ => unpack!(block = this.as_place(block, arg))
+                };
                 block.and(Rvalue::Ref(region, borrow_kind, arg_place))
             }
             ExprKind::Binary { op, lhs, rhs } => {

src/librustc_mir/build/expr/as_temp.rs

@@ -21,33 +21,39 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
     pub fn as_temp<M>(&mut self,
                       block: BasicBlock,
                       temp_lifetime: Option<region::Scope>,
-                      expr: M)
+                      expr: M,
+                      mutability: Mutability)
                       -> BlockAnd<Local>
         where M: Mirror<'tcx, Output = Expr<'tcx>>
     {
         let expr = self.hir.mirror(expr);
-        self.expr_as_temp(block, temp_lifetime, expr)
+        self.expr_as_temp(block, temp_lifetime, expr, mutability)
     }
 
     fn expr_as_temp(&mut self,
                     mut block: BasicBlock,
                     temp_lifetime: Option<region::Scope>,
-                    expr: Expr<'tcx>)
+                    expr: Expr<'tcx>,
+                    mutability: Mutability)
                     -> BlockAnd<Local> {
-        debug!("expr_as_temp(block={:?}, temp_lifetime={:?}, expr={:?})",
+        debug!("expr_as_temp(block={:?}, temp_lifetime={:?}, expr={:?}, mutability={:?})",
-               block, temp_lifetime, expr);
+               block, temp_lifetime, expr, mutability);
         let this = self;
 
         let expr_span = expr.span;
         let source_info = this.source_info(expr_span);
         if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind {
             return this.in_scope((region_scope, source_info), lint_level, block, |this| {
-                this.as_temp(block, temp_lifetime, value)
+                this.as_temp(block, temp_lifetime, value, mutability)
             });
         }
 
         let expr_ty = expr.ty;
-        let temp = this.local_decls.push(LocalDecl::new_temp(expr_ty, expr_span));
+        let temp = if mutability == Mutability::Not {
+            this.local_decls.push(LocalDecl::new_immutable_temp(expr_ty, expr_span))
+        } else {
+            this.local_decls.push(LocalDecl::new_temp(expr_ty, expr_span))
+        };
 
         if !expr_ty.is_never() {
             this.cfg.push(block, Statement {

src/librustc_mir/dataflow/impls/borrows.rs

@@ -115,7 +115,7 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
         nonlexical_regioncx: Rc<RegionInferenceContext<'tcx>>,
         def_id: DefId,
         body_id: Option<hir::BodyId>,
-        borrow_set: &Rc<BorrowSet<'tcx>>
+        borrow_set: &Rc<BorrowSet<'tcx>>,
     ) -> Self {
         let scope_tree = tcx.region_scope_tree(def_id);
         let root_scope = body_id.map(|body_id| {
@@ -233,7 +233,13 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
                 // propagate_call_return method.
 
                 if let mir::Rvalue::Ref(region, _, ref place) = *rhs {
-                    if place.ignore_borrow(self.tcx, self.mir) { return; }
+                    if place.ignore_borrow(
+                        self.tcx,
+                        self.mir,
+                        &self.borrow_set.locals_state_at_exit,
+                    ) {
+                        return;
+                    }
                     let index = self.borrow_set.location_map.get(&location).unwrap_or_else(|| {
                         panic!("could not find BorrowIndex for location {:?}", location);
                     });

src/librustc_mir/dataflow/move_paths/mod.rs

@@ -316,4 +316,14 @@ impl<'a, 'gcx, 'tcx> MoveData<'tcx> {
                         -> Result<Self, (Self, Vec<(Place<'tcx>, MoveError<'tcx>)>)> {
         builder::gather_moves(mir, tcx)
     }
+
+    /// For the move path `mpi`, returns the root local variable (if any) that starts the path.
+    /// (e.g., for a path like `a.b.c` returns `Some(a)`)
+    pub fn base_local(&self, mut mpi: MovePathIndex) -> Option<Local> {
+        loop {
+            let path = &self.move_paths[mpi];
+            if let Place::Local(l) = path.place { return Some(l); }
+            if let Some(parent) = path.parent { mpi = parent; continue } else { return None }
+        }
+    }
 }

src/librustc_mir/util/pretty.rs

@@ -612,8 +612,9 @@ fn write_temp_decls(mir: &Mir, w: &mut dyn Write) -> io::Result<()> {
     for temp in mir.temps_iter() {
         writeln!(
             w,
-            "{}let mut {:?}: {};",
+            "{}let {}{:?}: {};",
             INDENT,
+            if mir.local_decls[temp].mutability == Mutability::Mut {"mut "} else {""},
             temp,
             mir.local_decls[temp].ty
         )?;

src/test/mir-opt/end_region_destruction_extents_1.rs

@@ -70,10 +70,10 @@ unsafe impl<'a, #[may_dangle] 'b> Drop for D1<'a, 'b> {
 //     let mut _2: D1<'12ds, '10s>;
 //     let mut _3: &'12ds S1;
 //     let mut _4: &'12ds S1;
-//     let mut _5: S1;
+//     let _5: S1;
 //     let mut _6: &'10s S1;
 //     let mut _7: &'10s S1;
-//     let mut _8: S1;
+//     let _8: S1;
 //     bb0: {
 //         StorageLive(_2);
 //         StorageLive(_3);
@@ -118,10 +118,10 @@ unsafe impl<'a, #[may_dangle] 'b> Drop for D1<'a, 'b> {
 //     let mut _2: D1<'12ds, '10s>;
 //     let mut _3: &'12ds S1;
 //     let mut _4: &'12ds S1;
-//     let mut _5: S1;
+//     let _5: S1;
 //     let mut _6: &'10s S1;
 //     let mut _7: &'10s S1;
-//     let mut _8: S1;
+//     let _8: S1;
 //     bb0: {
 //         StorageLive(_2);
 //         StorageLive(_3);

src/test/mir-opt/storage_live_dead_in_statics.rs

@@ -47,11 +47,11 @@ fn main() {
 // START rustc.XXX.mir_map.0.mir
 //    let mut _0: &'static Foo;
 //    let mut _1: &'static Foo;
-//    let mut _2: Foo;
+//    let _2: Foo;
 //    let mut _3: &'static [(u32, u32)];
 //    let mut _4: &'static [(u32, u32); 42];
 //    let mut _5: &'static [(u32, u32); 42];
-//    let mut _6: [(u32, u32); 42];
+//    let _6: [(u32, u32); 42];
 //    let mut _7: (u32, u32);
 //    let mut _8: (u32, u32);
 //    let mut _9: (u32, u32);