Extend handling of moved_locals in mir building to some unwind paths

Author: matthewjasper

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

@@ -738,7 +738,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 this.diverge_from(block);
                 block = success;
             }
-            this.record_operands_moved(&[Spanned { node: value_operand, span: DUMMY_SP }]);
+            this.record_operands_moved(&[value_operand]);
         }
         block.and(Rvalue::Aggregate(Box::new(AggregateKind::Array(elem_ty)), IndexVec::new()))
     }

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

@@ -239,27 +239,26 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             }
             ExprKind::Call { ty: _, fun, ref args, from_hir_call, fn_span } => {
                 let fun = unpack!(block = this.as_local_operand(block, fun));
-                let args: Box<[_]> = args
+                let spanned_args: Box<[_]> = args
                     .into_iter()
                     .copied()
                     .map(|arg| Spanned {
                         node: unpack!(block = this.as_local_call_operand(block, arg)),
                         span: this.thir.exprs[arg].span,
                     })
                     .collect();
+                let args: Vec<_> = spanned_args.iter().map(|arg| arg.node.clone()).collect();
 
                 let success = this.cfg.start_new_block();
 
-                this.record_operands_moved(&args);
-
                 debug!("expr_into_dest: fn_span={:?}", fn_span);
 
                 this.cfg.terminate(
                     block,
                     source_info,
                     TerminatorKind::Call {
                         func: fun,
-                        args,
+                        args: spanned_args,
                         unwind: UnwindAction::Continue,
                         destination,
                         // The presence or absence of a return edge affects control-flow sensitive
@@ -279,6 +278,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     },
                 );
                 this.diverge_from(block);
+
+                // This is here and not before `diverge_from` to avoid breaking
+                // the example in #80949.
+                // FIXME(matthewjasper): Look at this again if Polonius is
+                // stabilized.
+                this.record_operands_moved(&args);
                 success.unit()
             }
             ExprKind::Use { source } => this.expr_into_dest(destination, block, source),

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

@@ -106,14 +106,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
                 this.in_scope((region_scope, source_info), lint_level, |this| {
                     let fun = unpack!(block = this.as_local_operand(block, fun));
-                    let args: Box<[_]> = args
+                    let spanned_args: Box<[_]> = args
                         .into_iter()
                         .copied()
                         .map(|arg| Spanned {
                             node: unpack!(block = this.as_local_call_operand(block, arg)),
                             span: this.thir.exprs[arg].span,
                         })
                         .collect();
+                    let args: Vec<_> = spanned_args.iter().map(|arg| arg.node.clone()).collect();
 
                     this.record_operands_moved(&args);
 
@@ -124,7 +125,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     this.cfg.terminate(
                         block,
                         source_info,
-                        TerminatorKind::TailCall { func: fun, args, fn_span },
+                        TerminatorKind::TailCall { func: fun, args: spanned_args, fn_span },
                     );
 
                     this.cfg.start_new_block().unit()

compiler/rustc_mir_build/src/build/scope.rs

@@ -92,7 +92,6 @@ use rustc_middle::mir::*;
 use rustc_middle::thir::{ExprId, LintLevel};
 use rustc_middle::{bug, span_bug};
 use rustc_session::lint::Level;
-use rustc_span::source_map::Spanned;
 use rustc_span::{Span, DUMMY_SP};
 use tracing::{debug, instrument};
 
@@ -128,8 +127,6 @@ struct Scope {
     /// end of the vector (top of the stack) first.
     drops: Vec<DropData>,
 
-    moved_locals: Vec<Local>,
-
     /// The drop index that will drop everything in and below this scope on an
     /// unwind path.
     cached_unwind_block: Option<DropIdx>,
@@ -445,7 +442,6 @@ impl<'tcx> Scopes<'tcx> {
             source_scope: vis_scope,
             region_scope: region_scope.0,
             drops: vec![],
-            moved_locals: vec![],
             cached_unwind_block: None,
             cached_coroutine_drop_block: None,
         });
@@ -752,13 +748,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     pub(crate) fn break_for_tail_call(
         &mut self,
         mut block: BasicBlock,
-        args: &[Spanned<Operand<'tcx>>],
+        args: &[Operand<'tcx>],
         source_info: SourceInfo,
     ) -> BlockAnd<()> {
         let arg_drops: Vec<_> = args
             .iter()
             .rev()
-            .filter_map(|arg| match &arg.node {
+            .filter_map(|arg| match arg {
                 Operand::Copy(_) => bug!("copy op in tail call args"),
                 Operand::Move(place) => {
                     let local =
@@ -1102,14 +1098,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         span_bug!(span, "region scope {:?} not in scope to drop {:?}", region_scope, local);
     }
 
-    /// Indicates that the "local operand" stored in `local` is
+    /// Indicates that the "local operands" stored in `local` are
     /// *moved* at some point during execution (see `local_scope` for
     /// more information about what a "local operand" is -- in short,
     /// it's an intermediate operand created as part of preparing some
     /// MIR instruction). We use this information to suppress
-    /// redundant drops on the non-unwind paths. This results in less
-    /// MIR, but also avoids spurious borrow check errors
-    /// (c.f. #64391).
+    /// redundant drops. This results in less MIR, but also avoids spurious
+    /// borrow check errors (c.f. #64391).
     ///
     /// Example: when compiling the call to `foo` here:
     ///
@@ -1138,27 +1133,23 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     /// spurious borrow-check errors -- the problem, ironically, is
     /// not the `DROP(_X)` itself, but the (spurious) unwind pathways
     /// that it creates. See #64391 for an example.
-    pub(crate) fn record_operands_moved(&mut self, operands: &[Spanned<Operand<'tcx>>]) {
+    pub(crate) fn record_operands_moved(&mut self, operands: &[Operand<'tcx>]) {
         let local_scope = self.local_scope();
         let scope = self.scopes.scopes.last_mut().unwrap();
 
         assert_eq!(scope.region_scope, local_scope, "local scope is not the topmost scope!",);
 
         // look for moves of a local variable, like `MOVE(_X)`
-        let locals_moved = operands.iter().flat_map(|operand| match operand.node {
+        let locals_moved = operands.iter().flat_map(|operand| match operand {
             Operand::Copy(_) | Operand::Constant(_) => None,
             Operand::Move(place) => place.as_local(),
         });
 
         for local in locals_moved {
-            // check if we have a Drop for this operand and -- if so
-            // -- add it to the list of moved operands. Note that this
-            // local might not have been an operand created for this
-            // call, it could come from other places too.
-            if scope.drops.iter().any(|drop| drop.local == local && drop.kind == DropKind::Value) {
-                scope.moved_locals.push(local);
-            }
+            // Unschedule drops from the scope.
+            scope.drops.retain(|drop| drop.local != local || drop.kind != DropKind::Value);
         }
+        scope.invalidate_cache();
     }
 
     // Other
@@ -1382,14 +1373,6 @@ fn build_scope_drops<'tcx>(
                 debug_assert_eq!(unwind_drops.drops[unwind_to].data.kind, drop_data.kind);
                 unwind_to = unwind_drops.drops[unwind_to].next;
 
-                // If the operand has been moved, and we are not on an unwind
-                // path, then don't generate the drop. (We only take this into
-                // account for non-unwind paths so as not to disturb the
-                // caching mechanism.)
-                if scope.moved_locals.iter().any(|&o| o == local) {
-                    continue;
-                }
-
                 unwind_drops.add_entry_point(block, unwind_to);
 
                 let next = cfg.start_new_block();