Auto merge of rust-lang#53909 - mikhail-m1:53643, r=nikomatsakis

Skip a shared borrow of a immutable local variables

issue rust-lang#53643

r? @nikomatsakis

Author: bors

src/librustc/mir/mod.rs

@@ -249,11 +249,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
         }
     }
@@ -782,33 +777,38 @@ impl<'tcx> LocalDecl<'tcx> {
     /// Create a new `LocalDecl` for a temporary.
     #[inline]
     pub fn new_temp(ty: Ty<'tcx>, span: Span) -> Self {
-        LocalDecl {
-            mutability: Mutability::Mut,
-            ty,
-            name: None,
-            source_info: SourceInfo {
-                span,
-                scope: OUTERMOST_SOURCE_SCOPE,
-            },
-            visibility_scope: OUTERMOST_SOURCE_SCOPE,
-            internal: false,
-            is_user_variable: None,
-        }
+        Self::new_local(ty, Mutability::Mut, false, span)
+    }
+
+    /// Create a new immutable `LocalDecl` for a temporary.
+    #[inline]
+    pub fn new_immutable_temp(ty: Ty<'tcx>, span: Span) -> Self {
+        Self::new_local(ty, Mutability::Not, false, span)
     }
 
     /// 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 {
                 span,
                 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) {
-            hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) => false,
-            hir::BodyOwnerKind::Fn => true,
-        },
+        locals_are_invalidated_at_exit,
         access_place_error_reported: FxHashSet(),
         reservation_error_reported: FxHashSet(),
         moved_error_reported: FxHashSet(),

src/librustc_mir/borrow_check/place_ext.rs

@@ -10,27 +10,57 @@
 
 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> {
     /// Returns true if we can safely ignore borrows of this place.
     /// 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.
     fn root_local(&self) -> Option<Local>;
 }
 
 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::Local(_) => false,
+            Place::Promoted(_) => 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_constant.rs

@@ -18,26 +18,34 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
     /// Compile `expr`, yielding a compile-time constant. Assumes that
     /// `expr` is a valid compile-time constant!
     pub fn as_constant<M>(&mut self, expr: M) -> Constant<'tcx>
-        where M: Mirror<'tcx, Output=Expr<'tcx>>
+    where
+        M: Mirror<'tcx, Output = Expr<'tcx>>,
     {
         let expr = self.hir.mirror(expr);
         self.expr_as_constant(expr)
     }
 
     fn expr_as_constant(&mut self, expr: Expr<'tcx>) -> Constant<'tcx> {
         let this = self;
-        let Expr { ty, temp_lifetime: _, span, kind }
-            = expr;
+        let Expr {
+            ty,
+            temp_lifetime: _,
+            span,
+            kind,
+        } = expr;
         match kind {
-            ExprKind::Scope { region_scope: _, lint_level: _, value } =>
-                this.as_constant(value),
-            ExprKind::Literal { literal, user_ty } =>
-                Constant { span, ty, user_ty, literal },
-            _ =>
-                span_bug!(
-                    span,
-                    "expression is not a valid constant {:?}",
-                    kind),
+            ExprKind::Scope {
+                region_scope: _,
+                lint_level: _,
+                value,
+            } => this.as_constant(value),
+            ExprKind::Literal { literal, user_ty } => Constant {
+                span,
+                ty,
+                user_ty,
+                literal,
+            },
+            _ => span_bug!(span, "expression is not a valid constant {:?}", kind),
         }
     }
 }