Use correct drop scopes for if expressions

Author: matthewjasper

compiler/rustc_ast_lowering/src/expr.rs

@@ -443,9 +443,20 @@ impl<'hir> LoweringContext<'_, 'hir> {
         else_opt: Option<&Expr>,
     ) -> hir::ExprKind<'hir> {
         let cond = self.lower_expr(cond);
-        let then = self.arena.alloc(self.lower_block_expr(then));
-        let els = else_opt.map(|els| self.lower_expr(els));
-        hir::ExprKind::If(cond, then, els)
+        let wrapped_cond = match cond.kind {
+            hir::ExprKind::Let(..) => cond,
+            _ => self.expr_drop_temps(cond.span, cond, AttrVec::new()),
+        };
+        let then_expr = self.lower_block_expr(then);
+        if let Some(rslt) = else_opt {
+            hir::ExprKind::If(
+                wrapped_cond,
+                self.arena.alloc(then_expr),
+                Some(self.lower_expr(rslt)),
+            )
+        } else {
+            hir::ExprKind::If(wrapped_cond, self.arena.alloc(then_expr), None)
+        }
     }
 
     fn lower_expr_if_let(

compiler/rustc_mir_build/src/build/matches/mod.rs

@@ -35,6 +35,47 @@ use std::convert::TryFrom;
 use std::mem;
 
 impl<'a, 'tcx> Builder<'a, 'tcx> {
+    pub(crate) fn then_else_blocks(
+        &mut self,
+        mut block: BasicBlock,
+        expr: ExprRef<'tcx>,
+        source_info: SourceInfo,
+    ) -> (BasicBlock, BasicBlock) {
+        let this = self;
+        let expr = this.hir.mirror(expr);
+        let expr_span = expr.span;
+
+        match expr.kind {
+            ExprKind::Scope { region_scope, lint_level, value } => {
+                let region_scope = (region_scope, source_info);
+                let then_block;
+                let else_block = unpack!(
+                    then_block = this.in_scope(region_scope, lint_level, |this| {
+                        let (then_block, else_block) =
+                            this.then_else_blocks(block, value, source_info);
+                        then_block.and(else_block)
+                    })
+                );
+                (then_block, else_block)
+            }
+            ExprKind::Let { expr, pat } => {
+                // TODO: Use correct span.
+                this.lower_let(block, &expr, &pat, expr_span)
+            }
+            _ => {
+                let local_scope = Some(this.local_scope());
+                let place =
+                    unpack!(block = this.as_temp(block, local_scope, expr, Mutability::Mut));
+                let operand = Operand::Move(Place::from(place));
+                let then_block = this.cfg.start_new_block();
+                let else_block = this.cfg.start_new_block();
+                let term = TerminatorKind::if_(this.hir.tcx(), operand, then_block, else_block);
+                this.cfg.terminate(block, source_info, term);
+                (then_block, else_block)
+            }
+        }
+    }
+
     /// Generates MIR for a `match` expression.
     ///
     /// The MIR that we generate for a match looks like this.

compiler/rustc_passes/src/region.rs

@@ -233,14 +233,12 @@ fn resolve_expr<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, expr: &'tcx h
                 terminating(r.hir_id.local_id);
             }
 
-            hir::ExprKind::If(ref expr, ref then, Some(ref otherwise)) => {
-                terminating(expr.hir_id.local_id);
+            hir::ExprKind::If(_, ref then, Some(ref otherwise)) => {
                 terminating(then.hir_id.local_id);
                 terminating(otherwise.hir_id.local_id);
             }
 
-            hir::ExprKind::If(ref expr, ref then, None) => {
-                terminating(expr.hir_id.local_id);
+            hir::ExprKind::If(_, ref then, None) => {
                 terminating(then.hir_id.local_id);
             }
 
@@ -392,6 +390,24 @@ fn resolve_expr<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, expr: &'tcx h
             }
         }
 
+        hir::ExprKind::If(ref cond, ref then, Some(ref otherwise)) => {
+            // FIXME(matthewjasper): ideally the scope we use here would only
+            // contain the condition and then expression. This works, but
+            // can result in some extra drop flags.
+            visitor.cx.var_parent = visitor.cx.parent;
+            visitor.visit_expr(cond);
+            visitor.cx.var_parent = prev_cx.var_parent;
+            visitor.visit_expr(then);
+            visitor.visit_expr(otherwise);
+        }
+
+        hir::ExprKind::If(ref cond, ref then, None) => {
+            visitor.cx.var_parent = visitor.cx.parent;
+            visitor.visit_expr(cond);
+            visitor.cx.var_parent = prev_cx.var_parent;
+            visitor.visit_expr(then);
+        }
+
         _ => intravisit::walk_expr(visitor, expr),
     }