Unschedule moved locals

Cargo.lock

@@ -3764,6 +3764,7 @@ dependencies = [
  "rustc_span",
  "rustc_trait_selection",
  "smallvec",
+ "thin-vec",
  "tracing",
 ]
 

compiler/rustc_borrowck/Cargo.toml

@@ -23,5 +23,6 @@ rustc_session = { path = "../rustc_session" }
 rustc_span = { path = "../rustc_span" }
 rustc_trait_selection = { path = "../rustc_trait_selection" }
 smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }
+thin-vec = "0.2.18"
 tracing = "0.1"
 # tidy-alphabetical-end

compiler/rustc_borrowck/src/renumber.rs

@@ -1,9 +1,10 @@
-use rustc_index::IndexSlice;
+use rustc_index::{IndexSlice, IndexVec};
 use rustc_infer::infer::NllRegionVariableOrigin;
 use rustc_middle::mir::visit::{MutVisitor, TyContext};
-use rustc_middle::mir::{Body, ConstOperand, Location, Promoted};
+use rustc_middle::mir::*;
 use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt, TypeFoldable, fold_regions};
 use rustc_span::Symbol;
+use thin_vec::ThinVec;
 use tracing::{debug, instrument};
 
 use crate::BorrowckInferCtxt;
@@ -21,12 +22,51 @@ pub(crate) fn renumber_mir<'tcx>(
     let mut renumberer = RegionRenumberer { infcx };
 
     for body in promoted.iter_mut() {
+        split_critical_unwind_edges(body);
         renumberer.visit_body_preserves_cfg(body);
     }
 
+    split_critical_unwind_edges(body);
     renumberer.visit_body_preserves_cfg(body);
 }
 
+#[instrument(skip(body), level = "debug")]
+fn split_critical_unwind_edges(body: &mut Body<'_>) {
+    let predecessors: IndexVec<BasicBlock, _> =
+        body.basic_blocks.predecessors().iter().map(|preds| preds.len()).collect();
+    debug!(?predecessors);
+
+    let mut new_blocks = vec![];
+    for bb in predecessors.indices() {
+        let term = body.basic_blocks[bb].terminator();
+        let Some(&UnwindAction::Cleanup(unwind)) = term.unwind() else { continue };
+        if predecessors[unwind] <= 1 {
+            continue;
+        }
+
+        debug!("{bb:?} has critical unwind edge: {unwind:?}");
+        new_blocks.push((bb, unwind));
+    }
+
+    if new_blocks.is_empty() {
+        return;
+    }
+
+    debug!(?new_blocks);
+    let basic_blocks = body.basic_blocks.as_mut();
+    for (bb, target) in new_blocks {
+        let source_info = basic_blocks[bb].terminator().source_info;
+        let terminator = Terminator {
+            source_info,
+            kind: TerminatorKind::Goto { target },
+            attributes: ThinVec::new(),
+        };
+        let new_target = basic_blocks.push(BasicBlockData::new(Some(terminator), true));
+        *basic_blocks[bb].terminator_mut().unwind_mut().unwrap() =
+            UnwindAction::Cleanup(new_target);
+    }
+}
+
 // The fields are used only for debugging output in `sccs_info`.
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
 pub(crate) enum RegionCtxt {

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

@@ -11,7 +11,7 @@ use rustc_middle::ty::adjustment::PointerCoercion;
 use rustc_middle::ty::cast::{CastTy, mir_cast_kind};
 use rustc_middle::ty::util::IntTypeExt;
 use rustc_middle::ty::{self, Ty, UpvarArgs};
-use rustc_span::{DUMMY_SP, Span, Spanned};
+use rustc_span::Span;
 use tracing::debug;
 
 use crate::builder::expr::as_place::PlaceBase;
@@ -74,6 +74,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                             NeedsTemporary::No
                         )
                     );
+                    this.record_operands_moved([&value_operand]);
                     block.and(Rvalue::Repeat(value_operand, count))
                 }
             }
@@ -219,6 +220,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     })
                     .collect();
 
+                this.record_operands_moved(&fields);
                 block.and(Rvalue::Aggregate(Box::new(AggregateKind::Array(el_ty)), fields))
             }
             ExprKind::Tuple { ref fields } => {
@@ -240,6 +242,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     })
                     .collect();
 
+                this.record_operands_moved(&fields);
                 block.and(Rvalue::Aggregate(Box::new(AggregateKind::Tuple), fields))
             }
             ExprKind::Closure(ClosureExpr {
@@ -342,6 +345,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         Box::new(AggregateKind::CoroutineClosure(closure_id.to_def_id(), args))
                     }
                 };
+                this.record_operands_moved(&operands);
                 block.and(Rvalue::Aggregate(result, operands))
             }
             ExprKind::Assign { .. } | ExprKind::AssignOp { .. } => {
@@ -424,6 +428,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         NeedsTemporary::No,
                     )
                 );
+                this.record_operands_moved([&operand]);
                 block.and(Rvalue::Use(operand, WithRetag::Yes))
             }
 
@@ -647,7 +652,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/builder/expr/into.rs

@@ -6,6 +6,7 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_hir as hir;
 use rustc_hir::lang_items::LangItem;
+use rustc_index::IndexVec;
 use rustc_middle::mir::*;
 use rustc_middle::span_bug;
 use rustc_middle::thir::*;
@@ -490,8 +491,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     .collect();
 
                 let success = this.cfg.start_new_block();
-
-                this.record_operands_moved(&args);
+                this.record_operands_moved(args.iter().map(|operand| &operand.node));
 
                 debug!("expr_into_dest: fn_span={:?}", fn_span);
 
@@ -632,7 +632,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let variant = adt_def.variant(variant_index);
                 let field_names = variant.fields.indices();
 
-                let fields = match base {
+                let fields: IndexVec<_, _> = match base {
                     AdtExprBase::None => {
                         field_names.filter_map(|n| fields_map.get(&n).cloned()).collect()
                     }
@@ -697,6 +697,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     user_ty,
                     active_field_index,
                 ));
+                this.record_operands_moved(&fields);
                 this.cfg.push_assign(
                     block,
                     source_info,
@@ -845,7 +846,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 debug_assert!(Category::of(&expr.kind) == Some(Category::Place));
 
                 let place = unpack!(block = this.as_place(block, expr_id));
-                let rvalue = Rvalue::Use(this.consume_by_copy_or_move(place), WithRetag::Yes);
+                let operand = this.consume_by_copy_or_move(place);
+                this.record_operands_moved([&operand]);
+                let rvalue = Rvalue::Use(operand, WithRetag::Yes);
                 this.cfg.push_assign(block, source_info, destination, rvalue);
                 block.unit()
             }
@@ -871,6 +874,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     block =
                         this.as_operand(block, scope, value, LocalInfo::Boring, NeedsTemporary::No)
                 );
+                this.record_operands_moved([&value]);
                 let resume = this.cfg.start_new_block();
                 this.cfg.terminate(
                     block,

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

@@ -126,7 +126,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         })
                         .collect();
 
-                    this.record_operands_moved(&args);
+                    this.record_operands_moved(args.iter().map(|operand| &operand.node));
 
                     debug!("expr_into_dest: fn_span={:?}", fn_span);
 

compiler/rustc_mir_build/src/builder/scope.rs

@@ -84,7 +84,7 @@ that contains only loops and breakable blocks. It tracks where a `break`,
 use std::mem;
 
 use interpret::ErrorHandled;
-use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_hir::HirId;
 use rustc_index::{IndexSlice, IndexVec};
 use rustc_middle::middle::region;
@@ -137,8 +137,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>,
@@ -494,7 +492,6 @@ impl<'tcx> Scopes<'tcx> {
             source_scope: vis_scope,
             region_scope,
             drops: vec![],
-            moved_locals: vec![],
             cached_unwind_block: None,
             cached_coroutine_drop_block: None,
         });
@@ -1522,7 +1519,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         self.schedule_drop(span, region_scope, local, DropKind::ForLint);
     }
 
-    /// Indicates that the "local operand" stored in `local` is
+    /// Indicates that the "local operand" stored in `operand` is
     /// *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
@@ -1558,27 +1555,30 @@ 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>>]) {
-        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!",);
-
+    #[instrument(level = "debug", skip(self, operands))]
+    pub(crate) fn record_operands_moved<'o>(
+        &mut self,
+        operands: impl IntoIterator<Item = &'o Operand<'tcx>>,
+    ) where
+        'tcx: 'o,
+    {
         // look for moves of a local variable, like `MOVE(_X)`
-        let locals_moved = operands.iter().flat_map(|operand| match operand.node {
-            Operand::Copy(_) | Operand::Constant(_) | Operand::RuntimeChecks(_) => None,
-            Operand::Move(place) => place.as_local(),
-        });
+        let moved_locals: FxHashSet<Local> = operands
+            .into_iter()
+            .filter_map(|operand| match operand {
+                Operand::Copy(_) | Operand::Constant(_) | Operand::RuntimeChecks(_) => None,
+                Operand::Move(place) => place.as_local(),
+            })
+            .collect();
 
-        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);
-            }
-        }
+        // We only remove drops from the innermost scope. Outer scopes may have branches
+        // or other funny control flow that we cannot know from here.
+        let scope = self.scopes.scopes.last_mut().unwrap();
+        scope.drops.retain(|drop| match drop.kind {
+            DropKind::Storage => true,
+            DropKind::ForLint | DropKind::Value => !moved_locals.contains(&drop.local),
+        });
+        scope.invalidate_cache();
     }
 
     // Other
@@ -1874,14 +1874,6 @@ where
                     dropline_to = Some(coroutine_drops.drop_nodes[idx].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.contains(&local) {
-                    continue;
-                }
-
                 unwind_drops.add_entry_point(block, unwind_to);
                 if let Some(to) = dropline_to
                     && is_async_drop(local)
@@ -1918,14 +1910,6 @@ where
                     unwind_to = unwind_drops.drop_nodes[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.contains(&local) {
-                    continue;
-                }
-
                 cfg.push(
                     block,
                     Statement::new(

src/tools/tidy/src/issues.txt

@@ -1659,7 +1659,6 @@ ui/mir/issue-77002.rs
 ui/mir/issue-77359-simplify-arm-identity.rs
 ui/mir/issue-77911.rs
 ui/mir/issue-78496.rs
-ui/mir/issue-80949.rs
 ui/mir/issue-83499-input-output-iteration-ice.rs
 ui/mir/issue-89485.rs
 ui/mir/issue-91745.rs