Auto merge of #30635 - nagisa:mir-rid-unit-temp, r=nikomatsakis

Get rid of that nasty unit_ty temporary variable created just because it might be handy to have one around, when in reality it isn’t really that useful at all.

r? @nikomatsakis

Fixes https://github.com/rust-lang/rust/issues/30637

6 files changed, 56 insertions, 46 deletions

diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs
index 5f7f87cb862..12b9130b48c 100644
--- a/src/librustc_mir/build/block.rs
+++ b/src/librustc_mir/build/block.rs
@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use build::{BlockAnd, Builder};
+use build::{BlockAnd, BlockAndExtension, Builder};
 use hair::*;
 use rustc::mir::repr::*;
 use rustc_front::hir;
@@ -19,11 +19,16 @@ impl<'a,'tcx> Builder<'a,'tcx> {
                      mut block: BasicBlock,
                      ast_block: &'tcx hir::Block)
                      -> BlockAnd<()> {
-        let this = self;
-        let Block { extent, span: _, stmts, expr } = this.hir.mirror(ast_block);
-        this.in_scope(extent, block, |this| {
+        let Block { extent, span, stmts, expr } = self.hir.mirror(ast_block);
+        self.in_scope(extent, block, move |this| {
             unpack!(block = this.stmts(block, stmts));
-            this.into(destination, block, expr)
+            match expr {
+                Some(expr) => this.into(destination, block, expr),
+                None => {
+                    this.cfg.push_assign_unit(block, span, destination);
+                    block.unit()
+                }
+            }
         })
     }
 }
diff --git a/src/librustc_mir/build/cfg.rs b/src/librustc_mir/build/cfg.rs
index 2e70e6bb5ae..523ac85cdc5 100644
--- a/src/librustc_mir/build/cfg.rs
+++ b/src/librustc_mir/build/cfg.rs
@@ -37,14 +37,6 @@ impl<'tcx> CFG<'tcx> {
         self.block_data_mut(block).statements.push(statement);
     }
 
-    pub fn push_assign_constant(&mut self,
-                                block: BasicBlock,
-                                span: Span,
-                                temp: &Lvalue<'tcx>,
-                                constant: Constant<'tcx>) {
-        self.push_assign(block, span, temp, Rvalue::Use(Operand::Constant(constant)));
-    }
-
     pub fn push_drop(&mut self, block: BasicBlock, span: Span,
                      kind: DropKind, lvalue: &Lvalue<'tcx>) {
         self.push(block, Statement {
@@ -64,6 +56,23 @@ impl<'tcx> CFG<'tcx> {
         });
     }
 
+    pub fn push_assign_constant(&mut self,
+                                block: BasicBlock,
+                                span: Span,
+                                temp: &Lvalue<'tcx>,
+                                constant: Constant<'tcx>) {
+        self.push_assign(block, span, temp, Rvalue::Use(Operand::Constant(constant)));
+    }
+
+    pub fn push_assign_unit(&mut self,
+                            block: BasicBlock,
+                            span: Span,
+                            lvalue: &Lvalue<'tcx>) {
+        self.push_assign(block, span, lvalue, Rvalue::Aggregate(
+            AggregateKind::Tuple, vec![]
+        ));
+    }
+
     pub fn terminate(&mut self,
                      block: BasicBlock,
                      terminator: Terminator<'tcx>) {
diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs
index 44d1d52a06a..63eb7607204 100644
--- a/src/librustc_mir/build/expr/into.rs
+++ b/src/librustc_mir/build/expr/into.rs
@@ -58,7 +58,14 @@ impl<'a,'tcx> Builder<'a,'tcx> {
                 });
 
                 unpack!(then_block = this.into(destination, then_block, then_expr));
-                unpack!(else_block = this.into(destination, else_block, else_expr));
+                else_block = if let Some(else_expr) = else_expr {
+                    unpack!(this.into(destination, else_block, else_expr))
+                } else {
+                    // Body of the `if` expression without an `else` clause must return `()`, thus
+                    // we implicitly generate a `else {}` if it is not specified.
+                    this.cfg.push_assign_unit(else_block, expr_span, &Lvalue::ReturnPointer);
+                    else_block
+                };
 
                 let join_block = this.cfg.start_new_block();
                 this.cfg.terminate(then_block, Terminator::Goto { target: join_block });
@@ -157,11 +164,18 @@ impl<'a,'tcx> Builder<'a,'tcx> {
                     }
 
                     // execute the body, branching back to the test
-                    let unit_temp = this.unit_temp.clone();
-                    let body_block_end = unpack!(this.into(&unit_temp, body_block, body));
+                    // We write body’s “return value” into the destination of loop. This is fine,
+                    // because:
+                    //
+                    // * In Rust both loop expression and its body are required to have `()`
+                    //   as the “return value”;
+                    // * The destination will be considered uninitialised (given it was
+                    //   uninitialised before the loop) during the first iteration, thus
+                    //   disallowing its use inside the body. Alternatively, if it was already
+                    //   initialised, the `destination` can only possibly have a value of `()`,
+                    //   therefore, “mutating” the destination during iteration is fine.
+                    let body_block_end = unpack!(this.into(destination, body_block, body));
                     this.cfg.terminate(body_block_end, Terminator::Goto { target: loop_block });
-
-                    // final point is exit_block
                     exit_block.unit()
                 })
             }
@@ -206,7 +220,13 @@ impl<'a,'tcx> Builder<'a,'tcx> {
                 this.break_or_continue(expr_span, label, block, |loop_scope| loop_scope.break_block)
             }
             ExprKind::Return { value } => {
-                unpack!(block = this.into(&Lvalue::ReturnPointer, block, value));
+                block = match value {
+                    Some(value) => unpack!(this.into(&Lvalue::ReturnPointer, block, value)),
+                    None => {
+                        this.cfg.push_assign_unit(block, expr_span, &Lvalue::ReturnPointer);
+                        block
+                    }
+                };
                 let extent = this.extent_of_outermost_scope();
                 this.exit_scope(expr_span, extent, block, END_BLOCK);
                 this.cfg.start_new_block().unit()
diff --git a/src/librustc_mir/build/into.rs b/src/librustc_mir/build/into.rs
index 66d6c49ef12..77d9d926328 100644
--- a/src/librustc_mir/build/into.rs
+++ b/src/librustc_mir/build/into.rs
@@ -14,7 +14,7 @@
 //! wrapped up as expressions (e.g. blocks). To make this ergonomic, we use this
 //! latter `EvalInto` trait.
 
-use build::{BlockAnd, BlockAndExtension, Builder};
+use build::{BlockAnd, Builder};
 use hair::*;
 use rustc::mir::repr::*;
 
@@ -58,16 +58,3 @@ impl<'tcx> EvalInto<'tcx> for Expr<'tcx> {
         builder.into_expr(destination, block, self)
     }
 }
-
-impl<'tcx> EvalInto<'tcx> for Option<ExprRef<'tcx>> {
-    fn eval_into<'a>(self,
-                     builder: &mut Builder<'a, 'tcx>,
-                     destination: &Lvalue<'tcx>,
-                     block: BasicBlock)
-                     -> BlockAnd<()> {
-        match self {
-            Some(expr) => builder.into(destination, block, expr),
-            None => block.unit(),
-        }
-    }
-}
diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs
index e6e5b8380b2..d217eb06647 100644
--- a/src/librustc_mir/build/mod.rs
+++ b/src/librustc_mir/build/mod.rs
@@ -23,7 +23,6 @@ pub struct Builder<'a, 'tcx: 'a> {
     cfg: CFG<'tcx>,
     scopes: Vec<scope::Scope<'tcx>>,
     loop_scopes: Vec<scope::LoopScope>,
-    unit_temp: Lvalue<'tcx>,
     var_decls: Vec<VarDecl<'tcx>>,
     var_indices: FnvHashMap<ast::NodeId, u32>,
     temp_decls: Vec<TempDecl<'tcx>>,
@@ -79,7 +78,7 @@ macro_rules! unpack {
 ///////////////////////////////////////////////////////////////////////////
 // construct() -- the main entry point for building MIR for a function
 
-pub fn construct<'a,'tcx>(mut hir: Cx<'a,'tcx>,
+pub fn construct<'a,'tcx>(hir: Cx<'a,'tcx>,
                           _span: Span,
                           implicit_arguments: Vec<Ty<'tcx>>,
                           explicit_arguments: Vec<(Ty<'tcx>, &'tcx hir::Pat)>,
@@ -89,20 +88,14 @@ pub fn construct<'a,'tcx>(mut hir: Cx<'a,'tcx>,
                           -> Mir<'tcx> {
     let cfg = CFG { basic_blocks: vec![] };
 
-    // it's handy to have a temporary of type `()` sometimes, so make
-    // one from the start and keep it available
-    let temp_decls = vec![TempDecl::<'tcx> { ty: hir.unit_ty() }];
-    let unit_temp = Lvalue::Temp(0);
-
     let mut builder = Builder {
         hir: hir,
         cfg: cfg,
         scopes: vec![],
         loop_scopes: vec![],
-        temp_decls: temp_decls,
+        temp_decls: vec![],
         var_decls: vec![],
         var_indices: FnvHashMap(),
-        unit_temp: unit_temp,
     };
 
     assert_eq!(builder.cfg.start_new_block(), START_BLOCK);
diff --git a/src/librustc_mir/hair/cx/mod.rs b/src/librustc_mir/hair/cx/mod.rs
index f9dd40defdf..b49dc6d8962 100644
--- a/src/librustc_mir/hair/cx/mod.rs
+++ b/src/librustc_mir/hair/cx/mod.rs
@@ -46,10 +46,6 @@ impl<'a,'tcx:'a> Cx<'a, 'tcx> {
         ast.make_mirror(self)
     }
 
-    pub fn unit_ty(&mut self) -> Ty<'tcx> {
-        self.tcx.mk_nil()
-    }
-
     pub fn usize_ty(&mut self) -> Ty<'tcx> {
         self.tcx.types.usize
     }