make operands live to the end of their containing expression

In MIR construction, operands need to live exactly until they are used,
which is during the (sub)expression that made the call to `as_operand`.

Before this PR, operands lived until the end of the temporary scope,
which was sometimes unnecessarily longer and sometimes too short.

Fixes #38669.

Author: arielb1

src/librustc_mir/build/expr/as_lvalue.rs

@@ -56,8 +56,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                 let (usize_ty, bool_ty) = (this.hir.usize_ty(), this.hir.bool_ty());
 
                 let slice = unpack!(block = this.as_lvalue(block, lhs));
-
-                let idx = unpack!(block = this.as_operand(block, index));
+                // extent=None so lvalue 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_operand(block, None, index));
 
                 // bounds check:
                 let (len, lt) = (this.temp(usize_ty.clone()), this.temp(bool_ty));
@@ -121,7 +123,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                     Some(Category::Lvalue) => false,
                     _ => true,
                 });
-                this.as_temp(block, expr)
+                this.as_temp(block, expr.temp_lifetime, expr)
             }
         }
     }

src/librustc_mir/build/expr/as_operand.rs

@@ -13,29 +13,52 @@
 use build::{BlockAnd, BlockAndExtension, Builder};
 use build::expr::category::Category;
 use hair::*;
+use rustc::middle::region::CodeExtent;
 use rustc::mir::*;
 
 impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
+    /// Returns an operand suitable for use until the end of the current
+    /// scope expression.
+    ///
+    /// The operand returned from this function will *not be valid* after
+    /// an ExprKind::Scope is passed, so please do *not* return it from
+    /// functions to avoid bad miscompiles.
+    pub fn as_local_operand<M>(&mut self, block: BasicBlock, expr: M)
+                             -> BlockAnd<Operand<'tcx>>
+        where M: Mirror<'tcx, Output = Expr<'tcx>>
+    {
+        let topmost_scope = self.topmost_scope(); // FIXME(#6393)
+        self.as_operand(block, Some(topmost_scope), expr)
+    }
+
     /// Compile `expr` into a value that can be used as an operand.
     /// If `expr` is an lvalue like `x`, this will introduce a
     /// temporary `tmp = x`, so that we capture the value of `x` at
     /// this time.
-    pub fn as_operand<M>(&mut self, block: BasicBlock, expr: M) -> BlockAnd<Operand<'tcx>>
+    ///
+    /// The operand is known to be live until the end of `scope`.
+    pub fn as_operand<M>(&mut self,
+                         block: BasicBlock,
+                         scope: Option<CodeExtent>,
+                         expr: M) -> BlockAnd<Operand<'tcx>>
         where M: Mirror<'tcx, Output = Expr<'tcx>>
     {
         let expr = self.hir.mirror(expr);
-        self.expr_as_operand(block, expr)
+        self.expr_as_operand(block, scope, expr)
     }
 
     fn expr_as_operand(&mut self,
                        mut block: BasicBlock,
+                       scope: Option<CodeExtent>,
                        expr: Expr<'tcx>)
                        -> BlockAnd<Operand<'tcx>> {
         debug!("expr_as_operand(block={:?}, expr={:?})", block, expr);
         let this = self;
 
         if let ExprKind::Scope { extent, value } = expr.kind {
-            return this.in_scope(extent, block, |this| this.as_operand(block, value));
+            return this.in_scope(extent, block, |this| {
+                this.as_operand(block, scope, value)
+            });
         }
 
         let category = Category::of(&expr.kind).unwrap();
@@ -47,7 +70,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             }
             Category::Lvalue |
             Category::Rvalue(..) => {
-                let operand = unpack!(block = this.as_temp(block, expr));
+                let operand =
+                    unpack!(block = this.as_temp(block, scope, expr));
                 block.and(Operand::Consume(operand))
             }
         }

src/librustc_mir/build/expr/as_rvalue.rs

@@ -21,22 +21,34 @@ use build::expr::category::{Category, RvalueFunc};
 use hair::*;
 use rustc_const_math::{ConstInt, ConstIsize};
 use rustc::middle::const_val::ConstVal;
+use rustc::middle::region::CodeExtent;
 use rustc::ty;
 use rustc::mir::*;
 use syntax::ast;
 use syntax_pos::Span;
 
 impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
+    /// See comment on `as_local_operand`
+    pub fn as_local_rvalue<M>(&mut self, block: BasicBlock, expr: M)
+                             -> BlockAnd<Rvalue<'tcx>>
+        where M: Mirror<'tcx, Output = Expr<'tcx>>
+    {
+        let topmost_scope = self.topmost_scope(); // FIXME(#6393)
+        self.as_rvalue(block, Some(topmost_scope), expr)
+    }
+
     /// Compile `expr`, yielding an rvalue.
-    pub fn as_rvalue<M>(&mut self, block: BasicBlock, expr: M) -> BlockAnd<Rvalue<'tcx>>
+    pub fn as_rvalue<M>(&mut self, block: BasicBlock, scope: Option<CodeExtent>, expr: M)
+                        -> BlockAnd<Rvalue<'tcx>>
         where M: Mirror<'tcx, Output = Expr<'tcx>>
     {
         let expr = self.hir.mirror(expr);
-        self.expr_as_rvalue(block, expr)
+        self.expr_as_rvalue(block, scope, expr)
     }
 
     fn expr_as_rvalue(&mut self,
                       mut block: BasicBlock,
+                      scope: Option<CodeExtent>,
                       expr: Expr<'tcx>)
                       -> BlockAnd<Rvalue<'tcx>> {
         debug!("expr_as_rvalue(block={:?}, expr={:?})", block, expr);
@@ -47,24 +59,24 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
 
         match expr.kind {
             ExprKind::Scope { extent, value } => {
-                this.in_scope(extent, block, |this| this.as_rvalue(block, value))
+                this.in_scope(extent, block, |this| this.as_rvalue(block, scope, value))
             }
             ExprKind::Repeat { value, count } => {
-                let value_operand = unpack!(block = this.as_operand(block, value));
+                let value_operand = unpack!(block = this.as_operand(block, scope, value));
                 block.and(Rvalue::Repeat(value_operand, count))
             }
             ExprKind::Borrow { region, borrow_kind, arg } => {
                 let arg_lvalue = unpack!(block = this.as_lvalue(block, arg));
                 block.and(Rvalue::Ref(region, borrow_kind, arg_lvalue))
             }
             ExprKind::Binary { op, lhs, rhs } => {
-                let lhs = unpack!(block = this.as_operand(block, lhs));
-                let rhs = unpack!(block = this.as_operand(block, rhs));
+                let lhs = unpack!(block = this.as_operand(block, scope, lhs));
+                let rhs = unpack!(block = this.as_operand(block, scope, rhs));
                 this.build_binary_op(block, op, expr_span, expr.ty,
                                      lhs, rhs)
             }
             ExprKind::Unary { op, arg } => {
-                let arg = unpack!(block = this.as_operand(block, arg));
+                let arg = unpack!(block = this.as_operand(block, scope, arg));
                 // Check for -MIN on signed integers
                 if this.hir.check_overflow() && op == UnOp::Neg && expr.ty.is_signed() {
                     let bool_ty = this.hir.bool_ty();
@@ -97,27 +109,27 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             ExprKind::Cast { source } => {
                 let source = this.hir.mirror(source);
 
-                let source = unpack!(block = this.as_operand(block, source));
+                let source = unpack!(block = this.as_operand(block, scope, source));
                 block.and(Rvalue::Cast(CastKind::Misc, source, expr.ty))
             }
             ExprKind::Use { source } => {
-                let source = unpack!(block = this.as_operand(block, source));
+                let source = unpack!(block = this.as_operand(block, scope, source));
                 block.and(Rvalue::Use(source))
             }
             ExprKind::ReifyFnPointer { source } => {
-                let source = unpack!(block = this.as_operand(block, source));
+                let source = unpack!(block = this.as_operand(block, scope, source));
                 block.and(Rvalue::Cast(CastKind::ReifyFnPointer, source, expr.ty))
             }
             ExprKind::UnsafeFnPointer { source } => {
-                let source = unpack!(block = this.as_operand(block, source));
+                let source = unpack!(block = this.as_operand(block, scope, source));
                 block.and(Rvalue::Cast(CastKind::UnsafeFnPointer, source, expr.ty))
             }
             ExprKind::ClosureFnPointer { source } => {
-                let source = unpack!(block = this.as_operand(block, source));
+                let source = unpack!(block = this.as_operand(block, scope, source));
                 block.and(Rvalue::Cast(CastKind::ClosureFnPointer, source, expr.ty))
             }
             ExprKind::Unsize { source } => {
-                let source = unpack!(block = this.as_operand(block, source));
+                let source = unpack!(block = this.as_operand(block, scope, source));
                 block.and(Rvalue::Cast(CastKind::Unsize, source, expr.ty))
             }
             ExprKind::Array { fields } => {
@@ -150,7 +162,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                 // first process the set of fields
                 let fields: Vec<_> =
                     fields.into_iter()
-                          .map(|f| unpack!(block = this.as_operand(block, f)))
+                          .map(|f| unpack!(block = this.as_operand(block, scope, f)))
                           .collect();
 
                 block.and(Rvalue::Aggregate(AggregateKind::Array, fields))
@@ -159,15 +171,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                 // first process the set of fields
                 let fields: Vec<_> =
                     fields.into_iter()
-                          .map(|f| unpack!(block = this.as_operand(block, f)))
+                          .map(|f| unpack!(block = this.as_operand(block, scope, f)))
                           .collect();
 
                 block.and(Rvalue::Aggregate(AggregateKind::Tuple, fields))
             }
             ExprKind::Closure { closure_id, substs, upvars } => { // see (*) above
                 let upvars =
                     upvars.into_iter()
-                          .map(|upvar| unpack!(block = this.as_operand(block, upvar)))
+                          .map(|upvar| unpack!(block = this.as_operand(block, scope, upvar)))
                           .collect();
                 block.and(Rvalue::Aggregate(AggregateKind::Closure(closure_id, substs), upvars))
             }
@@ -179,10 +191,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
 
                 // first process the set of fields that were provided
                 // (evaluating them in order given by user)
-                let fields_map: FxHashMap<_, _> =
-                    fields.into_iter()
-                          .map(|f| (f.name, unpack!(block = this.as_operand(block, f.expr))))
-                          .collect();
+                let fields_map: FxHashMap<_, _> = fields.into_iter()
+                    .map(|f| (f.name, unpack!(block = this.as_operand(block, scope, f.expr))))
+                    .collect();
 
                 let field_names = this.hir.all_fields(adt_def, variant_index);
 
@@ -235,7 +246,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                     Some(Category::Rvalue(RvalueFunc::AsRvalue)) => false,
                     _ => true,
                 });
-                let operand = unpack!(block = this.as_operand(block, expr));
+                let operand = unpack!(block = this.as_operand(block, scope, expr));
                 block.and(Rvalue::Use(operand))
             }
         }

src/librustc_mir/build/expr/as_temp.rs

@@ -13,29 +13,38 @@
 use build::{BlockAnd, BlockAndExtension, Builder};
 use build::expr::category::Category;
 use hair::*;
+use rustc::middle::region::CodeExtent;
 use rustc::mir::*;
 
 impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
     /// Compile `expr` into a fresh temporary. This is used when building
     /// up rvalues so as to freeze the value that will be consumed.
-    pub fn as_temp<M>(&mut self, block: BasicBlock, expr: M) -> BlockAnd<Lvalue<'tcx>>
+    pub fn as_temp<M>(&mut self,
+                      block: BasicBlock,
+                      temp_lifetime: Option<CodeExtent>,
+                      expr: M)
+                      -> BlockAnd<Lvalue<'tcx>>
         where M: Mirror<'tcx, Output = Expr<'tcx>>
     {
         let expr = self.hir.mirror(expr);
-        self.expr_as_temp(block, expr)
+        self.expr_as_temp(block, temp_lifetime, expr)
     }
 
-    fn expr_as_temp(&mut self, mut block: BasicBlock, expr: Expr<'tcx>) -> BlockAnd<Lvalue<'tcx>> {
+    fn expr_as_temp(&mut self,
+                    mut block: BasicBlock,
+                    temp_lifetime: Option<CodeExtent>,
+                    expr: Expr<'tcx>)
+                    -> BlockAnd<Lvalue<'tcx>> {
         debug!("expr_as_temp(block={:?}, expr={:?})", block, expr);
         let this = self;
 
-        if let ExprKind::Scope { extent, value } = expr.kind {
-            return this.in_scope(extent, block, |this| this.as_temp(block, value));
+        if let ExprKind::Scope { .. } = expr.kind {
+            span_bug!(expr.span, "unexpected scope expression in as_temp: {:?}",
+                      expr);
         }
 
         let expr_ty = expr.ty.clone();
         let temp = this.temp(expr_ty.clone());
-        let temp_lifetime = expr.temp_lifetime;
         let expr_span = expr.span;
         let source_info = this.source_info(expr_span);
 

src/librustc_mir/build/expr/into.rs

@@ -52,7 +52,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                     _ => false,
                 };
 
-                unpack!(block = this.as_rvalue(block, source));
+                unpack!(block = this.as_local_rvalue(block, source));
 
                 // This is an optimization. If the expression was a call then we already have an
                 // unreachable block. Don't bother to terminate it and create a new one.
@@ -65,7 +65,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                 }
             }
             ExprKind::If { condition: cond_expr, then: then_expr, otherwise: else_expr } => {
-                let operand = unpack!(block = this.as_operand(block, cond_expr));
+                let operand = unpack!(block = this.as_local_operand(block, cond_expr));
 
                 let mut then_block = this.cfg.start_new_block();
                 let mut else_block = this.cfg.start_new_block();
@@ -107,15 +107,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                     (this.cfg.start_new_block(), this.cfg.start_new_block(),
                      this.cfg.start_new_block(), this.cfg.start_new_block());
 
-                let lhs = unpack!(block = this.as_operand(block, lhs));
+                let lhs = unpack!(block = this.as_local_operand(block, lhs));
                 let blocks = match op {
                     LogicalOp::And => (else_block, false_block),
                     LogicalOp::Or => (true_block, else_block),
                 };
                 let term = TerminatorKind::if_(this.hir.tcx(), lhs, blocks.0, blocks.1);
                 this.cfg.terminate(block, source_info, term);
 
-                let rhs = unpack!(else_block = this.as_operand(else_block, rhs));
+                let rhs = unpack!(else_block = this.as_local_operand(else_block, rhs));
                 let term = TerminatorKind::if_(this.hir.tcx(), rhs, true_block, false_block);
                 this.cfg.terminate(else_block, source_info, term);
 
@@ -173,7 +173,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                         if let Some(cond_expr) = opt_cond_expr {
                             let loop_block_end;
                             let cond = unpack!(
-                                loop_block_end = this.as_operand(loop_block, cond_expr));
+                                loop_block_end = this.as_local_operand(loop_block, cond_expr));
                             body_block = this.cfg.start_new_block();
                             let term = TerminatorKind::if_(this.hir.tcx(), cond,
                                                            body_block, exit_block);
@@ -206,10 +206,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                     }
                     _ => false
                 };
-                let fun = unpack!(block = this.as_operand(block, fun));
+                let fun = unpack!(block = this.as_local_operand(block, fun));
                 let args: Vec<_> =
                     args.into_iter()
-                        .map(|arg| unpack!(block = this.as_operand(block, arg)))
+                        .map(|arg| unpack!(block = this.as_local_operand(block, arg)))
                         .collect();
 
                 let success = this.cfg.start_new_block();
@@ -265,7 +265,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                     _ => true,
                 });
 
-                let rvalue = unpack!(block = this.as_rvalue(block, expr));
+                let rvalue = unpack!(block = this.as_local_rvalue(block, expr));
                 this.cfg.push_assign(block, source_info, destination, rvalue);
                 block.unit()
             }

src/librustc_mir/build/expr/stmt.rs

@@ -38,14 +38,14 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                 // Generate better code for things that don't need to be
                 // dropped.
                 if this.hir.needs_drop(lhs.ty) {
-                    let rhs = unpack!(block = this.as_operand(block, rhs));
+                    let rhs = unpack!(block = this.as_local_operand(block, rhs));
                     let lhs = unpack!(block = this.as_lvalue(block, lhs));
                     unpack!(block = this.build_drop_and_replace(
                         block, lhs_span, lhs, rhs
                     ));
                     block.unit()
                 } else {
-                    let rhs = unpack!(block = this.as_rvalue(block, rhs));
+                    let rhs = unpack!(block = this.as_local_rvalue(block, rhs));
                     let lhs = unpack!(block = this.as_lvalue(block, lhs));
                     this.cfg.push_assign(block, source_info, &lhs, rhs);
                     block.unit()
@@ -64,7 +64,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                 let lhs_ty = lhs.ty;
 
                 // As above, RTL.
-                let rhs = unpack!(block = this.as_operand(block, rhs));
+                let rhs = unpack!(block = this.as_local_operand(block, rhs));
                 let lhs = unpack!(block = this.as_lvalue(block, lhs));
 
                 // we don't have to drop prior contents or anything
@@ -122,7 +122,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                     unpack!(block = this.as_lvalue(block, output))
                 }).collect();
                 let inputs = inputs.into_iter().map(|input| {
-                    unpack!(block = this.as_operand(block, input))
+                    unpack!(block = this.as_local_operand(block, input))
                 }).collect();
                 this.cfg.push(block, Statement {
                     source_info: source_info,

src/librustc_mir/build/matches/mod.rs

@@ -661,7 +661,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             // guard, this block is simply unreachable
             let guard = self.hir.mirror(guard);
             let source_info = self.source_info(guard.span);
-            let cond = unpack!(block = self.as_operand(block, guard));
+            let cond = unpack!(block = self.as_local_operand(block, guard));
             let otherwise = self.cfg.start_new_block();
             self.cfg.terminate(block, source_info,
                                TerminatorKind::if_(self.hir.tcx(), cond, arm_block, otherwise));