From 4ba50103eab680a283c316c49ee653cacd791811 Mon Sep 17 00:00:00 2001 From: lcnr Date: Wed, 14 Aug 2024 11:05:56 +0200 Subject: [PATCH] update comment, rename `fn record_operand_moved` the previous name made it seem as if the operand was moved while it's actually just looking at all `Move` operands. --- compiler/rustc_mir_build/src/build/expr/as_rvalue.rs | 10 +++++----- compiler/rustc_mir_build/src/build/expr/into.rs | 12 +++++++----- compiler/rustc_mir_build/src/build/expr/stmt.rs | 2 +- compiler/rustc_mir_build/src/build/scope.rs | 12 +++++++----- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs index 481915905d94b..6819ebcaac3a0 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -197,7 +197,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ) ); let result_operand = Operand::Move(Place::from(result)); - this.record_operands_moved(slice::from_ref(&result_operand)); + this.record_move_operands(slice::from_ref(&result_operand)); block.and(Rvalue::Use(result_operand)) } ExprKind::Cast { source } => { @@ -366,7 +366,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }) .collect(); - this.record_operands_moved(&fields.raw); + this.record_move_operands(&fields.raw); block.and(Rvalue::Aggregate(Box::new(AggregateKind::Array(el_ty)), fields)) } ExprKind::Tuple { ref fields } => { @@ -388,7 +388,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }) .collect(); - this.record_operands_moved(&fields.raw); + this.record_move_operands(&fields.raw); block.and(Rvalue::Aggregate(Box::new(AggregateKind::Tuple), fields)) } ExprKind::Closure(box ClosureExpr { @@ -491,7 +491,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { Box::new(AggregateKind::CoroutineClosure(closure_id.to_def_id(), args)) } }; - this.record_operands_moved(&operands.raw); + this.record_move_operands(&operands.raw); block.and(Rvalue::Aggregate(result, operands)) } ExprKind::Assign { .. } | ExprKind::AssignOp { .. } => { @@ -747,7 +747,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.diverge_from(block); block = success; } - this.record_operands_moved(&[value_operand]); + this.record_move_operands(&[value_operand]); } block.and(Rvalue::Aggregate(Box::new(AggregateKind::Array(elem_ty)), IndexVec::new())) } diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 58a8611a9b827..2178ced2957ef 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -314,11 +314,13 @@ 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. + // By only recording the moved operands after `diverge_from`, we emit redundant + // drop calls for the arguments of the call. While these should be unnecessary, + // they cause borrowck to shrink the required lifetime of some borrows which is + // necessary to avoid breaking the example in #80949. // FIXME(matthewjasper): Look at this again if Polonius is // stabilized. - this.record_operands_moved(&args); + this.record_move_operands(&args); schedule_drop(this); success.unit() } @@ -419,7 +421,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { user_ty, active_field_index, )); - this.record_operands_moved(&fields.raw); + this.record_move_operands(&fields.raw); this.cfg.push_assign( block, source_info, @@ -602,7 +604,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ) ); let resume = this.cfg.start_new_block(); - this.record_operands_moved(slice::from_ref(&value)); + this.record_move_operands(slice::from_ref(&value)); this.cfg.terminate( block, source_info, diff --git a/compiler/rustc_mir_build/src/build/expr/stmt.rs b/compiler/rustc_mir_build/src/build/expr/stmt.rs index 327ccb159c60d..7529016826799 100644 --- a/compiler/rustc_mir_build/src/build/expr/stmt.rs +++ b/compiler/rustc_mir_build/src/build/expr/stmt.rs @@ -116,7 +116,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { .collect(); let args: Vec<_> = spanned_args.iter().map(|arg| arg.node.clone()).collect(); - this.record_operands_moved(&args); + this.record_move_operands(&args); debug!("expr_into_dest: fn_span={:?}", fn_span); diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index f9e1872791d33..58ad2ff82305c 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -1009,13 +1009,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { region_scope: region::Scope, local: Local, ) { + // We first drop the value and then the underlying storage. self.schedule_drop(span, region_scope, local, DropKind::Storage); self.schedule_drop(span, region_scope, local, DropKind::Value); } - /// Indicates that `place` should be dropped on exit from `region_scope`. + /// Indicates that `local` should be dropped on exit from `region_scope`. /// - /// When called with `DropKind::Storage`, `place` shouldn't be the return + /// When called with `DropKind::Storage`, `local` shouldn't be the return /// place, or a function parameter. pub(crate) fn schedule_drop( &mut self, @@ -1121,7 +1122,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } /// Unschedule a drop. Used for `break`, `return` and `match` expressions, - /// where `record_operands_moved` is not powerful enough. + /// where `record_move_operands` is not powerful enough. /// /// The given local is expected to have a value drop scheduled in the given /// scope and for that drop to be the most recent thing scheduled in that @@ -1146,8 +1147,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // type does not need drop. None if self.local_decls[local].ty.has_opaque_types() => return, _ => bug!( - "found wrong drop, expected value drop of {:?}, found {:?}", + "found wrong drop, expected value drop of {:?} in scope {:?}, found {:?}", local, + region_scope, drop, ), } @@ -1192,7 +1194,7 @@ 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: &[Operand<'tcx>]) { + pub(crate) fn record_move_operands(&mut self, operands: &[Operand<'tcx>]) { let local_scope = self.local_scope(); let scope = self.scopes.scopes.last_mut().unwrap();