Make into schedule drop for the destination again

Author: matthewjasper

src/librustc_mir_build/build/block.rs

@@ -2,14 +2,16 @@ use crate::build::matches::ArmHasGuard;
 use crate::build::ForGuard::OutsideGuard;
 use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
 use crate::hair::*;
+use rustc::middle::region;
 use rustc::mir::*;
 use rustc_hir as hir;
 use rustc_span::Span;
 
 impl<'a, 'tcx> Builder<'a, 'tcx> {
-    crate fn ast_block(
+    pub fn ast_block(
         &mut self,
         destination: &Place<'tcx>,
+        scope: Option<region::Scope>,
         block: BasicBlock,
         ast_block: &'tcx hir::Block<'tcx>,
         source_info: SourceInfo,
@@ -26,9 +28,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         self.in_opt_scope(opt_destruction_scope.map(|de| (de, source_info)), move |this| {
             this.in_scope((region_scope, source_info), LintLevel::Inherited, move |this| {
                 if targeted_by_break {
-                    this.in_breakable_scope(None, destination.clone(), span, |this| {
+                    this.in_breakable_scope(None, destination.clone(), scope, span, |this| {
                         Some(this.ast_block_stmts(
                             destination,
+                            scope,
                             block,
                             span,
                             stmts,
@@ -37,7 +40,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         ))
                     })
                 } else {
-                    this.ast_block_stmts(destination, block, span, stmts, expr, safety_mode)
+                    this.ast_block_stmts(destination, scope, block, span, stmts, expr, safety_mode)
                 }
             })
         })
@@ -46,6 +49,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     fn ast_block_stmts(
         &mut self,
         destination: &Place<'tcx>,
+        scope: Option<region::Scope>,
         mut block: BasicBlock,
         span: Span,
         stmts: Vec<StmtRef<'tcx>>,
@@ -177,7 +181,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 destination_ty.is_unit() || this.block_context.currently_ignores_tail_results();
             this.block_context.push(BlockFrame::TailExpr { tail_result_is_ignored });
 
-            unpack!(block = this.into(destination, block, expr));
+            unpack!(block = this.into(destination, scope, block, expr));
             let popped = this.block_context.pop();
 
             assert!(popped.map_or(false, |bf| bf.is_tail_expr()));

src/librustc_mir_build/build/expr/as_rvalue.rs

@@ -111,12 +111,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let box_ = Rvalue::NullaryOp(NullOp::Box, value.ty);
                 this.cfg.push_assign(block, source_info, &Place::from(result), box_);
 
-                // initialize the box contents:
+                // Initialize the box contents. No scope is needed since the
+                // `Box` is already scheduled to be dropped.
                 unpack!(
                     block = this.into(
                         &this.hir.tcx().mk_place_deref(Place::from(result)),
+                        None,
                         block,
-                        value
+                        value,
                     )
                 );
                 block.and(Rvalue::Use(Operand::Move(Place::from(result))))

src/librustc_mir_build/build/expr/as_temp.rs

@@ -100,11 +100,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             }
         }
 
-        unpack!(block = this.into(temp_place, block, expr));
-
-        if let Some(temp_lifetime) = temp_lifetime {
-            this.schedule_drop(expr_span, temp_lifetime, temp, DropKind::Value);
-        }
+        unpack!(block = this.into(temp_place, temp_lifetime, block, expr));
 
         block.and(temp)
     }

src/librustc_mir_build/build/expr/into.rs

@@ -1,8 +1,10 @@
 //! See docs in build/expr/mod.rs
 
 use crate::build::expr::category::{Category, RvalueFunc};
+use crate::build::scope::DropKind;
 use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
 use crate::hair::*;
+use rustc::middle::region;
 use rustc::mir::*;
 use rustc::ty::{self, CanonicalUserTypeAnnotation};
 use rustc_data_structures::fx::FxHashMap;
@@ -14,13 +16,19 @@ use rustc_target::spec::abi::Abi;
 impl<'a, 'tcx> Builder<'a, 'tcx> {
     /// Compile `expr`, storing the result into `destination`, which
     /// is assumed to be uninitialized.
+    /// If a `drop_scope` is provided, `destination` is scheduled to be dropped
+    /// in `scope` once it has been initialized.
     crate fn into_expr(
         &mut self,
         destination: &Place<'tcx>,
+        scope: Option<region::Scope>,
         mut block: BasicBlock,
         expr: Expr<'tcx>,
     ) -> BlockAnd<()> {
-        debug!("into_expr(destination={:?}, block={:?}, expr={:?})", destination, block, expr);
+        debug!(
+            "into_expr(destination={:?}, scope={:?}, block={:?}, expr={:?})",
+            destination, scope, block, expr
+        );
 
         // since we frequently have to reference `self` from within a
         // closure, where `self` would be shadowed, it's easier to
@@ -35,20 +43,30 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             _ => false,
         };
 
+        let schedule_drop = move |this: &mut Self| {
+            if let Some(drop_scope) = scope {
+                let local =
+                    destination.as_local().expect("cannot schedule drop of non-Local place");
+                this.schedule_drop(expr_span, drop_scope, local, DropKind::Value);
+            }
+        };
+
         if !expr_is_block_or_scope {
             this.block_context.push(BlockFrame::SubExpr);
         }
 
         let block_and = match expr.kind {
             ExprKind::Scope { region_scope, lint_level, value } => {
                 let region_scope = (region_scope, source_info);
-                this.in_scope(region_scope, lint_level, |this| this.into(destination, block, value))
+                this.in_scope(region_scope, lint_level, |this| {
+                    this.into(destination, scope, block, value)
+                })
             }
             ExprKind::Block { body: ast_block } => {
-                this.ast_block(destination, block, ast_block, source_info)
+                this.ast_block(destination, scope, block, ast_block, source_info)
             }
             ExprKind::Match { scrutinee, arms } => {
-                this.match_expr(destination, expr_span, block, scrutinee, arms)
+                this.match_expr(destination, scope, expr_span, block, scrutinee, arms)
             }
             ExprKind::NeverToAny { source } => {
                 let source = this.hir.mirror(source);
@@ -63,6 +81,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
                 // 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.
+                schedule_drop(this);
                 if is_call {
                     block.unit()
                 } else {
@@ -141,6 +160,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 this.in_breakable_scope(
                     Some(loop_block),
                     destination.clone(),
+                    scope,
                     expr_span,
                     move |this| {
                         // conduct the test, if necessary
@@ -156,8 +176,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         // introduce a unit temporary as the destination for the loop body.
                         let tmp = this.get_unit_temp();
                         // Execute the body, branching back to the test.
-                        let body_block_end = unpack!(this.into(&tmp, body_block, body));
-                        this.cfg.goto(body_block_end, source_info, loop_block);
+                        // No scope is provided, since we've scheduled the drop above.
+                        let body_block_end = unpack!(this.into(&tmp, None, body_block, body));
+                        this.cfg.terminate(
+                            body_block_end,
+                            source_info,
+                            TerminatorKind::Goto { target: loop_block },
+                        );
+                        schedule_drop(this);
                         None
                     },
                 )
@@ -198,8 +224,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         is_block_tail: None,
                     });
                     let ptr_temp = Place::from(ptr_temp);
-                    let block = unpack!(this.into(&ptr_temp, block, ptr));
-                    this.into(&this.hir.tcx().mk_place_deref(ptr_temp), block, val)
+                    // No need for a scope, ptr_temp doesn't need drop
+                    let block = unpack!(this.into(&ptr_temp, None, block, ptr));
+                    // Maybe we should provide a scope here so that
+                    // `move_val_init` wouldn't leak on panic even with an
+                    // arbitrary `val` expression, but `schedule_drop`,
+                    // borrowck and drop elaboration all prevent us from
+                    // dropping `ptr_temp.deref()`.
+                    this.into(&this.hir.tcx().mk_place_deref(ptr_temp), None, block, val)
                 } else {
                     let args: Vec<_> = args
                         .into_iter()
@@ -229,10 +261,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                             from_hir_call,
                         },
                     );
+                    schedule_drop(this);
                     success.unit()
                 }
             }
-            ExprKind::Use { source } => this.into(destination, block, source),
+            ExprKind::Use { source } => this.into(destination, scope, block, source),
             ExprKind::Borrow { arg, borrow_kind } => {
                 // We don't do this in `as_rvalue` because we use `as_place`
                 // for borrow expressions, so we cannot create an `RValue` that
@@ -316,6 +349,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     destination,
                     Rvalue::Aggregate(adt, fields),
                 );
+                schedule_drop(this);
                 block.unit()
             }
 
@@ -341,6 +375,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let place = unpack!(block = this.as_place(block, expr));
                 let rvalue = Rvalue::Use(this.consume_by_copy_or_move(place));
                 this.cfg.push_assign(block, source_info, destination, rvalue);
+                schedule_drop(this);
                 block.unit()
             }
             ExprKind::Index { .. } | ExprKind::Deref { .. } | ExprKind::Field { .. } => {
@@ -358,6 +393,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let place = unpack!(block = this.as_place(block, expr));
                 let rvalue = Rvalue::Use(this.consume_by_copy_or_move(place));
                 this.cfg.push_assign(block, source_info, destination, rvalue);
+                schedule_drop(this);
                 block.unit()
             }
 
@@ -371,6 +407,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     source_info,
                     TerminatorKind::Yield { value, resume, resume_arg: *destination, drop: None },
                 );
+                schedule_drop(this);
                 resume.unit()
             }
 
@@ -400,6 +437,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
                 let rvalue = unpack!(block = this.as_local_rvalue(block, expr));
                 this.cfg.push_assign(block, source_info, destination, rvalue);
+                schedule_drop(this);
                 block.unit()
             }
         };

src/librustc_mir_build/build/into.rs

@@ -6,28 +6,31 @@
 
 use crate::build::{BlockAnd, Builder};
 use crate::hair::*;
+use rustc::middle::region;
 use rustc::mir::*;
 
 pub(in crate::build) trait EvalInto<'tcx> {
     fn eval_into(
         self,
         builder: &mut Builder<'_, 'tcx>,
         destination: &Place<'tcx>,
+        scope: Option<region::Scope>,
         block: BasicBlock,
     ) -> BlockAnd<()>;
 }
 
 impl<'a, 'tcx> Builder<'a, 'tcx> {
-    crate fn into<E>(
+    pub fn into<E>(
         &mut self,
         destination: &Place<'tcx>,
+        scope: Option<region::Scope>,
         block: BasicBlock,
         expr: E,
     ) -> BlockAnd<()>
     where
         E: EvalInto<'tcx>,
     {
-        expr.eval_into(self, destination, block)
+        expr.eval_into(self, destination, scope, block)
     }
 }
 
@@ -36,10 +39,11 @@ impl<'tcx> EvalInto<'tcx> for ExprRef<'tcx> {
         self,
         builder: &mut Builder<'_, 'tcx>,
         destination: &Place<'tcx>,
+        scope: Option<region::Scope>,
         block: BasicBlock,
     ) -> BlockAnd<()> {
         let expr = builder.hir.mirror(self);
-        builder.into_expr(destination, block, expr)
+        builder.into_expr(destination, scope, block, expr)
     }
 }
 
@@ -48,8 +52,9 @@ impl<'tcx> EvalInto<'tcx> for Expr<'tcx> {
         self,
         builder: &mut Builder<'_, 'tcx>,
         destination: &Place<'tcx>,
+        scope: Option<region::Scope>,
         block: BasicBlock,
     ) -> BlockAnd<()> {
-        builder.into_expr(destination, block, self)
+        builder.into_expr(destination, scope, block, self)
     }
 }