Auto merge of rust-lang#50603 - eddyb:issue-49955, r=nikomatsakis

 rustc_mir: allow promotion of promotable temps indexed at runtime.

Fixes rust-lang#49955.

r? @nikomatsakis

Author: bors

src/librustc_mir/transform/promote_consts.rs

@@ -25,14 +25,12 @@
 use rustc::mir::*;
 use rustc::mir::visit::{PlaceContext, MutVisitor, Visitor};
 use rustc::mir::traversal::ReversePostorder;
-use rustc::ty::TyCtxt;
+use rustc::ty::{self, TyCtxt};
 use syntax_pos::Span;
 
 use rustc_data_structures::indexed_vec::{IndexVec, Idx};
 
-use std::iter;
-use std::mem;
-use std::usize;
+use std::{cmp, iter, mem, usize};
 
 /// State of a temporary during collection and promotion.
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
@@ -150,9 +148,11 @@ pub fn collect_temps(mir: &Mir, rpo: &mut ReversePostorder) -> IndexVec<Local, T
 }
 
 struct Promoter<'a, 'tcx: 'a> {
+    tcx: TyCtxt<'a, 'tcx, 'tcx>,
     source: &'a mut Mir<'tcx>,
     promoted: Mir<'tcx>,
     temps: &'a mut IndexVec<Local, TempState>,
+    extra_statements: &'a mut Vec<(Location, Statement<'tcx>)>,
 
     /// If true, all nested temps are also kept in the
     /// source MIR, not moved to the promoted MIR.
@@ -288,38 +288,90 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
     }
 
     fn promote_candidate(mut self, candidate: Candidate) {
-        let span = self.promoted.span;
-        let new_operand = Operand::Constant(box Constant {
-            span,
-            ty: self.promoted.return_ty(),
-            literal: Literal::Promoted {
+        let mut rvalue = {
+            let promoted = &mut self.promoted;
+            let literal = Literal::Promoted {
                 index: Promoted::new(self.source.promoted.len())
-            }
-        });
-        let mut rvalue = match candidate {
-            Candidate::Ref(Location { block: bb, statement_index: stmt_idx }) => {
-                let ref mut statement = self.source[bb].statements[stmt_idx];
-                match statement.kind {
-                    StatementKind::Assign(_, ref mut rvalue) => {
-                        mem::replace(rvalue, Rvalue::Use(new_operand))
+            };
+            let operand = |ty, span| {
+                promoted.span = span;
+                promoted.local_decls[RETURN_PLACE] =
+                    LocalDecl::new_return_place(ty, span);
+                Operand::Constant(box Constant {
+                    span,
+                    ty,
+                    literal
+                })
+            };
+            let (blocks, local_decls) = self.source.basic_blocks_and_local_decls_mut();
+            match candidate {
+                Candidate::Ref(loc) => {
+                    let ref mut statement = blocks[loc.block].statements[loc.statement_index];
+                    match statement.kind {
+                        StatementKind::Assign(_, Rvalue::Ref(r, bk, ref mut place)) => {
+                            // Find the underlying local for this (necessarilly interior) borrow.
+                            // HACK(eddyb) using a recursive function because of mutable borrows.
+                            fn interior_base<'a, 'tcx>(place: &'a mut Place<'tcx>)
+                                                       -> &'a mut Place<'tcx> {
+                                if let Place::Projection(ref mut proj) = *place {
+                                    assert_ne!(proj.elem, ProjectionElem::Deref);
+                                    return interior_base(&mut proj.base);
+                                }
+                                place
+                            }
+                            let place = interior_base(place);
+
+                            let ty = place.ty(local_decls, self.tcx).to_ty(self.tcx);
+                            let ref_ty = self.tcx.mk_ref(r,
+                                ty::TypeAndMut {
+                                    ty,
+                                    mutbl: bk.to_mutbl_lossy()
+                                }
+                            );
+                            let span = statement.source_info.span;
+
+                            // Create a temp to hold the promoted reference.
+                            // This is because `*r` requires `r` to be a local,
+                            // otherwise we would use the `promoted` directly.
+                            let mut promoted_ref = LocalDecl::new_temp(ref_ty, span);
+                            promoted_ref.source_info = statement.source_info;
+                            let promoted_ref = local_decls.push(promoted_ref);
+                            assert_eq!(self.temps.push(TempState::Unpromotable), promoted_ref);
+                            self.extra_statements.push((loc, Statement {
+                                source_info: statement.source_info,
+                                kind: StatementKind::Assign(
+                                    Place::Local(promoted_ref),
+                                    Rvalue::Use(operand(ref_ty, span)),
+                                )
+                            }));
+                            let promoted_place = Place::Local(promoted_ref).deref();
+
+                            Rvalue::Ref(r, bk, mem::replace(place, promoted_place))
+                        }
+                        _ => bug!()
                     }
-                    _ => bug!()
                 }
-            }
-            Candidate::Argument { bb, index } => {
-                match self.source[bb].terminator_mut().kind {
-                    TerminatorKind::Call { ref mut args, .. } => {
-                        Rvalue::Use(mem::replace(&mut args[index], new_operand))
+                Candidate::Argument { bb, index } => {
+                    let terminator = blocks[bb].terminator_mut();
+                    match terminator.kind {
+                        TerminatorKind::Call { ref mut args, .. } => {
+                            let ty = args[index].ty(local_decls, self.tcx);
+                            let span = terminator.source_info.span;
+                            Rvalue::Use(mem::replace(&mut args[index], operand(ty, span)))
+                        }
+                        _ => bug!()
                     }
-                    _ => bug!()
                 }
             }
         };
+
+        assert_eq!(self.new_block(), START_BLOCK);
         self.visit_rvalue(&mut rvalue, Location {
             block: BasicBlock::new(0),
             statement_index: usize::MAX
         });
 
+        let span = self.promoted.span;
         self.assign(RETURN_PLACE, rvalue, span);
         self.source.promoted.push(self.promoted);
     }
@@ -343,43 +395,29 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>,
                                     candidates: Vec<Candidate>) {
     // Visit candidates in reverse, in case they're nested.
     debug!("promote_candidates({:?})", candidates);
+
+    let mut extra_statements = vec![];
     for candidate in candidates.into_iter().rev() {
-        let (span, ty) = match candidate {
-            Candidate::Ref(Location { block: bb, statement_index: stmt_idx }) => {
-                let statement = &mir[bb].statements[stmt_idx];
-                let dest = match statement.kind {
-                    StatementKind::Assign(ref dest, _) => dest,
-                    _ => {
-                        span_bug!(statement.source_info.span,
-                                  "expected assignment to promote");
-                    }
-                };
-                if let Place::Local(index) = *dest {
-                    if temps[index] == TempState::PromotedOut {
-                        // Already promoted.
-                        continue;
+        match candidate {
+            Candidate::Ref(Location { block, statement_index }) => {
+                match mir[block].statements[statement_index].kind {
+                    StatementKind::Assign(Place::Local(local), _) => {
+                        if temps[local] == TempState::PromotedOut {
+                            // Already promoted.
+                            continue;
+                        }
                     }
+                    _ => {}
                 }
-                (statement.source_info.span, dest.ty(mir, tcx).to_ty(tcx))
-            }
-            Candidate::Argument { bb, index } => {
-                let terminator = mir[bb].terminator();
-                let ty = match terminator.kind {
-                    TerminatorKind::Call { ref args, .. } => {
-                        args[index].ty(mir, tcx)
-                    }
-                    _ => {
-                        span_bug!(terminator.source_info.span,
-                                  "expected call argument to promote");
-                    }
-                };
-                (terminator.source_info.span, ty)
             }
-        };
+            Candidate::Argument { .. } => {}
+        }
+
 
-        // Declare return place local
-        let initial_locals = iter::once(LocalDecl::new_return_place(ty, span))
-            .collect();
+        // Declare return place local so that `Mir::new` doesn't complain.
+        let initial_locals = iter::once(
+            LocalDecl::new_return_place(tcx.types.never, mir.span)
+        ).collect();
 
         let mut promoter = Promoter {
             promoted: Mir::new(
@@ -393,16 +431,24 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>,
                 initial_locals,
                 0,
                 vec![],
-                span
+                mir.span
             ),
+            tcx,
             source: mir,
             temps: &mut temps,
+            extra_statements: &mut extra_statements,
             keep_original: false
         };
-        assert_eq!(promoter.new_block(), START_BLOCK);
         promoter.promote_candidate(candidate);
     }
 
+    // Insert each of `extra_statements` before its indicated location, which
+    // has to be done in reverse location order, to not invalidate the rest.
+    extra_statements.sort_by_key(|&(loc, _)| cmp::Reverse(loc));
+    for (loc, statement) in extra_statements {
+        mir[loc.block].statements.insert(loc.statement_index, statement);
+    }
+
     // Eliminate assignments to, and drops of promoted temps.
     let promoted = |index: Local| temps[index] == TempState::PromotedOut;
     for block in mir.basic_blocks_mut() {

src/librustc_mir/transform/qualify_consts.rs

@@ -229,12 +229,12 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
     }
 
     /// Check if a Local with the current qualifications is promotable.
-    fn can_promote(&mut self) -> bool {
+    fn can_promote(&self, qualif: Qualif) -> bool {
         // References to statics are allowed, but only in other statics.
         if self.mode == Mode::Static || self.mode == Mode::StaticMut {
-            (self.qualif - Qualif::STATIC_REF).is_empty()
+            (qualif - Qualif::STATIC_REF).is_empty()
         } else {
-            self.qualif.is_empty()
+            qualif.is_empty()
         }
     }
 
@@ -679,24 +679,31 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
                 }
 
                 let ty = place.ty(self.mir, self.tcx).to_ty(self.tcx);
+
+                // Default to forbidding the borrow and/or its promotion,
+                // due to the potential for direct or interior mutability,
+                // and only proceed by setting `forbidden_mut` to `false`.
+                let mut forbidden_mut = true;
+
                 if let BorrowKind::Mut { .. } = kind {
                     // In theory, any zero-sized value could be borrowed
                     // mutably without consequences. However, only &mut []
                     // is allowed right now, and only in functions.
-                    let allow = if self.mode == Mode::StaticMut {
+                    if self.mode == Mode::StaticMut {
                         // Inside a `static mut`, &mut [...] is also allowed.
                         match ty.sty {
-                            ty::TyArray(..) | ty::TySlice(_) => true,
-                            _ => false
+                            ty::TyArray(..) | ty::TySlice(_) => forbidden_mut = false,
+                            _ => {}
                         }
                     } else if let ty::TyArray(_, len) = ty.sty {
-                        len.unwrap_usize(self.tcx) == 0 &&
-                            self.mode == Mode::Fn
-                    } else {
-                        false
-                    };
+                        // FIXME(eddyb) the `self.mode == Mode::Fn` condition
+                        // seems unnecessary, given that this is merely a ZST.
+                        if len.unwrap_usize(self.tcx) == 0 && self.mode == Mode::Fn {
+                            forbidden_mut = false;
+                        }
+                    }
 
-                    if !allow {
+                    if forbidden_mut {
                         self.add(Qualif::NOT_CONST);
                         if self.mode != Mode::Fn {
                             let mut err = struct_span_err!(self.tcx.sess,  self.span, E0017,
@@ -722,25 +729,46 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
                     // it means that our "silent insertion of statics" could change
                     // initializer values (very bad).
                     if self.qualif.intersects(Qualif::MUTABLE_INTERIOR) {
-                        // Replace MUTABLE_INTERIOR with NOT_CONST to avoid
+                        // A reference of a MUTABLE_INTERIOR place is instead
+                        // NOT_CONST (see `if forbidden_mut` below), to avoid
                         // duplicate errors (from reborrowing, for example).
                         self.qualif = self.qualif - Qualif::MUTABLE_INTERIOR;
-                        self.add(Qualif::NOT_CONST);
                         if self.mode != Mode::Fn {
                             span_err!(self.tcx.sess, self.span, E0492,
                                       "cannot borrow a constant which may contain \
                                        interior mutability, create a static instead");
                         }
+                    } else {
+                        // We allow immutable borrows of frozen data.
+                        forbidden_mut = false;
                     }
                 }
 
-                // We might have a candidate for promotion.
-                let candidate = Candidate::Ref(location);
-                if self.can_promote() {
-                    // We can only promote direct borrows of temps.
+                if forbidden_mut {
+                    self.add(Qualif::NOT_CONST);
+                } else {
+                    // We might have a candidate for promotion.
+                    let candidate = Candidate::Ref(location);
+                    // We can only promote interior borrows of promotable temps.
+                    let mut place = place;
+                    while let Place::Projection(ref proj) = *place {
+                        if proj.elem == ProjectionElem::Deref {
+                            break;
+                        }
+                        place = &proj.base;
+                    }
                     if let Place::Local(local) = *place {
                         if self.mir.local_kind(local) == LocalKind::Temp {
-                            self.promotion_candidates.push(candidate);
+                            if let Some(qualif) = self.temp_qualif[local] {
+                                // `forbidden_mut` is false, so we can safely ignore
+                                // `MUTABLE_INTERIOR` from the local's qualifications.
+                                // This allows borrowing fields which don't have
+                                // `MUTABLE_INTERIOR`, from a type that does, e.g.:
+                                // `let _: &'static _ = &(Cell::new(1), 2).1;`
+                                if self.can_promote(qualif - Qualif::MUTABLE_INTERIOR) {
+                                    self.promotion_candidates.push(candidate);
+                                }
+                            }
                         }
                     }
                 }
@@ -897,7 +925,7 @@ This does not pose a problem by itself because they can't be accessed directly."
                     }
                     let candidate = Candidate::Argument { bb, index: i };
                     if is_shuffle && i == 2 {
-                        if this.can_promote() {
+                        if this.can_promote(this.qualif) {
                             this.promotion_candidates.push(candidate);
                         } else {
                             span_err!(this.tcx.sess, this.span, E0526,
@@ -913,7 +941,7 @@ This does not pose a problem by itself because they can't be accessed directly."
                     if !constant_arguments.contains(&i) {
                         return
                     }
-                    if this.can_promote() {
+                    if this.can_promote(this.qualif) {
                         this.promotion_candidates.push(candidate);
                     } else {
                         this.tcx.sess.span_err(this.span,

src/test/mir-opt/end_region_destruction_extents_1.rs

@@ -130,17 +130,21 @@ unsafe impl<'a, #[may_dangle] 'b> Drop for D1<'a, 'b> {
 //     let mut _7: &'10s S1;
 //     let mut _8: &'10s S1;
 //     let mut _9: S1;
+//     let mut _10: &'10s S1;
+//     let mut _11: &'12ds S1;
 //
 //     bb0: {
 //         StorageLive(_2);
 //         StorageLive(_3);
 //         StorageLive(_4);
 //         StorageLive(_5);
-//         _5 = promoted[1];
+//         _11 = promoted[1];
+//         _5 = &'12ds (*_11);
 //         _4 = &'12ds (*_5);
 //         StorageLive(_7);
 //         StorageLive(_8);
-//         _8 = promoted[0];
+//         _10 = promoted[0];
+//         _8 = &'10s (*_10);
 //         _7 = &'10s (*_8);
 //         _3 = D1<'12ds, '10s>::{{constructor}}(move _4, move _7);
 //         EndRegion('10s);

src/test/mir-opt/match_false_edges.rs

@@ -88,7 +88,8 @@ fn main() {
 //  }
 //  bb9: { // binding1 and guard
 //      StorageLive(_5);
-//      _5 = &((_2 as Some).0: i32);
+//      _11 = promoted[0];
+//      _5 = &(((*_11) as Some).0: i32);
 //      StorageLive(_8);
 //      _8 = const guard() -> [return: bb10, unwind: bb1];
 //  }