Auto merge of #81257 - pnkfelix:issue-80949-short-term-resolution-via-revert-of-pr-78373, r=matthewjasper

Revert 78373 ("dont leak return value after panic in drop")

Short term resolution for issue #80949.

Reopen #47949 after this lands.

(We plan to fine-tune PR #78373 to not run into this problem.)

Author: bors

compiler/rustc_mir_build/src/build/block.rs

@@ -3,7 +3,6 @@ use crate::build::ForGuard::OutsideGuard;
 use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
 use crate::thir::*;
 use rustc_hir as hir;
-use rustc_middle::middle::region;
 use rustc_middle::mir::*;
 use rustc_session::lint::builtin::UNSAFE_OP_IN_UNSAFE_FN;
 use rustc_session::lint::Level;
@@ -13,7 +12,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     crate fn ast_block(
         &mut self,
         destination: Place<'tcx>,
-        scope: Option<region::Scope>,
         block: BasicBlock,
         ast_block: &'tcx hir::Block<'tcx>,
         source_info: SourceInfo,
@@ -30,10 +28,9 @@ 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, scope, span, |this| {
+                    this.in_breakable_scope(None, destination, span, |this| {
                         Some(this.ast_block_stmts(
                             destination,
-                            scope,
                             block,
                             span,
                             stmts,
@@ -42,7 +39,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         ))
                     })
                 } else {
-                    this.ast_block_stmts(destination, scope, block, span, stmts, expr, safety_mode)
+                    this.ast_block_stmts(destination, block, span, stmts, expr, safety_mode)
                 }
             })
         })
@@ -51,7 +48,6 @@ 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>>,
@@ -186,7 +182,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             };
             this.block_context.push(BlockFrame::TailExpr { tail_result_is_ignored, span });
 
-            unpack!(block = this.into(destination, scope, block, expr));
+            unpack!(block = this.into(destination, block, expr));
             let popped = this.block_context.pop();
 
             assert!(popped.map_or(false, |bf| bf.is_tail_expr()));

compiler/rustc_mir_build/src/build/expr/as_rvalue.rs

@@ -12,8 +12,6 @@ use rustc_middle::mir::*;
 use rustc_middle::ty::{self, Ty, UpvarSubsts};
 use rustc_span::Span;
 
-use std::slice;
-
 impl<'a, 'tcx> Builder<'a, 'tcx> {
     /// Returns an rvalue suitable for use until the end of the current
     /// scope expression.
@@ -115,19 +113,12 @@ 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. No scope is needed since the
-                // `Box` is already scheduled to be dropped.
+                // initialize the box contents:
                 unpack!(
-                    block = this.into(
-                        this.hir.tcx().mk_place_deref(Place::from(result)),
-                        None,
-                        block,
-                        value,
-                    )
+                    block =
+                        this.into(this.hir.tcx().mk_place_deref(Place::from(result)), block, value)
                 );
-                let result_operand = Operand::Move(Place::from(result));
-                this.record_operands_moved(slice::from_ref(&result_operand));
-                block.and(Rvalue::Use(result_operand))
+                block.and(Rvalue::Use(Operand::Move(Place::from(result))))
             }
             ExprKind::Cast { source } => {
                 let source = unpack!(block = this.as_operand(block, scope, source));
@@ -171,7 +162,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     .map(|f| unpack!(block = this.as_operand(block, scope, f)))
                     .collect();
 
-                this.record_operands_moved(&fields);
                 block.and(Rvalue::Aggregate(box AggregateKind::Array(el_ty), fields))
             }
             ExprKind::Tuple { fields } => {
@@ -182,7 +172,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     .map(|f| unpack!(block = this.as_operand(block, scope, f)))
                     .collect();
 
-                this.record_operands_moved(&fields);
                 block.and(Rvalue::Aggregate(box AggregateKind::Tuple, fields))
             }
             ExprKind::Closure { closure_id, substs, upvars, movability } => {
@@ -234,7 +223,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     }
                     UpvarSubsts::Closure(substs) => box AggregateKind::Closure(closure_id, substs),
                 };
-                this.record_operands_moved(&operands);
                 block.and(Rvalue::Aggregate(result, operands))
             }
             ExprKind::Assign { .. } | ExprKind::AssignOp { .. } => {

compiler/rustc_mir_build/src/build/expr/as_temp.rs

@@ -114,7 +114,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             }
         }
 
-        unpack!(block = this.into(temp_place, temp_lifetime, block, expr));
+        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);
+        }
 
         block.and(temp)
     }

compiler/rustc_mir_build/src/build/expr/into.rs

@@ -1,35 +1,25 @@
 //! 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::thir::*;
 use rustc_ast::InlineAsmOptions;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_hir as hir;
-use rustc_middle::middle::region;
 use rustc_middle::mir::*;
 use rustc_middle::ty::CanonicalUserTypeAnnotation;
 
-use std::slice;
-
 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={:?}, scope={:?}, block={:?}, expr={:?})",
-            destination, scope, block, expr
-        );
+        debug!("into_expr(destination={:?}, block={:?}, expr={:?})", destination, block, expr);
 
         // since we frequently have to reference `self` from within a
         // closure, where `self` would be shadowed, it's easier to
@@ -41,14 +31,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         let expr_is_block_or_scope =
             matches!(expr.kind, ExprKind::Block { .. } | ExprKind::Scope { .. });
 
-        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);
         }
@@ -58,15 +40,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let region_scope = (region_scope, source_info);
                 ensure_sufficient_stack(|| {
                     this.in_scope(region_scope, lint_level, |this| {
-                        this.into(destination, scope, block, value)
+                        this.into(destination, block, value)
                     })
                 })
             }
             ExprKind::Block { body: ast_block } => {
-                this.ast_block(destination, scope, block, ast_block, source_info)
+                this.ast_block(destination, block, ast_block, source_info)
             }
             ExprKind::Match { scrutinee, arms } => {
-                this.match_expr(destination, scope, expr_span, block, scrutinee, arms)
+                this.match_expr(destination, expr_span, block, scrutinee, arms)
             }
             ExprKind::If { cond, then, else_opt } => {
                 let place = unpack!(
@@ -79,9 +61,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let term = TerminatorKind::if_(this.hir.tcx(), operand, then_block, else_block);
                 this.cfg.terminate(block, source_info, term);
 
-                unpack!(then_block = this.into(destination, scope, then_block, then));
+                unpack!(then_block = this.into(destination, then_block, then));
                 else_block = if let Some(else_opt) = else_opt {
-                    unpack!(this.into(destination, None, else_block, else_opt))
+                    unpack!(this.into(destination, else_block, else_opt))
                 } else {
                     // Body of the `if` expression without an `else` clause must return `()`, thus
                     // we implicitly generate a `else {}` if it is not specified.
@@ -117,7 +99,6 @@ 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 {
@@ -193,35 +174,26 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 // Start the loop.
                 this.cfg.goto(block, source_info, loop_block);
 
-                this.in_breakable_scope(
-                    Some(loop_block),
-                    destination,
-                    scope,
-                    expr_span,
-                    move |this| {
-                        // conduct the test, if necessary
-                        let body_block = this.cfg.start_new_block();
-                        this.cfg.terminate(
-                            loop_block,
-                            source_info,
-                            TerminatorKind::FalseUnwind { real_target: body_block, unwind: None },
-                        );
-                        this.diverge_from(loop_block);
-
-                        // The “return” value of the loop body must always be an unit. We therefore
-                        // 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.
-                        // We don't need to provide a drop scope because `tmp`
-                        // has type `()`.
-                        let body_block_end = unpack!(this.into(tmp, None, body_block, body));
-                        this.cfg.goto(body_block_end, source_info, loop_block);
-                        schedule_drop(this);
-
-                        // Loops are only exited by `break` expressions.
-                        None
-                    },
-                )
+                this.in_breakable_scope(Some(loop_block), destination, expr_span, move |this| {
+                    // conduct the test, if necessary
+                    let body_block = this.cfg.start_new_block();
+                    this.cfg.terminate(
+                        loop_block,
+                        source_info,
+                        TerminatorKind::FalseUnwind { real_target: body_block, unwind: None },
+                    );
+                    this.diverge_from(loop_block);
+
+                    // The “return” value of the loop body must always be an unit. We therefore
+                    // 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);
+
+                    // Loops are only exited by `break` expressions.
+                    None
+                })
             }
             ExprKind::Call { ty: _, fun, args, from_hir_call, fn_span } => {
                 let fun = unpack!(block = this.as_local_operand(block, fun));
@@ -256,10 +228,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     },
                 );
                 this.diverge_from(block);
-                schedule_drop(this);
                 success.unit()
             }
-            ExprKind::Use { source } => this.into(destination, scope, block, source),
+            ExprKind::Use { source } => this.into(destination, 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
@@ -341,14 +312,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     user_ty,
                     active_field_index,
                 );
-                this.record_operands_moved(&fields);
                 this.cfg.push_assign(
                     block,
                     source_info,
                     destination,
                     Rvalue::Aggregate(adt, fields),
                 );
-                schedule_drop(this);
                 block.unit()
             }
             ExprKind::InlineAsm { template, operands, options, line_spans } => {
@@ -445,7 +414,6 @@ 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 { .. } => {
@@ -463,22 +431,19 @@ 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::Yield { value } => {
                 let scope = this.local_scope();
                 let value = unpack!(block = this.as_operand(block, Some(scope), value));
                 let resume = this.cfg.start_new_block();
-                this.record_operands_moved(slice::from_ref(&value));
                 this.cfg.terminate(
                     block,
                     source_info,
                     TerminatorKind::Yield { value, resume, resume_arg: destination, drop: None },
                 );
                 this.generator_drop_cleanup(block);
-                schedule_drop(this);
                 resume.unit()
             }
 
@@ -510,7 +475,6 @@ 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()
             }
         };

compiler/rustc_mir_build/src/build/expr/stmt.rs

@@ -3,7 +3,6 @@ use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
 use crate::thir::*;
 use rustc_middle::middle::region;
 use rustc_middle::mir::*;
-use std::slice;
 
 impl<'a, 'tcx> Builder<'a, 'tcx> {
     /// Builds a block of MIR statements to evaluate the THIR `expr`.
@@ -47,7 +46,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 if this.hir.needs_drop(lhs.ty) {
                     let rhs = unpack!(block = this.as_local_operand(block, rhs));
                     let lhs = unpack!(block = this.as_place(block, lhs));
-                    this.record_operands_moved(slice::from_ref(&rhs));
                     unpack!(block = this.build_drop_and_replace(block, lhs_span, lhs, rhs));
                 } else {
                     let rhs = unpack!(block = this.as_local_rvalue(block, rhs));