From 0dab117adc0306a5634a91fd50e7d26a85aa9b92 Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Wed, 16 Nov 2022 12:55:01 +0100 Subject: [PATCH 01/13] Remove DropAndReplace terminator --- .../src/diagnostics/conflict_errors.rs | 8 +- .../rustc_borrowck/src/diagnostics/mod.rs | 17 +- compiler/rustc_borrowck/src/invalidation.rs | 56 ++++--- compiler/rustc_borrowck/src/lib.rs | 61 +++++--- compiler/rustc_borrowck/src/type_check/mod.rs | 20 --- compiler/rustc_borrowck/src/used_muts.rs | 7 +- compiler/rustc_codegen_cranelift/src/base.rs | 3 +- .../rustc_codegen_cranelift/src/constant.rs | 3 +- compiler/rustc_codegen_ssa/src/mir/analyze.rs | 1 - compiler/rustc_codegen_ssa/src/mir/block.rs | 6 +- .../src/interpret/terminator.rs | 8 +- .../src/transform/check_consts/check.rs | 3 +- .../check_consts/post_drop_elaboration.rs | 3 +- .../src/transform/check_consts/resolver.rs | 23 +-- .../src/transform/validate.rs | 25 +-- compiler/rustc_middle/src/mir/spanview.rs | 1 - compiler/rustc_middle/src/mir/syntax.rs | 40 +---- compiler/rustc_middle/src/mir/terminator.rs | 17 +- compiler/rustc_middle/src/mir/visit.rs | 15 +- .../src/build/custom/parse/instruction.rs | 9 +- .../src/build/expr/as_rvalue.rs | 7 +- .../rustc_mir_build/src/build/expr/stmt.rs | 16 +- compiler/rustc_mir_build/src/build/scope.rs | 43 ++++-- compiler/rustc_mir_build/src/lints.rs | 1 - .../rustc_mir_dataflow/src/elaborate_drops.rs | 16 +- .../src/framework/direction.rs | 4 +- .../src/impls/borrowed_locals.rs | 3 +- compiler/rustc_mir_dataflow/src/impls/mod.rs | 26 +++- .../src/impls/storage_liveness.rs | 2 - .../src/move_paths/builder.rs | 7 +- .../rustc_mir_dataflow/src/value_analysis.rs | 2 +- .../src/abort_unwinding_calls.rs | 2 +- .../src/add_moves_for_packed_drops.rs | 12 +- compiler/rustc_mir_transform/src/add_retag.rs | 2 +- .../rustc_mir_transform/src/check_unsafety.rs | 1 - .../rustc_mir_transform/src/const_prop.rs | 1 - .../src/const_prop_lint.rs | 1 - .../rustc_mir_transform/src/coverage/debug.rs | 1 - .../rustc_mir_transform/src/coverage/graph.rs | 1 - .../rustc_mir_transform/src/coverage/spans.rs | 1 - .../rustc_mir_transform/src/coverage/tests.rs | 2 - compiler/rustc_mir_transform/src/dest_prop.rs | 3 +- .../src/elaborate_drops.rs | 145 +++++------------- compiler/rustc_mir_transform/src/generator.rs | 20 ++- compiler/rustc_mir_transform/src/inline.rs | 10 +- .../src/remove_noop_landing_pads.rs | 1 - .../src/remove_uninit_drops.rs | 17 +- .../src/separate_const_switch.rs | 2 - compiler/rustc_mir_transform/src/shim.rs | 22 ++- compiler/rustc_mir_transform/src/sroa.rs | 4 +- compiler/rustc_monomorphize/src/collector.rs | 3 +- .../clippy_utils/src/qualify_min_const_fn.rs | 4 - 52 files changed, 312 insertions(+), 396 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 50cd13a2ccc8a..aecbade6378ae 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -2364,10 +2364,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ProjectionElem::Deref => match kind { StorageDeadOrDrop::LocalStorageDead | StorageDeadOrDrop::BoxedStorageDead => { - assert!( - place_ty.ty.is_box(), - "Drop of value behind a reference or raw pointer" - ); + // assert!( + // place_ty.ty.is_box(), + // "Drop of value behind a reference or raw pointer" + // ); StorageDeadOrDrop::BoxedStorageDead } StorageDeadOrDrop::Destructor(_) => kind, diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index 63b16aa95a6a5..d1fc66234b3fb 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -939,7 +939,22 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { return OtherUse(use_span); } - for stmt in &self.body[location.block].statements[location.statement_index + 1..] { + // drop and replace might have moved the assignment to the next block + let maybe_additional_statement = if let Some(Terminator { + kind: TerminatorKind::Drop { target: drop_target, is_replace: true, .. }, + .. + }) = self.body[location.block].terminator + { + self.body[drop_target].statements.iter().take(1) + } else { + [].iter().take(0) + }; + + let statements = self.body[location.block].statements[location.statement_index + 1..] + .iter() + .chain(maybe_additional_statement); + + for stmt in statements { if let StatementKind::Assign(box (_, Rvalue::Aggregate(kind, places))) = &stmt.kind { let (&def_id, is_generator) = match kind { box AggregateKind::Closure(def_id, _) => (def_id, false), diff --git a/compiler/rustc_borrowck/src/invalidation.rs b/compiler/rustc_borrowck/src/invalidation.rs index 6fd9290058c36..943d5fd845264 100644 --- a/compiler/rustc_borrowck/src/invalidation.rs +++ b/compiler/rustc_borrowck/src/invalidation.rs @@ -10,8 +10,8 @@ use rustc_middle::ty::TyCtxt; use crate::{ borrow_set::BorrowSet, facts::AllFacts, location::LocationTable, path_utils::*, AccessDepth, - Activation, ArtificialField, BorrowIndex, Deep, LocalMutationIsAllowed, Read, ReadKind, - ReadOrWrite, Reservation, Shallow, Write, WriteKind, + Activation, ArtificialField, BorrowIndex, Deep, FxHashSet, LocalMutationIsAllowed, Read, + ReadKind, ReadOrWrite, Reservation, Shallow, Write, WriteKind, }; pub(super) fn generate_invalidates<'tcx>( @@ -36,6 +36,7 @@ pub(super) fn generate_invalidates<'tcx>( location_table, body: &body, dominators, + to_skip: Default::default(), }; ig.visit_body(body); } @@ -48,6 +49,7 @@ struct InvalidationGenerator<'cx, 'tcx> { body: &'cx Body<'tcx>, dominators: Dominators, borrow_set: &'cx BorrowSet<'tcx>, + to_skip: FxHashSet, } /// Visits the whole MIR and generates `invalidates()` facts. @@ -60,7 +62,9 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { StatementKind::Assign(box (lhs, rhs)) => { self.consume_rvalue(location, rhs); - self.mutate_place(location, *lhs, Shallow(None)); + if !self.to_skip.contains(&location) { + self.mutate_place(location, *lhs, Shallow(None)); + } } StatementKind::FakeRead(box (_, _)) => { // Only relevant for initialized/liveness/safety checks. @@ -109,22 +113,36 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { TerminatorKind::SwitchInt { discr, targets: _ } => { self.consume_operand(location, discr); } - TerminatorKind::Drop { place: drop_place, target: _, unwind: _ } => { - self.access_place( - location, - *drop_place, - (AccessDepth::Drop, Write(WriteKind::StorageDeadOrDrop)), - LocalMutationIsAllowed::Yes, - ); - } - TerminatorKind::DropAndReplace { - place: drop_place, - value: new_value, - target: _, - unwind: _, - } => { - self.mutate_place(location, *drop_place, Deep); - self.consume_operand(location, new_value); + TerminatorKind::Drop { place: drop_place, target, unwind, is_replace } => { + let next_statement = if *is_replace { + self.body + .basic_blocks + .get(*target) + .expect("MIR should be complete at this point") + .statements + .first() + } else { + None + }; + + match next_statement { + Some(Statement { kind: StatementKind::Assign(_), source_info: _ }) => { + // this is a drop from a replace operation, for better diagnostic report + // here possible conflicts and mute the assign statement errors + self.to_skip.insert(Location { block: *target, statement_index: 0 }); + self.to_skip + .insert(Location { block: unwind.unwrap(), statement_index: 0 }); + self.mutate_place(location, *drop_place, AccessDepth::Deep); + } + _ => { + self.access_place( + location, + *drop_place, + (AccessDepth::Drop, Write(WriteKind::StorageDeadOrDrop)), + LocalMutationIsAllowed::Yes, + ); + } + } } TerminatorKind::Call { func, diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 278ffed07477b..ef0a03a546fe1 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -340,6 +340,7 @@ fn do_mir_borrowck<'tcx>( next_region_name: RefCell::new(1), polonius_output: None, errors, + to_skip: Default::default(), }; promoted_mbcx.report_move_errors(move_errors); errors = promoted_mbcx.errors; @@ -371,6 +372,7 @@ fn do_mir_borrowck<'tcx>( next_region_name: RefCell::new(1), polonius_output, errors, + to_skip: Default::default(), }; // Compute and report region errors, if any. @@ -553,6 +555,8 @@ struct MirBorrowckCtxt<'cx, 'tcx> { polonius_output: Option>, errors: error::BorrowckErrors<'tcx>, + + to_skip: FxHashSet, } // Check that: @@ -577,8 +581,9 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx match &stmt.kind { StatementKind::Assign(box (lhs, rhs)) => { self.consume_rvalue(location, (rhs, span), flow_state); - - self.mutate_place(location, (*lhs, span), Shallow(None), flow_state); + if !self.to_skip.contains(&location) { + self.mutate_place(location, (*lhs, span), Shallow(None), flow_state); + } } StatementKind::FakeRead(box (_, place)) => { // Read for match doesn't access any memory and is used to @@ -644,29 +649,43 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx TerminatorKind::SwitchInt { discr, targets: _ } => { self.consume_operand(loc, (discr, span), flow_state); } - TerminatorKind::Drop { place, target: _, unwind: _ } => { + TerminatorKind::Drop { place, target, unwind, is_replace } => { debug!( "visit_terminator_drop \ loc: {:?} term: {:?} place: {:?} span: {:?}", loc, term, place, span ); - self.access_place( - loc, - (*place, span), - (AccessDepth::Drop, Write(WriteKind::StorageDeadOrDrop)), - LocalMutationIsAllowed::Yes, - flow_state, - ); - } - TerminatorKind::DropAndReplace { - place: drop_place, - value: new_value, - target: _, - unwind: _, - } => { - self.mutate_place(loc, (*drop_place, span), Deep, flow_state); - self.consume_operand(loc, (new_value, span), flow_state); + let next_statement = if *is_replace { + self.body() + .basic_blocks + .get(*target) + .expect("MIR should be complete at this point") + .statements + .first() + } else { + None + }; + + match next_statement { + Some(Statement { kind: StatementKind::Assign(_), source_info: _ }) => { + // this is a drop from a replace operation, for better diagnostic report + // here possible conflicts and mute the assign statement errors + self.to_skip.insert(Location { block: *target, statement_index: 0 }); + self.to_skip + .insert(Location { block: unwind.unwrap(), statement_index: 0 }); + self.mutate_place(loc, (*place, span), AccessDepth::Deep, flow_state); + } + _ => { + self.access_place( + loc, + (*place, span), + (AccessDepth::Drop, Write(WriteKind::StorageDeadOrDrop)), + LocalMutationIsAllowed::Yes, + flow_state, + ); + } + } } TerminatorKind::Call { func, @@ -782,7 +801,6 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx | TerminatorKind::Assert { .. } | TerminatorKind::Call { .. } | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } | TerminatorKind::FalseEdge { real_target: _, imaginary_target: _ } | TerminatorKind::FalseUnwind { real_target: _, unwind: _ } | TerminatorKind::Goto { .. } @@ -1592,7 +1610,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { (prefix, place_span.0, place_span.1), mpi, ); - } // Only query longest prefix with a MovePath, not further + } + // Only query longest prefix with a MovePath, not further // ancestors; dataflow recurs on children when parents // move (to support partial (re)inits). // diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index 247607ff29e20..4b02b251e95d1 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -1345,25 +1345,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { | TerminatorKind::InlineAsm { .. } => { // no checks needed for these } - - TerminatorKind::DropAndReplace { place, value, target: _, unwind: _ } => { - let place_ty = place.ty(body, tcx).ty; - let rv_ty = value.ty(body, tcx); - - let locations = term_location.to_locations(); - if let Err(terr) = - self.sub_types(rv_ty, place_ty, locations, ConstraintCategory::Assignment) - { - span_mirbug!( - self, - term, - "bad DropAndReplace ({:?} = {:?}): {:?}", - place_ty, - rv_ty, - terr - ); - } - } TerminatorKind::SwitchInt { discr, .. } => { self.check_operand(discr, term_location); @@ -1637,7 +1618,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { } TerminatorKind::Unreachable => {} TerminatorKind::Drop { target, unwind, .. } - | TerminatorKind::DropAndReplace { target, unwind, .. } | TerminatorKind::Assert { target, cleanup: unwind, .. } => { self.assert_iscleanup(body, block_data, target, is_cleanup); if let Some(unwind) = unwind { diff --git a/compiler/rustc_borrowck/src/used_muts.rs b/compiler/rustc_borrowck/src/used_muts.rs index e297b1230ea0c..86c784da3a02f 100644 --- a/compiler/rustc_borrowck/src/used_muts.rs +++ b/compiler/rustc_borrowck/src/used_muts.rs @@ -71,9 +71,10 @@ impl<'visit, 'cx, 'tcx> Visitor<'tcx> for GatherUsedMutsVisitor<'visit, 'cx, 'tc TerminatorKind::Call { destination, .. } => { self.remove_never_initialized_mut_locals(*destination); } - TerminatorKind::DropAndReplace { place, .. } => { - self.remove_never_initialized_mut_locals(*place); - } + // FIXME: ?? + // TerminatorKind::DropAndReplace { place, .. } => { + // self.remove_never_initialized_mut_locals(*place); + // } _ => {} } diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index 89d955e8bf2e1..7a583684f832c 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -474,11 +474,10 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) { TerminatorKind::Yield { .. } | TerminatorKind::FalseEdge { .. } | TerminatorKind::FalseUnwind { .. } - | TerminatorKind::DropAndReplace { .. } | TerminatorKind::GeneratorDrop => { bug!("shouldn't exist at codegen {:?}", bb_data.terminator()); } - TerminatorKind::Drop { place, target, unwind: _ } => { + TerminatorKind::Drop { place, target, unwind: _, is_replace: _ } => { let drop_place = codegen_place(fx, *place); crate::abi::codegen_drop(fx, source_info, drop_place); diff --git a/compiler/rustc_codegen_cranelift/src/constant.rs b/compiler/rustc_codegen_cranelift/src/constant.rs index dee6fb5b5130d..530c00e17167a 100644 --- a/compiler/rustc_codegen_cranelift/src/constant.rs +++ b/compiler/rustc_codegen_cranelift/src/constant.rs @@ -542,8 +542,7 @@ pub(crate) fn mir_operand_get_const_val<'tcx>( | TerminatorKind::Unreachable | TerminatorKind::Drop { .. } | TerminatorKind::Assert { .. } => {} - TerminatorKind::DropAndReplace { .. } - | TerminatorKind::Yield { .. } + TerminatorKind::Yield { .. } | TerminatorKind::GeneratorDrop | TerminatorKind::FalseEdge { .. } | TerminatorKind::FalseUnwind { .. } => unreachable!(), diff --git a/compiler/rustc_codegen_ssa/src/mir/analyze.rs b/compiler/rustc_codegen_ssa/src/mir/analyze.rs index c7617d2e464fa..a9e31be998144 100644 --- a/compiler/rustc_codegen_ssa/src/mir/analyze.rs +++ b/compiler/rustc_codegen_ssa/src/mir/analyze.rs @@ -281,7 +281,6 @@ pub fn cleanup_kinds(mir: &mir::Body<'_>) -> IndexVec { if let Some(unwind) = unwind { debug!( diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 978aff511bfa7..7d3e8768bcc0f 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -1305,7 +1305,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { MergingSucc::False } - mir::TerminatorKind::Drop { place, target, unwind } => { + mir::TerminatorKind::Drop { place, target, unwind, is_replace: _ } => { self.codegen_drop_terminator(helper, bx, place, target, unwind, mergeable_succ()) } @@ -1322,10 +1322,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { mergeable_succ(), ), - mir::TerminatorKind::DropAndReplace { .. } => { - bug!("undesugared DropAndReplace in codegen: {:?}", terminator); - } - mir::TerminatorKind::Call { ref func, ref args, diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index 550c7a44c4199..ba84d06ddaf86 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -118,7 +118,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } - Drop { place, target, unwind } => { + Drop { place, target, unwind, is_replace: _ } => { let frame = self.frame(); let ty = place.ty(&frame.body.local_decls, *self.tcx).ty; let ty = self.subst_from_frame_and_normalize_erasing_regions(frame, ty)?; @@ -164,11 +164,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Unreachable => throw_ub!(Unreachable), // These should never occur for MIR we actually run. - DropAndReplace { .. } - | FalseEdge { .. } - | FalseUnwind { .. } - | Yield { .. } - | GeneratorDrop => span_bug!( + FalseEdge { .. } | FalseUnwind { .. } | Yield { .. } | GeneratorDrop => span_bug!( terminator.source_info.span, "{:#?} should have been eliminated by MIR pass", terminator.kind diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs index 54213d55a2da7..a2624c7408cc4 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/check.rs @@ -961,8 +961,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { // Forbid all `Drop` terminators unless the place being dropped is a local with no // projections that cannot be `NeedsNonConstDrop`. - TerminatorKind::Drop { place: dropped_place, .. } - | TerminatorKind::DropAndReplace { place: dropped_place, .. } => { + TerminatorKind::Drop { place: dropped_place, .. } => { // If we are checking live drops after drop-elaboration, don't emit duplicate // errors here. if super::post_drop_elaboration::checking_enabled(self.ccx) { diff --git a/compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs b/compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs index d4570c5988914..722b3eb7aac1f 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs @@ -80,8 +80,7 @@ impl<'tcx> Visitor<'tcx> for CheckLiveDrops<'_, 'tcx> { trace!("visit_terminator: terminator={:?} location={:?}", terminator, location); match &terminator.kind { - mir::TerminatorKind::Drop { place: dropped_place, .. } - | mir::TerminatorKind::DropAndReplace { place: dropped_place, .. } => { + mir::TerminatorKind::Drop { place: dropped_place, .. } => { let dropped_ty = dropped_place.ty(self.body, self.tcx).ty; if !NeedsNonConstDrop::in_any_value_of_ty(self.ccx, dropped_ty) { // Instead of throwing a bug, we just return here. This is because we have to diff --git a/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs b/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs index 805e6096b35c8..beb224d150bdf 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs @@ -222,17 +222,18 @@ where // The effect of assignment to the return place in `TerminatorKind::Call` is not applied // here; that occurs in `apply_call_return_effect`. - if let mir::TerminatorKind::DropAndReplace { value, place, .. } = &terminator.kind { - let qualif = qualifs::in_operand::( - self.ccx, - &mut |l| self.state.qualif.contains(l), - value, - ); - - if !place.is_indirect() { - self.assign_qualif_direct(place, qualif); - } - } + // FIXME ?? + // if let mir::TerminatorKind::DropAndReplace { value, place, .. } = &terminator.kind { + // let qualif = qualifs::in_operand::( + // self.ccx, + // &mut |l| self.state.qualif.contains(l), + // value, + // ); + + // if !place.is_indirect() { + // self.assign_qualif_direct(place, qualif); + // } + // } // We ignore borrow on drop because custom drop impls are not allowed in consts. // FIXME: Reconsider if accounting for borrows in drops is necessary for const drop. diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index 94e1b95a0eb3c..87cea20ca3d91 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -736,18 +736,19 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { self.check_edge(location, *unwind, EdgeKind::Unwind); } } - TerminatorKind::DropAndReplace { target, unwind, .. } => { - if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { - self.fail( - location, - "`DropAndReplace` should have been removed during drop elaboration", - ); - } - self.check_edge(location, *target, EdgeKind::Normal); - if let Some(unwind) = unwind { - self.check_edge(location, *unwind, EdgeKind::Unwind); - } - } + // FIXME ?? + // TerminatorKind::DropAndReplace { target, unwind, .. } => { + // if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { + // self.fail( + // location, + // "`DropAndReplace` should have been removed during drop elaboration", + // ); + // } + // self.check_edge(location, *target, EdgeKind::Normal); + // if let Some(unwind) = unwind { + // self.check_edge(location, *unwind, EdgeKind::Unwind); + // } + // } TerminatorKind::Call { func, args, destination, target, cleanup, .. } => { let func_ty = func.ty(&self.body.local_decls, self.tcx); match func_ty.kind() { diff --git a/compiler/rustc_middle/src/mir/spanview.rs b/compiler/rustc_middle/src/mir/spanview.rs index 887ee57157540..613dc3bcfd508 100644 --- a/compiler/rustc_middle/src/mir/spanview.rs +++ b/compiler/rustc_middle/src/mir/spanview.rs @@ -264,7 +264,6 @@ pub fn terminator_kind_name(term: &Terminator<'_>) -> &'static str { Return => "Return", Unreachable => "Unreachable", Drop { .. } => "Drop", - DropAndReplace { .. } => "DropAndReplace", Call { .. } => "Call", Assert { .. } => "Assert", Yield { .. } => "Yield", diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 6b4489026d3d3..eb704a439c24a 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -133,7 +133,6 @@ pub enum AnalysisPhase { pub enum RuntimePhase { /// In addition to the semantic changes, beginning with this phase, the following variants are /// disallowed: - /// * [`TerminatorKind::DropAndReplace`] /// * [`TerminatorKind::Yield`] /// * [`TerminatorKind::GeneratorDrop`] /// * [`Rvalue::Aggregate`] for any `AggregateKind` except `Array` @@ -578,44 +577,7 @@ pub enum TerminatorKind<'tcx> { /// > The drop glue is executed if, among all statements executed within this `Body`, an assignment to /// > the place or one of its "parents" occurred more recently than a move out of it. This does not /// > consider indirect assignments. - Drop { place: Place<'tcx>, target: BasicBlock, unwind: Option }, - - /// Drops the place and assigns a new value to it. - /// - /// This first performs the exact same operation as the pre drop-elaboration `Drop` terminator; - /// it then additionally assigns the `value` to the `place` as if by an assignment statement. - /// This assignment occurs both in the unwind and the regular code paths. The semantics are best - /// explained by the elaboration: - /// - /// ```ignore (MIR) - /// BB0 { - /// DropAndReplace(P <- V, goto BB1, unwind BB2) - /// } - /// ``` - /// - /// becomes - /// - /// ```ignore (MIR) - /// BB0 { - /// Drop(P, goto BB1, unwind BB2) - /// } - /// BB1 { - /// // P is now uninitialized - /// P <- V - /// } - /// BB2 { - /// // P is now uninitialized -- its dtor panicked - /// P <- V - /// } - /// ``` - /// - /// Disallowed after drop elaboration. - DropAndReplace { - place: Place<'tcx>, - value: Operand<'tcx>, - target: BasicBlock, - unwind: Option, - }, + Drop { place: Place<'tcx>, target: BasicBlock, unwind: Option, is_replace: bool }, /// Roughly speaking, evaluates the `func` operand and the arguments, and starts execution of /// the referred to function. The operand types must match the argument types of the function. diff --git a/compiler/rustc_middle/src/mir/terminator.rs b/compiler/rustc_middle/src/mir/terminator.rs index 438f36373ca91..a157ddf446232 100644 --- a/compiler/rustc_middle/src/mir/terminator.rs +++ b/compiler/rustc_middle/src/mir/terminator.rs @@ -148,7 +148,6 @@ impl<'tcx> TerminatorKind<'tcx> { | Call { target: None, cleanup: Some(t), .. } | Call { target: Some(t), cleanup: None, .. } | Yield { resume: t, drop: None, .. } - | DropAndReplace { target: t, unwind: None, .. } | Drop { target: t, unwind: None, .. } | Assert { target: t, cleanup: None, .. } | FalseUnwind { real_target: t, unwind: None } @@ -158,7 +157,6 @@ impl<'tcx> TerminatorKind<'tcx> { } Call { target: Some(t), cleanup: Some(ref u), .. } | Yield { resume: t, drop: Some(ref u), .. } - | DropAndReplace { target: t, unwind: Some(ref u), .. } | Drop { target: t, unwind: Some(ref u), .. } | Assert { target: t, cleanup: Some(ref u), .. } | FalseUnwind { real_target: t, unwind: Some(ref u) } @@ -188,7 +186,6 @@ impl<'tcx> TerminatorKind<'tcx> { | Call { target: None, cleanup: Some(ref mut t), .. } | Call { target: Some(ref mut t), cleanup: None, .. } | Yield { resume: ref mut t, drop: None, .. } - | DropAndReplace { target: ref mut t, unwind: None, .. } | Drop { target: ref mut t, unwind: None, .. } | Assert { target: ref mut t, cleanup: None, .. } | FalseUnwind { real_target: ref mut t, unwind: None } @@ -198,7 +195,6 @@ impl<'tcx> TerminatorKind<'tcx> { } Call { target: Some(ref mut t), cleanup: Some(ref mut u), .. } | Yield { resume: ref mut t, drop: Some(ref mut u), .. } - | DropAndReplace { target: ref mut t, unwind: Some(ref mut u), .. } | Drop { target: ref mut t, unwind: Some(ref mut u), .. } | Assert { target: ref mut t, cleanup: Some(ref mut u), .. } | FalseUnwind { real_target: ref mut t, unwind: Some(ref mut u) } @@ -225,7 +221,6 @@ impl<'tcx> TerminatorKind<'tcx> { | TerminatorKind::FalseEdge { .. } => None, TerminatorKind::Call { cleanup: ref unwind, .. } | TerminatorKind::Assert { cleanup: ref unwind, .. } - | TerminatorKind::DropAndReplace { ref unwind, .. } | TerminatorKind::Drop { ref unwind, .. } | TerminatorKind::FalseUnwind { ref unwind, .. } | TerminatorKind::InlineAsm { cleanup: ref unwind, .. } => Some(unwind), @@ -245,7 +240,6 @@ impl<'tcx> TerminatorKind<'tcx> { | TerminatorKind::FalseEdge { .. } => None, TerminatorKind::Call { cleanup: ref mut unwind, .. } | TerminatorKind::Assert { cleanup: ref mut unwind, .. } - | TerminatorKind::DropAndReplace { ref mut unwind, .. } | TerminatorKind::Drop { ref mut unwind, .. } | TerminatorKind::FalseUnwind { ref mut unwind, .. } | TerminatorKind::InlineAsm { cleanup: ref mut unwind, .. } => Some(unwind), @@ -309,9 +303,6 @@ impl<'tcx> TerminatorKind<'tcx> { Yield { value, resume_arg, .. } => write!(fmt, "{:?} = yield({:?})", resume_arg, value), Unreachable => write!(fmt, "unreachable"), Drop { place, .. } => write!(fmt, "drop({:?})", place), - DropAndReplace { place, value, .. } => { - write!(fmt, "replace({:?} <- {:?})", place, value) - } Call { func, args, destination, .. } => { write!(fmt, "{:?} = ", destination)?; write!(fmt, "{:?}(", func)?; @@ -403,12 +394,8 @@ impl<'tcx> TerminatorKind<'tcx> { Call { target: None, cleanup: None, .. } => vec![], Yield { drop: Some(_), .. } => vec!["resume".into(), "drop".into()], Yield { drop: None, .. } => vec!["resume".into()], - DropAndReplace { unwind: None, .. } | Drop { unwind: None, .. } => { - vec!["return".into()] - } - DropAndReplace { unwind: Some(_), .. } | Drop { unwind: Some(_), .. } => { - vec!["return".into(), "unwind".into()] - } + Drop { unwind: None, .. } => vec!["return".into()], + Drop { unwind: Some(_), .. } => vec!["return".into(), "unwind".into()], Assert { cleanup: None, .. } => vec!["".into()], Assert { .. } => vec!["success".into(), "unwind".into()], FalseEdge { .. } => vec!["real".into(), "imaginary".into()], diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 1a264d2d5af9a..cf1d1da53abdf 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -486,6 +486,7 @@ macro_rules! make_mir_visitor { place, target: _, unwind: _, + is_replace: _, } => { self.visit_place( place, @@ -494,20 +495,6 @@ macro_rules! make_mir_visitor { ); } - TerminatorKind::DropAndReplace { - place, - value, - target: _, - unwind: _, - } => { - self.visit_place( - place, - PlaceContext::MutatingUse(MutatingUseContext::Drop), - location - ); - self.visit_operand(value, location); - } - TerminatorKind::Call { func, args, diff --git a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs index dca4906c07de5..1c13714d090d6 100644 --- a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs +++ b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs @@ -47,14 +47,7 @@ impl<'tcx, 'body> ParseCtxt<'tcx, 'body> { place: self.parse_place(args[0])?, target: self.parse_block(args[1])?, unwind: None, - }) - }, - @call("mir_drop_and_replace", args) => { - Ok(TerminatorKind::DropAndReplace { - place: self.parse_place(args[0])?, - value: self.parse_operand(args[1])?, - target: self.parse_block(args[2])?, - unwind: None, + is_replace: false }) }, @call("mir_call", args) => { 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 c7b3eb44dc5fb..d02ccf03b82fc 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -624,7 +624,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.cfg.terminate( block, outer_source_info, - TerminatorKind::Drop { place: to_drop, target: success, unwind: None }, + TerminatorKind::Drop { + place: to_drop, + target: success, + unwind: None, + is_replace: false, + }, ); this.diverge_from(block); block = success; diff --git a/compiler/rustc_mir_build/src/build/expr/stmt.rs b/compiler/rustc_mir_build/src/build/expr/stmt.rs index e9f327978aab1..243911de4fb98 100644 --- a/compiler/rustc_mir_build/src/build/expr/stmt.rs +++ b/compiler/rustc_mir_build/src/build/expr/stmt.rs @@ -39,16 +39,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Generate better code for things that don't need to be // dropped. - if lhs.ty.needs_drop(this.tcx, this.param_env) { - let rhs = unpack!(block = this.as_local_operand(block, rhs)); - let lhs = unpack!(block = this.as_place(block, lhs)); - unpack!(block = this.build_drop_and_replace(block, lhs_span, lhs, rhs)); - } else { - let rhs = unpack!(block = this.as_local_rvalue(block, rhs)); - let lhs = unpack!(block = this.as_place(block, lhs)); - this.cfg.push_assign(block, source_info, lhs, rhs); - } + let needs_drop = lhs.ty.needs_drop(this.tcx, this.param_env); + let rhs = unpack!(block = this.as_local_rvalue(block, rhs)); + let lhs = unpack!(block = this.as_place(block, lhs)); + if needs_drop { + unpack!(block = this.build_drop_and_replace(block, lhs_span, lhs, rhs.clone())); + } + this.cfg.push_assign(block, source_info, lhs, rhs); this.block_context.pop(); block.unit() } diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index c92634a609de0..a32cbc0271107 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -371,6 +371,7 @@ impl DropTree { // The caller will handle this if needed. unwind: None, place: drop_data.0.local.into(), + is_replace: false, }; cfg.terminate(block, drop_data.0.source_info, terminator); } @@ -1072,7 +1073,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { TerminatorKind::Assert { .. } | TerminatorKind::Call { .. } | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } | TerminatorKind::FalseUnwind { .. } | TerminatorKind::InlineAsm { .. } ), @@ -1118,21 +1118,35 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } /// Utility function for *non*-scope code to build their own drops + /// Force a drop at this point in the MIR by creating a new block. + /// Should only be used for DropAndReplace. + /// Note that this does *not* insert the assignement in the MIR, + /// that should be added in the newly returned block by the caller. pub(crate) fn build_drop_and_replace( &mut self, block: BasicBlock, span: Span, place: Place<'tcx>, - value: Operand<'tcx>, + value: Rvalue<'tcx>, ) -> BlockAnd<()> { let source_info = self.source_info(span); let next_target = self.cfg.start_new_block(); + let assign = self.cfg.start_new_cleanup_block(); + self.cfg.push_assign(assign, source_info, place, value.clone()); + self.cfg.terminate(assign, source_info, TerminatorKind::Goto { target: block }); + self.cfg.terminate( block, source_info, - TerminatorKind::DropAndReplace { place, value, target: next_target, unwind: None }, + TerminatorKind::Drop { + place, + target: next_target, + unwind: Some(assign), + is_replace: true, + }, ); + self.diverge_from(block); next_target.unit() @@ -1234,7 +1248,12 @@ fn build_scope_drops<'tcx>( cfg.terminate( block, source_info, - TerminatorKind::Drop { place: local.into(), target: next, unwind: None }, + TerminatorKind::Drop { + place: local.into(), + target: next, + unwind: None, + is_replace: false, + }, ); block = next; } @@ -1413,14 +1432,18 @@ impl<'tcx> DropTreeBuilder<'tcx> for Unwind { fn add_entry(cfg: &mut CFG<'tcx>, from: BasicBlock, to: BasicBlock) { let term = &mut cfg.block_data_mut(from).terminator_mut(); match &mut term.kind { - TerminatorKind::Drop { unwind, .. } - | TerminatorKind::DropAndReplace { unwind, .. } - | TerminatorKind::FalseUnwind { unwind, .. } + TerminatorKind::Drop { unwind, .. } => { + if let Some(unwind) = unwind.clone() { + cfg.block_data_mut(unwind).terminator_mut().kind = + TerminatorKind::Goto { target: to }; + } else { + *unwind = Some(to); + } + } + TerminatorKind::FalseUnwind { unwind, .. } | TerminatorKind::Call { cleanup: unwind, .. } | TerminatorKind::Assert { cleanup: unwind, .. } - | TerminatorKind::InlineAsm { cleanup: unwind, .. } => { - *unwind = Some(to); - } + | TerminatorKind::InlineAsm { cleanup: unwind, .. } => *unwind = Some(to), TerminatorKind::Goto { .. } | TerminatorKind::SwitchInt { .. } | TerminatorKind::Resume diff --git a/compiler/rustc_mir_build/src/lints.rs b/compiler/rustc_mir_build/src/lints.rs index 8529c64cd5cca..9caab58963418 100644 --- a/compiler/rustc_mir_build/src/lints.rs +++ b/compiler/rustc_mir_build/src/lints.rs @@ -128,7 +128,6 @@ impl<'mir, 'tcx> TriColorVisitor> for Search<'mir, 'tcx> { TerminatorKind::Assert { .. } | TerminatorKind::Call { .. } | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } | TerminatorKind::FalseEdge { .. } | TerminatorKind::FalseUnwind { .. } | TerminatorKind::Goto { .. } diff --git a/compiler/rustc_mir_dataflow/src/elaborate_drops.rs b/compiler/rustc_mir_dataflow/src/elaborate_drops.rs index 7836ae2e7b76f..c844fa5541159 100644 --- a/compiler/rustc_mir_dataflow/src/elaborate_drops.rs +++ b/compiler/rustc_mir_dataflow/src/elaborate_drops.rs @@ -164,6 +164,7 @@ where path: D::Path, succ: BasicBlock, unwind: Unwind, + is_replace: bool, } /// "Elaborates" a drop of `place`/`path` and patches `bb`'s terminator to execute it. @@ -182,11 +183,12 @@ pub fn elaborate_drop<'b, 'tcx, D>( succ: BasicBlock, unwind: Unwind, bb: BasicBlock, + is_replace: bool, ) where D: DropElaborator<'b, 'tcx>, 'tcx: 'b, { - DropCtxt { elaborator, source_info, place, path, succ, unwind }.elaborate_drop(bb) + DropCtxt { elaborator, source_info, place, path, succ, unwind, is_replace }.elaborate_drop(bb) } impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D> @@ -237,6 +239,7 @@ where place: self.place, target: self.succ, unwind: self.unwind.into_option(), + is_replace: self.is_replace, }, ); } @@ -298,6 +301,7 @@ where place, succ, unwind, + is_replace: self.is_replace, } .elaborated_drop_block() } else { @@ -312,6 +316,7 @@ where // Using `self.path` here to condition the drop on // our own drop flag. path: self.path, + is_replace: self.is_replace, } .complete_drop(succ, unwind) } @@ -730,6 +735,7 @@ where place: tcx.mk_place_deref(ptr), target: loop_block, unwind: unwind.into_option(), + is_replace: self.is_replace, }, ); @@ -991,8 +997,12 @@ where } fn drop_block(&mut self, target: BasicBlock, unwind: Unwind) -> BasicBlock { - let block = - TerminatorKind::Drop { place: self.place, target, unwind: unwind.into_option() }; + let block = TerminatorKind::Drop { + place: self.place, + target, + unwind: unwind.into_option(), + is_replace: self.is_replace, + }; self.new_block(unwind, block) } diff --git a/compiler/rustc_mir_dataflow/src/framework/direction.rs b/compiler/rustc_mir_dataflow/src/framework/direction.rs index 077a21fc8afc0..9816e071bf41c 100644 --- a/compiler/rustc_mir_dataflow/src/framework/direction.rs +++ b/compiler/rustc_mir_dataflow/src/framework/direction.rs @@ -282,7 +282,6 @@ impl Direction for Backward { mir::TerminatorKind::Call { cleanup: Some(unwind), .. } | mir::TerminatorKind::Assert { cleanup: Some(unwind), .. } | mir::TerminatorKind::Drop { unwind: Some(unwind), .. } - | mir::TerminatorKind::DropAndReplace { unwind: Some(unwind), .. } | mir::TerminatorKind::FalseUnwind { unwind: Some(unwind), .. } | mir::TerminatorKind::InlineAsm { cleanup: Some(unwind), .. } if unwind == bb => @@ -498,8 +497,7 @@ impl Direction for Forward { Goto { target } => propagate(target, exit_state), Assert { target, cleanup: unwind, expected: _, msg: _, cond: _ } - | Drop { target, unwind, place: _ } - | DropAndReplace { target, unwind, value: _, place: _ } + | Drop { target, unwind, .. } | FalseUnwind { real_target: target, unwind } => { if let Some(unwind) = unwind { if dead_unwinds.map_or(true, |dead| !dead.contains(bb)) { diff --git a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs index 0f8e86d1d6679..6dd69ebe55793 100644 --- a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs +++ b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs @@ -111,8 +111,7 @@ where self.super_terminator(terminator, location); match terminator.kind { - mir::TerminatorKind::Drop { place: dropped_place, .. } - | mir::TerminatorKind::DropAndReplace { place: dropped_place, .. } => { + mir::TerminatorKind::Drop { place: dropped_place, .. } => { // Drop terminators may call custom drop glue (`Drop::drop`), which takes `&mut // self` as a parameter. In the general case, a drop impl could launder that // reference into the surrounding environment through a raw pointer, thus creating diff --git a/compiler/rustc_mir_dataflow/src/impls/mod.rs b/compiler/rustc_mir_dataflow/src/impls/mod.rs index bc31ec42b8b6e..85b177f093fb8 100644 --- a/compiler/rustc_mir_dataflow/src/impls/mod.rs +++ b/compiler/rustc_mir_dataflow/src/impls/mod.rs @@ -338,6 +338,14 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> { Self::update_bits(trans, path, s) }); + if let mir::TerminatorKind::Drop { place, .. } = terminator.kind { + if let LookupResult::Exact(mpi) = self.move_data().rev_lookup.find(place.as_ref()) { + on_all_children_bits(self.tcx, self.body, self.move_data(), mpi, |mpi| { + Self::update_bits(trans, mpi, DropFlagState::Absent) + }) + } + } + if !self.tcx.sess.opts.unstable_opts.precise_enum_drop_elaboration { return; } @@ -458,9 +466,16 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { fn terminator_effect( &self, trans: &mut impl GenKill, - _terminator: &mir::Terminator<'tcx>, + terminator: &mir::Terminator<'tcx>, location: Location, ) { + if let mir::TerminatorKind::Drop { place, .. } = terminator.kind { + if let LookupResult::Exact(mpi) = self.move_data().rev_lookup.find(place.as_ref()) { + on_all_children_bits(self.tcx, self.body, self.move_data(), mpi, |mpi| { + Self::update_bits(trans, mpi, DropFlagState::Absent) + }) + } + } drop_flag_effects_for_location(self.tcx, self.body, self.mdpe, location, |path, s| { Self::update_bits(trans, path, s) }); @@ -575,9 +590,16 @@ impl<'tcx> GenKillAnalysis<'tcx> for DefinitelyInitializedPlaces<'_, 'tcx> { fn terminator_effect( &self, trans: &mut impl GenKill, - _terminator: &mir::Terminator<'tcx>, + terminator: &mir::Terminator<'tcx>, location: Location, ) { + if let mir::TerminatorKind::Drop { place, .. } = terminator.kind { + if let LookupResult::Exact(mpi) = self.move_data().rev_lookup.find(place.as_ref()) { + on_all_children_bits(self.tcx, self.body, self.move_data(), mpi, |mpi| { + Self::update_bits(trans, mpi, DropFlagState::Absent) + }) + } + } drop_flag_effects_for_location(self.tcx, self.body, self.mdpe, location, |path, s| { Self::update_bits(trans, path, s) }) diff --git a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs index 8d379b90a86db..9c478bef8ecb1 100644 --- a/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs @@ -201,7 +201,6 @@ impl<'mir, 'tcx> crate::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, 'tc TerminatorKind::Abort | TerminatorKind::Assert { .. } | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } | TerminatorKind::FalseEdge { .. } | TerminatorKind::FalseUnwind { .. } | TerminatorKind::GeneratorDrop @@ -239,7 +238,6 @@ impl<'mir, 'tcx> crate::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, 'tc | TerminatorKind::Abort | TerminatorKind::Assert { .. } | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } | TerminatorKind::FalseEdge { .. } | TerminatorKind::FalseUnwind { .. } | TerminatorKind::GeneratorDrop diff --git a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs index f46fd118bde5d..687c22f23f643 100644 --- a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs +++ b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs @@ -391,13 +391,8 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { self.gather_init(place.as_ref(), InitKind::Deep); } - TerminatorKind::Drop { place, target: _, unwind: _ } => { - self.gather_move(place); - } - TerminatorKind::DropAndReplace { place, ref value, .. } => { + TerminatorKind::Drop { place, .. } => { self.create_move_path(place); - self.gather_operand(value); - self.gather_init(place.as_ref(), InitKind::Deep); } TerminatorKind::Call { ref func, diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index fe5ee4011ab84..8572e01d1575e 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -230,7 +230,7 @@ pub trait ValueAnalysis<'tcx> { TerminatorKind::Drop { .. } => { // We don't track dropped places. } - TerminatorKind::DropAndReplace { .. } | TerminatorKind::Yield { .. } => { + TerminatorKind::Yield { .. } => { // They would have an effect, but are not allowed in this phase. bug!("encountered disallowed terminator"); } diff --git a/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs b/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs index d8f85d2e37982..3597eb48a1309 100644 --- a/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs +++ b/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs @@ -74,7 +74,7 @@ impl<'tcx> MirPass<'tcx> for AbortUnwindingCalls { }; layout::fn_can_unwind(tcx, fn_def_id, sig.abi()) } - TerminatorKind::Drop { .. } | TerminatorKind::DropAndReplace { .. } => { + TerminatorKind::Drop { .. } => { tcx.sess.opts.unstable_opts.panic_in_drop == PanicStrategy::Unwind && layout::fn_can_unwind(tcx, None, Abi::Rust) } diff --git a/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs b/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs index 9b2260f68251a..69198719ee2fb 100644 --- a/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs +++ b/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs @@ -64,9 +64,6 @@ fn add_moves_for_packed_drops_patch<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) { add_move_for_packed_drop(tcx, body, &mut patch, terminator, loc, data.is_cleanup); } - TerminatorKind::DropAndReplace { .. } => { - span_bug!(terminator.source_info.span, "replace in AddMovesForPackedDrops"); - } _ => {} } } @@ -83,7 +80,7 @@ fn add_move_for_packed_drop<'tcx>( is_cleanup: bool, ) { debug!("add_move_for_packed_drop({:?} @ {:?})", terminator, loc); - let TerminatorKind::Drop { ref place, target, unwind } = terminator.kind else { + let TerminatorKind::Drop { ref place, target, unwind, is_replace: _ } = terminator.kind else { unreachable!(); }; @@ -101,6 +98,11 @@ fn add_move_for_packed_drop<'tcx>( patch.add_assign(loc, Place::from(temp), Rvalue::Use(Operand::Move(*place))); patch.patch_terminator( loc.block, - TerminatorKind::Drop { place: Place::from(temp), target: storage_dead_block, unwind }, + TerminatorKind::Drop { + place: Place::from(temp), + target: storage_dead_block, + unwind, + is_replace: false, + }, ); } diff --git a/compiler/rustc_mir_transform/src/add_retag.rs b/compiler/rustc_mir_transform/src/add_retag.rs index 3d22035f0785e..387c4f11adfea 100644 --- a/compiler/rustc_mir_transform/src/add_retag.rs +++ b/compiler/rustc_mir_transform/src/add_retag.rs @@ -100,7 +100,7 @@ impl<'tcx> MirPass<'tcx> for AddRetag { } // `Drop` is also a call, but it doesn't return anything so we are good. - TerminatorKind::Drop { .. } | TerminatorKind::DropAndReplace { .. } => None, + TerminatorKind::Drop { .. } => None, // Not a block ending in a Call -> ignore. _ => None, } diff --git a/compiler/rustc_mir_transform/src/check_unsafety.rs b/compiler/rustc_mir_transform/src/check_unsafety.rs index adf6ae4c7270f..60f43397576b4 100644 --- a/compiler/rustc_mir_transform/src/check_unsafety.rs +++ b/compiler/rustc_mir_transform/src/check_unsafety.rs @@ -55,7 +55,6 @@ impl<'tcx> Visitor<'tcx> for UnsafetyChecker<'_, 'tcx> { | TerminatorKind::Drop { .. } | TerminatorKind::Yield { .. } | TerminatorKind::Assert { .. } - | TerminatorKind::DropAndReplace { .. } | TerminatorKind::GeneratorDrop | TerminatorKind::Resume | TerminatorKind::Abort diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index e384cfe165990..1c00a43fcf266 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -1124,7 +1124,6 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> { | TerminatorKind::Return | TerminatorKind::Unreachable | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } | TerminatorKind::Yield { .. } | TerminatorKind::GeneratorDrop | TerminatorKind::FalseEdge { .. } diff --git a/compiler/rustc_mir_transform/src/const_prop_lint.rs b/compiler/rustc_mir_transform/src/const_prop_lint.rs index 0ab67228f3f40..c30f8e60d1fc7 100644 --- a/compiler/rustc_mir_transform/src/const_prop_lint.rs +++ b/compiler/rustc_mir_transform/src/const_prop_lint.rs @@ -684,7 +684,6 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> { | TerminatorKind::Return | TerminatorKind::Unreachable | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } | TerminatorKind::Yield { .. } | TerminatorKind::GeneratorDrop | TerminatorKind::FalseEdge { .. } diff --git a/compiler/rustc_mir_transform/src/coverage/debug.rs b/compiler/rustc_mir_transform/src/coverage/debug.rs index d6a298fade42e..d384d1be9bb4f 100644 --- a/compiler/rustc_mir_transform/src/coverage/debug.rs +++ b/compiler/rustc_mir_transform/src/coverage/debug.rs @@ -819,7 +819,6 @@ pub(super) fn term_type(kind: &TerminatorKind<'_>) -> &'static str { TerminatorKind::Return => "Return", TerminatorKind::Unreachable => "Unreachable", TerminatorKind::Drop { .. } => "Drop", - TerminatorKind::DropAndReplace { .. } => "DropAndReplace", TerminatorKind::Call { .. } => "Call", TerminatorKind::Assert { .. } => "Assert", TerminatorKind::Yield { .. } => "Yield", diff --git a/compiler/rustc_mir_transform/src/coverage/graph.rs b/compiler/rustc_mir_transform/src/coverage/graph.rs index 78d28f1ebab7c..9b08cfa04f50b 100644 --- a/compiler/rustc_mir_transform/src/coverage/graph.rs +++ b/compiler/rustc_mir_transform/src/coverage/graph.rs @@ -156,7 +156,6 @@ impl CoverageGraph { | TerminatorKind::Resume | TerminatorKind::Unreachable | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } | TerminatorKind::Call { .. } | TerminatorKind::GeneratorDrop | TerminatorKind::Assert { .. } diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 9f842c929dc24..f49ae1f8b4593 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -848,7 +848,6 @@ pub(super) fn filtered_terminator_span(terminator: &Terminator<'_>) -> Option MockBlocks<'tcx> { TerminatorKind::Assert { ref mut target, .. } | TerminatorKind::Call { target: Some(ref mut target), .. } | TerminatorKind::Drop { ref mut target, .. } - | TerminatorKind::DropAndReplace { ref mut target, .. } | TerminatorKind::FalseEdge { real_target: ref mut target, .. } | TerminatorKind::FalseUnwind { real_target: ref mut target, .. } | TerminatorKind::Goto { ref mut target } @@ -184,7 +183,6 @@ fn debug_basic_blocks(mir_body: &Body<'_>) -> String { TerminatorKind::Assert { target, .. } | TerminatorKind::Call { target: Some(target), .. } | TerminatorKind::Drop { target, .. } - | TerminatorKind::DropAndReplace { target, .. } | TerminatorKind::FalseEdge { real_target: target, .. } | TerminatorKind::FalseUnwind { real_target: target, .. } | TerminatorKind::Goto { target } diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index 08e296a837127..3a87c4d17f2bd 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -648,8 +648,7 @@ impl WriteInfo { TerminatorKind::Drop { .. } => { // `Drop`s create a `&mut` and so are not considered } - TerminatorKind::DropAndReplace { .. } - | TerminatorKind::Yield { .. } + TerminatorKind::Yield { .. } | TerminatorKind::GeneratorDrop | TerminatorKind::FalseEdge { .. } | TerminatorKind::FalseUnwind { .. } => { diff --git a/compiler/rustc_mir_transform/src/elaborate_drops.rs b/compiler/rustc_mir_transform/src/elaborate_drops.rs index 65f4956d23acd..2ed4e225304b2 100644 --- a/compiler/rustc_mir_transform/src/elaborate_drops.rs +++ b/compiler/rustc_mir_transform/src/elaborate_drops.rs @@ -93,8 +93,7 @@ fn find_dead_unwinds<'tcx>( .into_results_cursor(body); for (bb, bb_data) in body.basic_blocks.iter_enumerated() { let place = match bb_data.terminator().kind { - TerminatorKind::Drop { ref place, unwind: Some(_), .. } - | TerminatorKind::DropAndReplace { ref place, unwind: Some(_), .. } => { + TerminatorKind::Drop { ref place, unwind: Some(_), .. } => { und.derefer(place.as_ref(), body).unwrap_or(*place) } _ => continue, @@ -302,8 +301,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { for (bb, data) in self.body.basic_blocks.iter_enumerated() { let terminator = data.terminator(); let place = match terminator.kind { - TerminatorKind::Drop { ref place, .. } - | TerminatorKind::DropAndReplace { ref place, .. } => { + TerminatorKind::Drop { ref place, .. } => { self.un_derefer.derefer(place.as_ref(), self.body).unwrap_or(*place) } _ => continue, @@ -360,7 +358,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { let resume_block = self.patch.resume_block(); match terminator.kind { - TerminatorKind::Drop { mut place, target, unwind } => { + TerminatorKind::Drop { mut place, target, unwind, is_replace } => { if let Some(new_place) = self.un_derefer.derefer(place.as_ref(), self.body) { place = new_place; } @@ -379,112 +377,38 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { Unwind::To(Option::unwrap_or(unwind, resume_block)) }, bb, + is_replace, ), LookupResult::Parent(..) => { - self.tcx.sess.delay_span_bug( - terminator.source_info.span, - &format!("drop of untracked value {:?}", bb), + if !is_replace { + self.tcx.sess.delay_span_bug( + terminator.source_info.span, + &format!("drop of untracked value {:?}", bb), + ); + } + // If we cannot find it, it means it's a drop followed by a replace + // -> unconditional dro + Elaborator { ctxt: self }.patch().patch_terminator( + bb, + TerminatorKind::Drop { + place, + target, + unwind: if data.is_cleanup { + None + } else { + Some(Option::unwrap_or(unwind, resume_block)) + }, + is_replace, + }, ); } } } - TerminatorKind::DropAndReplace { mut place, ref value, target, unwind } => { - assert!(!data.is_cleanup); - - if let Some(new_place) = self.un_derefer.derefer(place.as_ref(), self.body) { - place = new_place; - } - self.elaborate_replace(loc, place, value, target, unwind); - } _ => continue, } } } - /// Elaborate a MIR `replace` terminator. This instruction - /// is not directly handled by codegen, and therefore - /// must be desugared. - /// - /// The desugaring drops the location if needed, and then writes - /// the value (including setting the drop flag) over it in *both* arms. - /// - /// The `replace` terminator can also be called on places that - /// are not tracked by elaboration (for example, - /// `replace x[i] <- tmp0`). The borrow checker requires that - /// these locations are initialized before the assignment, - /// so we just generate an unconditional drop. - fn elaborate_replace( - &mut self, - loc: Location, - place: Place<'tcx>, - value: &Operand<'tcx>, - target: BasicBlock, - unwind: Option, - ) { - let bb = loc.block; - let data = &self.body[bb]; - let terminator = data.terminator(); - assert!(!data.is_cleanup, "DropAndReplace in unwind path not supported"); - - let assign = Statement { - kind: StatementKind::Assign(Box::new((place, Rvalue::Use(value.clone())))), - source_info: terminator.source_info, - }; - - let unwind = unwind.unwrap_or_else(|| self.patch.resume_block()); - let unwind = self.patch.new_block(BasicBlockData { - statements: vec![assign.clone()], - terminator: Some(Terminator { - kind: TerminatorKind::Goto { target: unwind }, - ..*terminator - }), - is_cleanup: true, - }); - - let target = self.patch.new_block(BasicBlockData { - statements: vec![assign], - terminator: Some(Terminator { kind: TerminatorKind::Goto { target }, ..*terminator }), - is_cleanup: false, - }); - - match self.move_data().rev_lookup.find(place.as_ref()) { - LookupResult::Exact(path) => { - debug!("elaborate_drop_and_replace({:?}) - tracked {:?}", terminator, path); - self.init_data.seek_before(loc); - elaborate_drop( - &mut Elaborator { ctxt: self }, - terminator.source_info, - place, - path, - target, - Unwind::To(unwind), - bb, - ); - on_all_children_bits(self.tcx, self.body, self.move_data(), path, |child| { - self.set_drop_flag( - Location { block: target, statement_index: 0 }, - child, - DropFlagState::Present, - ); - self.set_drop_flag( - Location { block: unwind, statement_index: 0 }, - child, - DropFlagState::Present, - ); - }); - } - LookupResult::Parent(parent) => { - // drop and replace behind a pointer/array/whatever. The location - // must be initialized. - debug!("elaborate_drop_and_replace({:?}) - untracked {:?}", terminator, parent); - self.patch.patch_terminator( - bb, - TerminatorKind::Drop { place, target, unwind: Some(unwind) }, - ); - } - } - } - fn constant_bool(&self, span: Span, val: bool) -> Rvalue<'tcx> { Rvalue::Use(Operand::Constant(Box::new(Constant { span, @@ -550,22 +474,23 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { debug!("drop_flags_for_locs({:?})", data); for i in 0..(data.statements.len() + 1) { debug!("drop_flag_for_locs: stmt {}", i); - let mut allow_initializations = true; + let allow_initializations = true; if i == data.statements.len() { match data.terminator().kind { TerminatorKind::Drop { .. } => { // drop elaboration should handle that by itself continue; } - TerminatorKind::DropAndReplace { .. } => { - // this contains the move of the source and - // the initialization of the destination. We - // only want the former - the latter is handled - // by the elaboration code and must be done - // *after* the destination is dropped. - assert!(self.patch.is_patched(bb)); - allow_initializations = false; - } + // FIXME ?? + // TerminatorKind::DropAndReplace { .. } => { + // // this contains the move of the source and + // // the initialization of the destination. We + // // only want the former - the latter is handled + // // by the elaboration code and must be done + // // *after* the destination is dropped. + // assert!(self.patch.is_patched(bb)); + // allow_initializations = false; + // } TerminatorKind::Resume => { // It is possible for `Resume` to be patched // (in particular it can be patched to be replaced with diff --git a/compiler/rustc_mir_transform/src/generator.rs b/compiler/rustc_mir_transform/src/generator.rs index c097af6161159..67a1bc341b4eb 100644 --- a/compiler/rustc_mir_transform/src/generator.rs +++ b/compiler/rustc_mir_transform/src/generator.rs @@ -911,11 +911,14 @@ fn elaborate_generator_drops<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { let mut elaborator = DropShimElaborator { body, patch: MirPatch::new(body), tcx, param_env }; for (block, block_data) in body.basic_blocks.iter_enumerated() { - let (target, unwind, source_info) = match block_data.terminator() { - Terminator { source_info, kind: TerminatorKind::Drop { place, target, unwind } } => { + let (target, unwind, source_info, is_replace) = match block_data.terminator() { + Terminator { + source_info, + kind: TerminatorKind::Drop { place, target, unwind, is_replace }, + } => { if let Some(local) = place.as_local() { if local == SELF_ARG { - (target, unwind, source_info) + (target, unwind, source_info, is_replace) } else { continue; } @@ -938,6 +941,7 @@ fn elaborate_generator_drops<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { *target, unwind, block, + *is_replace, ); } elaborator.patch.apply(body); @@ -1074,7 +1078,6 @@ fn can_unwind<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) -> bool { // These may unwind. TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } | TerminatorKind::Call { .. } | TerminatorKind::InlineAsm { .. } | TerminatorKind::Assert { .. } => return true, @@ -1163,8 +1166,12 @@ fn create_generator_resume_function<'tcx>( fn insert_clean_drop(body: &mut Body<'_>) -> BasicBlock { let return_block = insert_term_block(body, TerminatorKind::Return); - let term = - TerminatorKind::Drop { place: Place::from(SELF_ARG), target: return_block, unwind: None }; + let term = TerminatorKind::Drop { + place: Place::from(SELF_ARG), + target: return_block, + unwind: None, + is_replace: false, + }; let source_info = SourceInfo::outermost(body.span); // Create a block to destroy an unresumed generators. This can only destroy upvars. @@ -1519,7 +1526,6 @@ impl<'tcx> Visitor<'tcx> for EnsureGeneratorFieldAssignmentsNeverAlias<'_> { | TerminatorKind::Return | TerminatorKind::Unreachable | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } | TerminatorKind::Assert { .. } | TerminatorKind::GeneratorDrop | TerminatorKind::FalseEdge { .. } diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index 4219e6280ebbc..d82b9f85d189e 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -453,9 +453,7 @@ impl<'tcx> Inliner<'tcx> { checker.visit_basic_block_data(bb, blk); let term = blk.terminator(); - if let TerminatorKind::Drop { ref place, target, unwind } - | TerminatorKind::DropAndReplace { ref place, target, unwind, .. } = term.kind - { + if let TerminatorKind::Drop { ref place, target, unwind, .. } = term.kind { work_list.push(target); // If the place doesn't actually need dropping, treat it like a regular goto. @@ -790,8 +788,7 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> { fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { let tcx = self.tcx; match terminator.kind { - TerminatorKind::Drop { ref place, unwind, .. } - | TerminatorKind::DropAndReplace { ref place, unwind, .. } => { + TerminatorKind::Drop { ref place, unwind, .. } => { // If the place doesn't actually need dropping, treat it like a regular goto. let ty = self.instance.subst_mir(tcx, &place.ty(self.callee_body, tcx).ty); if ty.needs_drop(tcx, self.param_env) { @@ -1116,8 +1113,7 @@ impl<'tcx> MutVisitor<'tcx> for Integrator<'_, 'tcx> { *tgt = self.map_block(*tgt); } } - TerminatorKind::Drop { ref mut target, ref mut unwind, .. } - | TerminatorKind::DropAndReplace { ref mut target, ref mut unwind, .. } => { + TerminatorKind::Drop { ref mut target, ref mut unwind, .. } => { *target = self.map_block(*target); *unwind = self.map_unwind(*unwind); } diff --git a/compiler/rustc_mir_transform/src/remove_noop_landing_pads.rs b/compiler/rustc_mir_transform/src/remove_noop_landing_pads.rs index f1bbf2ea7e8ea..1746540744f1c 100644 --- a/compiler/rustc_mir_transform/src/remove_noop_landing_pads.rs +++ b/compiler/rustc_mir_transform/src/remove_noop_landing_pads.rs @@ -74,7 +74,6 @@ impl RemoveNoopLandingPads { | TerminatorKind::Unreachable | TerminatorKind::Call { .. } | TerminatorKind::Assert { .. } - | TerminatorKind::DropAndReplace { .. } | TerminatorKind::Drop { .. } | TerminatorKind::InlineAsm { .. } => false, } diff --git a/compiler/rustc_mir_transform/src/remove_uninit_drops.rs b/compiler/rustc_mir_transform/src/remove_uninit_drops.rs index 78b6f714a9b0b..a9096ba3aad74 100644 --- a/compiler/rustc_mir_transform/src/remove_uninit_drops.rs +++ b/compiler/rustc_mir_transform/src/remove_uninit_drops.rs @@ -1,5 +1,5 @@ use rustc_index::bit_set::ChunkedBitSet; -use rustc_middle::mir::{Body, Field, Rvalue, Statement, StatementKind, TerminatorKind}; +use rustc_middle::mir::{Body, Field, TerminatorKind}; use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt, VariantDef}; use rustc_mir_dataflow::impls::MaybeInitializedPlaces; @@ -8,6 +8,7 @@ use rustc_mir_dataflow::{self, move_path_children_matching, Analysis, MoveDataPa use crate::MirPass; +// FIXME: can remove this altogether by moving drop elaboration *before* borrowck /// Removes `Drop` and `DropAndReplace` terminators whose target is known to be uninitialized at /// that point. /// @@ -37,7 +38,7 @@ impl<'tcx> MirPass<'tcx> for RemoveUninitDrops { let mut to_remove = vec![]; for (bb, block) in body.basic_blocks.iter_enumerated() { let terminator = block.terminator(); - let (TerminatorKind::Drop { place, .. } | TerminatorKind::DropAndReplace { place, .. }) + let TerminatorKind::Drop { place, .. } = &terminator.kind else { continue }; @@ -64,24 +65,16 @@ impl<'tcx> MirPass<'tcx> for RemoveUninitDrops { for bb in to_remove { let block = &mut body.basic_blocks_mut()[bb]; - let (TerminatorKind::Drop { target, .. } | TerminatorKind::DropAndReplace { target, .. }) + let TerminatorKind::Drop { target, .. } = &block.terminator().kind else { unreachable!() }; // Replace block terminator with `Goto`. let target = *target; - let old_terminator_kind = std::mem::replace( + let _ = std::mem::replace( &mut block.terminator_mut().kind, TerminatorKind::Goto { target }, ); - - // If this is a `DropAndReplace`, we need to emulate the assignment to the return place. - if let TerminatorKind::DropAndReplace { place, value, .. } = old_terminator_kind { - block.statements.push(Statement { - source_info: block.terminator().source_info, - kind: StatementKind::Assign(Box::new((place, Rvalue::Use(value)))), - }); - } } } } diff --git a/compiler/rustc_mir_transform/src/separate_const_switch.rs b/compiler/rustc_mir_transform/src/separate_const_switch.rs index 2f116aaa95849..151c02293bf2b 100644 --- a/compiler/rustc_mir_transform/src/separate_const_switch.rs +++ b/compiler/rustc_mir_transform/src/separate_const_switch.rs @@ -108,7 +108,6 @@ pub fn separate_const_switch(body: &mut Body<'_>) -> usize { // The following terminators are not allowed TerminatorKind::Resume | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } | TerminatorKind::Call { .. } | TerminatorKind::Assert { .. } | TerminatorKind::FalseUnwind { .. } @@ -170,7 +169,6 @@ pub fn separate_const_switch(body: &mut Body<'_>) -> usize { | TerminatorKind::Unreachable | TerminatorKind::GeneratorDrop | TerminatorKind::Assert { .. } - | TerminatorKind::DropAndReplace { .. } | TerminatorKind::FalseUnwind { .. } | TerminatorKind::Drop { .. } | TerminatorKind::Call { .. } diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index dace540fa29d2..6b953e593ceb2 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -218,6 +218,7 @@ fn build_drop_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, ty: Option>) return_block, elaborate_drops::Unwind::To(resume_block), START_BLOCK, + false, ); elaborator.patch }; @@ -511,7 +512,12 @@ impl<'tcx> CloneShimBuilder<'tcx> { self.make_clone_call(dest_field, src_field, ity, next_block, unwind); self.block( vec![], - TerminatorKind::Drop { place: dest_field, target: unwind, unwind: None }, + TerminatorKind::Drop { + place: dest_field, + target: unwind, + unwind: None, + is_replace: false, + }, true, ); unwind = next_unwind; @@ -763,7 +769,12 @@ fn build_call_shim<'tcx>( block( &mut blocks, vec![], - TerminatorKind::Drop { place: rcvr_place(), target: BasicBlock::new(2), unwind: None }, + TerminatorKind::Drop { + place: rcvr_place(), + target: BasicBlock::new(2), + unwind: None, + is_replace: false, + }, false, ); } @@ -774,7 +785,12 @@ fn build_call_shim<'tcx>( block( &mut blocks, vec![], - TerminatorKind::Drop { place: rcvr_place(), target: BasicBlock::new(4), unwind: None }, + TerminatorKind::Drop { + place: rcvr_place(), + target: BasicBlock::new(4), + unwind: None, + is_replace: false, + }, true, ); diff --git a/compiler/rustc_mir_transform/src/sroa.rs b/compiler/rustc_mir_transform/src/sroa.rs index 3a2bf05151655..48978a1fb7d1a 100644 --- a/compiler/rustc_mir_transform/src/sroa.rs +++ b/compiler/rustc_mir_transform/src/sroa.rs @@ -83,9 +83,7 @@ fn escaping_locals(body: &Body<'_>) -> BitSet { fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { // Drop implicitly calls `drop_in_place`, which takes a `&mut`. // This implies that `Drop` implicitly takes the address of the place. - if let TerminatorKind::Drop { place, .. } - | TerminatorKind::DropAndReplace { place, .. } = terminator.kind - { + if let TerminatorKind::Drop { place, .. } = terminator.kind { if !place.is_indirect() { // Raw pointers may be used to access anything inside the enclosing place. self.set.insert(place.local); diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 59cc500a99da7..e3bda9416abde 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -806,8 +806,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { let callee_ty = self.monomorphize(callee_ty); visit_fn_use(self.tcx, callee_ty, true, source, &mut self.output) } - mir::TerminatorKind::Drop { ref place, .. } - | mir::TerminatorKind::DropAndReplace { ref place, .. } => { + mir::TerminatorKind::Drop { ref place, .. } => { let ty = place.ty(self.body, self.tcx).ty; let ty = self.monomorphize(ty); visit_drop_use(self.tcx, ty, true, source, self.output); diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index e5d7da682813c..69fdc8590d6a0 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -296,10 +296,6 @@ fn check_terminator<'tcx>( | TerminatorKind::Unreachable => Ok(()), TerminatorKind::Drop { place, .. } => check_place(tcx, *place, span, body), - TerminatorKind::DropAndReplace { place, value, .. } => { - check_place(tcx, *place, span, body)?; - check_operand(tcx, value, span, body) - }, TerminatorKind::SwitchInt { discr, targets: _ } => check_operand(tcx, discr, span, body), From 474da0fcb4ec204e6384cef763d6aede249a34e5 Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Tue, 27 Dec 2022 11:25:18 +0100 Subject: [PATCH 02/13] add option to run dataflow on live code only --- compiler/rustc_borrowck/src/lib.rs | 10 ++-- .../src/transform/check_consts/check.rs | 10 ++-- .../src/transform/validate.rs | 4 +- .../src/framework/engine.rs | 48 +++++++++++++++---- .../rustc_mir_dataflow/src/framework/mod.rs | 8 ++-- compiler/rustc_mir_dataflow/src/lib.rs | 7 +-- compiler/rustc_mir_dataflow/src/rustc_peek.rs | 11 +++-- .../src/dataflow_const_prop.rs | 6 ++- .../src/dead_store_elimination.rs | 4 +- compiler/rustc_mir_transform/src/dest_prop.rs | 4 +- .../src/elaborate_drops.rs | 8 ++-- compiler/rustc_mir_transform/src/generator.rs | 14 +++--- .../src/remove_uninit_drops.rs | 4 +- 13 files changed, 89 insertions(+), 49 deletions(-) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index ef0a03a546fe1..928c15d6e38a9 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -48,8 +48,8 @@ use rustc_mir_dataflow::impls::{ }; use rustc_mir_dataflow::move_paths::{InitIndex, MoveOutIndex, MovePathIndex}; use rustc_mir_dataflow::move_paths::{InitLocation, LookupResult, MoveData, MoveError}; -use rustc_mir_dataflow::Analysis; use rustc_mir_dataflow::MoveDataParamEnv; +use rustc_mir_dataflow::{Analysis, Blocks}; use crate::session_diagnostics::VarNeedNotMut; @@ -234,7 +234,7 @@ fn do_mir_borrowck<'tcx>( let mdpe = MoveDataParamEnv { move_data, param_env }; let mut flow_inits = MaybeInitializedPlaces::new(tcx, &body, &mdpe) - .into_engine(tcx, &body) + .into_engine(tcx, &body, Blocks::All) .pass_name("borrowck") .iterate_to_fixpoint() .into_results_cursor(&body); @@ -290,15 +290,15 @@ fn do_mir_borrowck<'tcx>( let regioncx = Rc::new(regioncx); let flow_borrows = Borrows::new(tcx, body, ®ioncx, &borrow_set) - .into_engine(tcx, body) + .into_engine(tcx, body, Blocks::All) .pass_name("borrowck") .iterate_to_fixpoint(); let flow_uninits = MaybeUninitializedPlaces::new(tcx, body, &mdpe) - .into_engine(tcx, body) + .into_engine(tcx, body, Blocks::All) .pass_name("borrowck") .iterate_to_fixpoint(); let flow_ever_inits = EverInitializedPlaces::new(tcx, body, &mdpe) - .into_engine(tcx, body) + .into_engine(tcx, body, Blocks::All) .pass_name("borrowck") .iterate_to_fixpoint(); diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs index a2624c7408cc4..049a0393632fb 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/check.rs @@ -11,7 +11,7 @@ use rustc_middle::mir::*; use rustc_middle::ty::subst::{GenericArgKind, InternalSubsts}; use rustc_middle::ty::{self, adjustment::PointerCast, Instance, InstanceDef, Ty, TyCtxt}; use rustc_middle::ty::{Binder, TraitRef, TypeVisitable}; -use rustc_mir_dataflow::{self, Analysis}; +use rustc_mir_dataflow::{self, Analysis, Blocks}; use rustc_span::{sym, Span, Symbol}; use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _; use rustc_trait_selection::traits::{self, ObligationCauseCode, ObligationCtxt, SelectionContext}; @@ -58,7 +58,7 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> { let ConstCx { tcx, body, .. } = *ccx; FlowSensitiveAnalysis::new(NeedsDrop, ccx) - .into_engine(tcx, &body) + .into_engine(tcx, &body, Blocks::All) .iterate_to_fixpoint() .into_results_cursor(&body) }); @@ -85,7 +85,7 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> { let ConstCx { tcx, body, .. } = *ccx; FlowSensitiveAnalysis::new(NeedsNonConstDrop, ccx) - .into_engine(tcx, &body) + .into_engine(tcx, &body, Blocks::All) .iterate_to_fixpoint() .into_results_cursor(&body) }); @@ -115,7 +115,7 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> { let ConstCx { tcx, body, .. } = *ccx; FlowSensitiveAnalysis::new(HasMutInterior, ccx) - .into_engine(tcx, &body) + .into_engine(tcx, &body, Blocks::All) .iterate_to_fixpoint() .into_results_cursor(&body) }); @@ -163,7 +163,7 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> { hir::ConstContext::Const | hir::ConstContext::Static(_) => { let mut cursor = FlowSensitiveAnalysis::new(CustomEq, ccx) - .into_engine(ccx.tcx, &ccx.body) + .into_engine(ccx.tcx, &ccx.body, Blocks::All) .iterate_to_fixpoint() .into_results_cursor(&ccx.body); diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index 87cea20ca3d91..71bda6d486aa2 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -15,7 +15,7 @@ use rustc_middle::mir::{ use rustc_middle::ty::{self, InstanceDef, ParamEnv, Ty, TyCtxt, TypeVisitable}; use rustc_mir_dataflow::impls::MaybeStorageLive; use rustc_mir_dataflow::storage::always_storage_live_locals; -use rustc_mir_dataflow::{Analysis, ResultsCursor}; +use rustc_mir_dataflow::{Analysis, Blocks, ResultsCursor}; use rustc_target::abi::{Size, VariantIdx}; #[derive(Copy, Clone, Debug)] @@ -53,7 +53,7 @@ impl<'tcx> MirPass<'tcx> for Validator { let always_live_locals = always_storage_live_locals(body); let storage_liveness = MaybeStorageLive::new(std::borrow::Cow::Owned(always_live_locals)) - .into_engine(tcx, body) + .into_engine(tcx, body, Blocks::All) .iterate_to_fixpoint() .into_results_cursor(body); diff --git a/compiler/rustc_mir_dataflow/src/framework/engine.rs b/compiler/rustc_mir_dataflow/src/framework/engine.rs index 6ddbe69e17e75..64d3eadc0016c 100644 --- a/compiler/rustc_mir_dataflow/src/framework/engine.rs +++ b/compiler/rustc_mir_dataflow/src/framework/engine.rs @@ -71,6 +71,13 @@ where } } +pub enum Blocks { + /// Consider all MIR blocks + All, + /// Consider only the MIR blocks reachable from the start + Reachable, +} + /// A solver for dataflow problems. pub struct Engine<'a, 'tcx, A> where @@ -90,6 +97,7 @@ where // performance in practice. I've tried a few ways to avoid this, but they have downsides. See // the message for the commit that added this FIXME for more information. apply_trans_for_block: Option>, + blocks: Blocks, } impl<'a, 'tcx, A, D, T> Engine<'a, 'tcx, A> @@ -99,13 +107,18 @@ where T: Idx, { /// Creates a new `Engine` to solve a gen-kill dataflow problem. - pub fn new_gen_kill(tcx: TyCtxt<'tcx>, body: &'a mir::Body<'tcx>, analysis: A) -> Self { + pub fn new_gen_kill( + tcx: TyCtxt<'tcx>, + body: &'a mir::Body<'tcx>, + analysis: A, + blocks: Blocks, + ) -> Self { // If there are no back-edges in the control-flow graph, we only ever need to apply the // transfer function for each block exactly once (assuming that we process blocks in RPO). // // In this case, there's no need to compute the block transfer functions ahead of time. if !body.basic_blocks.is_cfg_cyclic() { - return Self::new(tcx, body, analysis, None); + return Self::new(tcx, body, analysis, None, blocks); } // Otherwise, compute and store the cumulative transfer function for each block. @@ -122,7 +135,7 @@ where trans_for_block[bb].apply(state); }); - Self::new(tcx, body, analysis, Some(apply_trans as Box<_>)) + Self::new(tcx, body, analysis, Some(apply_trans as Box<_>), blocks) } } @@ -136,8 +149,13 @@ where /// /// Gen-kill problems should use `new_gen_kill`, which will coalesce transfer functions for /// better performance. - pub fn new_generic(tcx: TyCtxt<'tcx>, body: &'a mir::Body<'tcx>, analysis: A) -> Self { - Self::new(tcx, body, analysis, None) + pub fn new_generic( + tcx: TyCtxt<'tcx>, + body: &'a mir::Body<'tcx>, + analysis: A, + blocks: Blocks, + ) -> Self { + Self::new(tcx, body, analysis, None, blocks) } fn new( @@ -145,6 +163,7 @@ where body: &'a mir::Body<'tcx>, analysis: A, apply_trans_for_block: Option>, + blocks: Blocks, ) -> Self { let bottom_value = analysis.bottom_value(body); let mut entry_sets = IndexVec::from_elem(bottom_value.clone(), &body.basic_blocks); @@ -162,6 +181,7 @@ where pass_name: None, entry_sets, apply_trans_for_block, + blocks, } } @@ -197,14 +217,24 @@ where tcx, apply_trans_for_block, pass_name, + blocks, .. } = self; let mut dirty_queue: WorkQueue = WorkQueue::with_none(body.basic_blocks.len()); + // could be tracked in `entry_sets` + let mut visited = BitSet::new_empty(body.basic_blocks.len()); if A::Direction::IS_FORWARD { - for (bb, _) in traversal::reverse_postorder(body) { - dirty_queue.insert(bb); + match blocks { + Blocks::All => { + for (bb, _) in traversal::reverse_postorder(body) { + dirty_queue.insert(bb); + } + } + Blocks::Reachable => { + dirty_queue.insert(mir::START_BLOCK); + } } } else { // Reverse post-order on the reverse CFG may generate a better iteration order for @@ -212,6 +242,7 @@ where for (bb, _) in traversal::postorder(body) { dirty_queue.insert(bb); } + assert!(matches!(blocks, Blocks::All)); } // `state` is not actually used between iterations; @@ -241,8 +272,9 @@ where (bb, bb_data), |target: BasicBlock, state: &A::Domain| { let set_changed = entry_sets[target].join(state); - if set_changed { + if set_changed || !visited.contains(target) { dirty_queue.insert(target); + visited.insert(target); } }, ); diff --git a/compiler/rustc_mir_dataflow/src/framework/mod.rs b/compiler/rustc_mir_dataflow/src/framework/mod.rs index d9aff94fef2f9..daf164c5175f2 100644 --- a/compiler/rustc_mir_dataflow/src/framework/mod.rs +++ b/compiler/rustc_mir_dataflow/src/framework/mod.rs @@ -47,7 +47,7 @@ mod visitor; pub use self::cursor::{ResultsCursor, ResultsRefCursor}; pub use self::direction::{Backward, Direction, Forward}; -pub use self::engine::{Engine, Results}; +pub use self::engine::{Blocks, Engine, Results}; pub use self::lattice::{JoinSemiLattice, MeetSemiLattice}; pub use self::visitor::{visit_results, ResultsVisitable, ResultsVisitor}; @@ -261,11 +261,12 @@ pub trait Analysis<'tcx>: AnalysisDomain<'tcx> { self, tcx: TyCtxt<'tcx>, body: &'mir mir::Body<'tcx>, + blocks: Blocks, ) -> Engine<'mir, 'tcx, Self> where Self: Sized, { - Engine::new_generic(tcx, body, self) + Engine::new_generic(tcx, body, self, blocks) } } @@ -419,11 +420,12 @@ where self, tcx: TyCtxt<'tcx>, body: &'mir mir::Body<'tcx>, + blocks: Blocks, ) -> Engine<'mir, 'tcx, Self> where Self: Sized, { - Engine::new_gen_kill(tcx, body, self) + Engine::new_gen_kill(tcx, body, self, blocks) } } diff --git a/compiler/rustc_mir_dataflow/src/lib.rs b/compiler/rustc_mir_dataflow/src/lib.rs index 7f40cfca32fff..e8bec506392cf 100644 --- a/compiler/rustc_mir_dataflow/src/lib.rs +++ b/compiler/rustc_mir_dataflow/src/lib.rs @@ -25,9 +25,10 @@ pub use self::drop_flag_effects::{ on_lookup_result_bits, }; pub use self::framework::{ - fmt, graphviz, lattice, visit_results, Analysis, AnalysisDomain, Backward, CallReturnPlaces, - Direction, Engine, Forward, GenKill, GenKillAnalysis, JoinSemiLattice, Results, ResultsCursor, - ResultsRefCursor, ResultsVisitable, ResultsVisitor, SwitchIntEdgeEffects, + fmt, graphviz, lattice, visit_results, Analysis, AnalysisDomain, Backward, Blocks, + CallReturnPlaces, Direction, Engine, Forward, GenKill, GenKillAnalysis, JoinSemiLattice, + Results, ResultsCursor, ResultsRefCursor, ResultsVisitable, ResultsVisitor, + SwitchIntEdgeEffects, }; use self::move_paths::MoveData; diff --git a/compiler/rustc_mir_dataflow/src/rustc_peek.rs b/compiler/rustc_mir_dataflow/src/rustc_peek.rs index 7cae68efbecc3..55b4b8f395b35 100644 --- a/compiler/rustc_mir_dataflow/src/rustc_peek.rs +++ b/compiler/rustc_mir_dataflow/src/rustc_peek.rs @@ -10,7 +10,7 @@ use crate::errors::{ PeekArgumentNotALocal, PeekArgumentUntracked, PeekBitNotSet, PeekMustBeNotTemporary, PeekMustBePlaceOrRefPlace, StopAfterDataFlowEndedCompilation, }; -use crate::framework::BitSetExt; +use crate::framework::{BitSetExt, Blocks}; use crate::impls::{ DefinitelyInitializedPlaces, MaybeInitializedPlaces, MaybeLiveLocals, MaybeUninitializedPlaces, }; @@ -39,7 +39,7 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { if has_rustc_mir_with(tcx, def_id, sym::rustc_peek_maybe_init).is_some() { let flow_inits = MaybeInitializedPlaces::new(tcx, body, &mdpe) - .into_engine(tcx, body) + .into_engine(tcx, body, Blocks::All) .iterate_to_fixpoint(); sanity_check_via_rustc_peek(tcx, body, &flow_inits); @@ -47,7 +47,7 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { if has_rustc_mir_with(tcx, def_id, sym::rustc_peek_maybe_uninit).is_some() { let flow_uninits = MaybeUninitializedPlaces::new(tcx, body, &mdpe) - .into_engine(tcx, body) + .into_engine(tcx, body, Blocks::All) .iterate_to_fixpoint(); sanity_check_via_rustc_peek(tcx, body, &flow_uninits); @@ -55,14 +55,15 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { if has_rustc_mir_with(tcx, def_id, sym::rustc_peek_definite_init).is_some() { let flow_def_inits = DefinitelyInitializedPlaces::new(tcx, body, &mdpe) - .into_engine(tcx, body) + .into_engine(tcx, body, Blocks::All) .iterate_to_fixpoint(); sanity_check_via_rustc_peek(tcx, body, &flow_def_inits); } if has_rustc_mir_with(tcx, def_id, sym::rustc_peek_liveness).is_some() { - let flow_liveness = MaybeLiveLocals.into_engine(tcx, body).iterate_to_fixpoint(); + let flow_liveness = + MaybeLiveLocals.into_engine(tcx, body, Blocks::All).iterate_to_fixpoint(); sanity_check_via_rustc_peek(tcx, body, &flow_liveness); } diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index c75fe2327de3e..34938082beede 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -9,7 +9,9 @@ use rustc_middle::mir::visit::{MutVisitor, Visitor}; use rustc_middle::mir::*; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_mir_dataflow::value_analysis::{Map, State, TrackElem, ValueAnalysis, ValueOrPlace}; -use rustc_mir_dataflow::{lattice::FlatSet, Analysis, ResultsVisitor, SwitchIntEdgeEffects}; +use rustc_mir_dataflow::{ + lattice::FlatSet, Analysis, Blocks, ResultsVisitor, SwitchIntEdgeEffects, +}; use rustc_span::DUMMY_SP; use rustc_target::abi::Align; @@ -53,7 +55,7 @@ impl<'tcx> MirPass<'tcx> for DataflowConstProp { // Perform the actual dataflow analysis. let analysis = ConstAnalysis::new(tcx, body, map); let results = debug_span!("analyze") - .in_scope(|| analysis.wrap().into_engine(tcx, body).iterate_to_fixpoint()); + .in_scope(|| analysis.wrap().into_engine(tcx, body, Blocks::All).iterate_to_fixpoint()); // Collect results and patch the body afterwards. let mut visitor = CollectAndPatch::new(tcx, &results.analysis.0.map); diff --git a/compiler/rustc_mir_transform/src/dead_store_elimination.rs b/compiler/rustc_mir_transform/src/dead_store_elimination.rs index 42c580c63f060..af756d32f4335 100644 --- a/compiler/rustc_mir_transform/src/dead_store_elimination.rs +++ b/compiler/rustc_mir_transform/src/dead_store_elimination.rs @@ -16,7 +16,7 @@ use rustc_index::bit_set::BitSet; use rustc_middle::mir::*; use rustc_middle::ty::TyCtxt; use rustc_mir_dataflow::impls::{borrowed_locals, MaybeTransitiveLiveLocals}; -use rustc_mir_dataflow::Analysis; +use rustc_mir_dataflow::{Analysis, Blocks}; /// Performs the optimization on the body /// @@ -24,7 +24,7 @@ use rustc_mir_dataflow::Analysis; /// can be generated via the [`borrowed_locals`] function. pub fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, borrowed: &BitSet) { let mut live = MaybeTransitiveLiveLocals::new(borrowed) - .into_engine(tcx, body) + .into_engine(tcx, body, Blocks::All) .iterate_to_fixpoint() .into_results_cursor(body); diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index 3a87c4d17f2bd..5b51dfaf4b6f8 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -141,7 +141,7 @@ use rustc_middle::mir::{ }; use rustc_middle::ty::TyCtxt; use rustc_mir_dataflow::impls::MaybeLiveLocals; -use rustc_mir_dataflow::{Analysis, ResultsCursor}; +use rustc_mir_dataflow::{Analysis, Blocks, ResultsCursor}; pub struct DestinationPropagation; @@ -189,7 +189,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation { ); trace!(?candidates); let mut live = MaybeLiveLocals - .into_engine(tcx, body) + .into_engine(tcx, body, Blocks::All) .iterate_to_fixpoint() .into_results_cursor(body); dest_prop_mir_dump(tcx, body, &mut live, round_count); diff --git a/compiler/rustc_mir_transform/src/elaborate_drops.rs b/compiler/rustc_mir_transform/src/elaborate_drops.rs index 2ed4e225304b2..701d1c6d0224d 100644 --- a/compiler/rustc_mir_transform/src/elaborate_drops.rs +++ b/compiler/rustc_mir_transform/src/elaborate_drops.rs @@ -13,7 +13,7 @@ use rustc_mir_dataflow::on_lookup_result_bits; use rustc_mir_dataflow::un_derefer::UnDerefer; use rustc_mir_dataflow::MoveDataParamEnv; use rustc_mir_dataflow::{on_all_children_bits, on_all_drop_children_bits}; -use rustc_mir_dataflow::{Analysis, ResultsCursor}; +use rustc_mir_dataflow::{Analysis, Blocks, ResultsCursor}; use rustc_span::Span; use rustc_target::abi::VariantIdx; use std::fmt; @@ -43,7 +43,7 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops { let dead_unwinds = find_dead_unwinds(tcx, body, &env, &un_derefer); let inits = MaybeInitializedPlaces::new(tcx, body, &env) - .into_engine(tcx, body) + .into_engine(tcx, body, Blocks::Reachable) .dead_unwinds(&dead_unwinds) .pass_name("elaborate_drops") .iterate_to_fixpoint() @@ -51,7 +51,7 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops { let uninits = MaybeUninitializedPlaces::new(tcx, body, &env) .mark_inactive_variants_as_uninit() - .into_engine(tcx, body) + .into_engine(tcx, body, Blocks::Reachable) .dead_unwinds(&dead_unwinds) .pass_name("elaborate_drops") .iterate_to_fixpoint() @@ -87,7 +87,7 @@ fn find_dead_unwinds<'tcx>( // reach cleanup blocks, which can't have unwind edges themselves. let mut dead_unwinds = BitSet::new_empty(body.basic_blocks.len()); let mut flow_inits = MaybeInitializedPlaces::new(tcx, body, &env) - .into_engine(tcx, body) + .into_engine(tcx, body, Blocks::Reachable) .pass_name("find_dead_unwinds") .iterate_to_fixpoint() .into_results_cursor(body); diff --git a/compiler/rustc_mir_transform/src/generator.rs b/compiler/rustc_mir_transform/src/generator.rs index 67a1bc341b4eb..2c4790f748c21 100644 --- a/compiler/rustc_mir_transform/src/generator.rs +++ b/compiler/rustc_mir_transform/src/generator.rs @@ -69,7 +69,7 @@ use rustc_mir_dataflow::impls::{ MaybeBorrowedLocals, MaybeLiveLocals, MaybeRequiresStorage, MaybeStorageLive, }; use rustc_mir_dataflow::storage::always_storage_live_locals; -use rustc_mir_dataflow::{self, Analysis}; +use rustc_mir_dataflow::{self, Analysis, Blocks}; use rustc_target::abi::VariantIdx; use rustc_target::spec::PanicStrategy; use std::{iter, ops}; @@ -491,14 +491,16 @@ fn locals_live_across_suspend_points<'tcx>( // Calculate when MIR locals have live storage. This gives us an upper bound of their // lifetimes. let mut storage_live = MaybeStorageLive::new(std::borrow::Cow::Borrowed(always_live_locals)) - .into_engine(tcx, body_ref) + .into_engine(tcx, body_ref, Blocks::All) .iterate_to_fixpoint() .into_results_cursor(body_ref); // Calculate the MIR locals which have been previously // borrowed (even if they are still active). - let borrowed_locals_results = - MaybeBorrowedLocals.into_engine(tcx, body_ref).pass_name("generator").iterate_to_fixpoint(); + let borrowed_locals_results = MaybeBorrowedLocals + .into_engine(tcx, body_ref, Blocks::All) + .pass_name("generator") + .iterate_to_fixpoint(); let mut borrowed_locals_cursor = rustc_mir_dataflow::ResultsCursor::new(body_ref, &borrowed_locals_results); @@ -506,14 +508,14 @@ fn locals_live_across_suspend_points<'tcx>( // Calculate the MIR locals that we actually need to keep storage around // for. let requires_storage_results = MaybeRequiresStorage::new(body, &borrowed_locals_results) - .into_engine(tcx, body_ref) + .into_engine(tcx, body_ref, Blocks::All) .iterate_to_fixpoint(); let mut requires_storage_cursor = rustc_mir_dataflow::ResultsCursor::new(body_ref, &requires_storage_results); // Calculate the liveness of MIR locals ignoring borrows. let mut liveness = MaybeLiveLocals - .into_engine(tcx, body_ref) + .into_engine(tcx, body_ref, Blocks::All) .pass_name("generator") .iterate_to_fixpoint() .into_results_cursor(body_ref); diff --git a/compiler/rustc_mir_transform/src/remove_uninit_drops.rs b/compiler/rustc_mir_transform/src/remove_uninit_drops.rs index a9096ba3aad74..f62e0380dccf1 100644 --- a/compiler/rustc_mir_transform/src/remove_uninit_drops.rs +++ b/compiler/rustc_mir_transform/src/remove_uninit_drops.rs @@ -4,7 +4,7 @@ use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt, VariantDef}; use rustc_mir_dataflow::impls::MaybeInitializedPlaces; use rustc_mir_dataflow::move_paths::{LookupResult, MoveData, MovePathIndex}; -use rustc_mir_dataflow::{self, move_path_children_matching, Analysis, MoveDataParamEnv}; +use rustc_mir_dataflow::{self, move_path_children_matching, Analysis, Blocks, MoveDataParamEnv}; use crate::MirPass; @@ -30,7 +30,7 @@ impl<'tcx> MirPass<'tcx> for RemoveUninitDrops { let mdpe = MoveDataParamEnv { move_data, param_env }; let mut maybe_inits = MaybeInitializedPlaces::new(tcx, body, &mdpe) - .into_engine(tcx, body) + .into_engine(tcx, body, Blocks::All) .pass_name("remove_uninit_drops") .iterate_to_fixpoint() .into_results_cursor(body); From ef51d1e20d10f484990adb5f4a016ed712104a91 Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Thu, 29 Dec 2022 01:53:10 +0100 Subject: [PATCH 03/13] remove unneeded dataflow engine blocks flag --- compiler/rustc_borrowck/src/lib.rs | 10 ++--- .../src/transform/check_consts/check.rs | 10 ++--- .../src/transform/validate.rs | 4 +- .../src/framework/engine.rs | 43 +++---------------- .../rustc_mir_dataflow/src/framework/mod.rs | 8 ++-- compiler/rustc_mir_dataflow/src/lib.rs | 7 ++- compiler/rustc_mir_dataflow/src/rustc_peek.rs | 11 +++-- .../src/dataflow_const_prop.rs | 6 +-- .../src/dead_store_elimination.rs | 4 +- compiler/rustc_mir_transform/src/dest_prop.rs | 4 +- .../src/elaborate_drops.rs | 8 ++-- compiler/rustc_mir_transform/src/generator.rs | 14 +++--- .../src/remove_uninit_drops.rs | 4 +- 13 files changed, 47 insertions(+), 86 deletions(-) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 928c15d6e38a9..ef0a03a546fe1 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -48,8 +48,8 @@ use rustc_mir_dataflow::impls::{ }; use rustc_mir_dataflow::move_paths::{InitIndex, MoveOutIndex, MovePathIndex}; use rustc_mir_dataflow::move_paths::{InitLocation, LookupResult, MoveData, MoveError}; +use rustc_mir_dataflow::Analysis; use rustc_mir_dataflow::MoveDataParamEnv; -use rustc_mir_dataflow::{Analysis, Blocks}; use crate::session_diagnostics::VarNeedNotMut; @@ -234,7 +234,7 @@ fn do_mir_borrowck<'tcx>( let mdpe = MoveDataParamEnv { move_data, param_env }; let mut flow_inits = MaybeInitializedPlaces::new(tcx, &body, &mdpe) - .into_engine(tcx, &body, Blocks::All) + .into_engine(tcx, &body) .pass_name("borrowck") .iterate_to_fixpoint() .into_results_cursor(&body); @@ -290,15 +290,15 @@ fn do_mir_borrowck<'tcx>( let regioncx = Rc::new(regioncx); let flow_borrows = Borrows::new(tcx, body, ®ioncx, &borrow_set) - .into_engine(tcx, body, Blocks::All) + .into_engine(tcx, body) .pass_name("borrowck") .iterate_to_fixpoint(); let flow_uninits = MaybeUninitializedPlaces::new(tcx, body, &mdpe) - .into_engine(tcx, body, Blocks::All) + .into_engine(tcx, body) .pass_name("borrowck") .iterate_to_fixpoint(); let flow_ever_inits = EverInitializedPlaces::new(tcx, body, &mdpe) - .into_engine(tcx, body, Blocks::All) + .into_engine(tcx, body) .pass_name("borrowck") .iterate_to_fixpoint(); diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs index 049a0393632fb..a2624c7408cc4 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/check.rs @@ -11,7 +11,7 @@ use rustc_middle::mir::*; use rustc_middle::ty::subst::{GenericArgKind, InternalSubsts}; use rustc_middle::ty::{self, adjustment::PointerCast, Instance, InstanceDef, Ty, TyCtxt}; use rustc_middle::ty::{Binder, TraitRef, TypeVisitable}; -use rustc_mir_dataflow::{self, Analysis, Blocks}; +use rustc_mir_dataflow::{self, Analysis}; use rustc_span::{sym, Span, Symbol}; use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _; use rustc_trait_selection::traits::{self, ObligationCauseCode, ObligationCtxt, SelectionContext}; @@ -58,7 +58,7 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> { let ConstCx { tcx, body, .. } = *ccx; FlowSensitiveAnalysis::new(NeedsDrop, ccx) - .into_engine(tcx, &body, Blocks::All) + .into_engine(tcx, &body) .iterate_to_fixpoint() .into_results_cursor(&body) }); @@ -85,7 +85,7 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> { let ConstCx { tcx, body, .. } = *ccx; FlowSensitiveAnalysis::new(NeedsNonConstDrop, ccx) - .into_engine(tcx, &body, Blocks::All) + .into_engine(tcx, &body) .iterate_to_fixpoint() .into_results_cursor(&body) }); @@ -115,7 +115,7 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> { let ConstCx { tcx, body, .. } = *ccx; FlowSensitiveAnalysis::new(HasMutInterior, ccx) - .into_engine(tcx, &body, Blocks::All) + .into_engine(tcx, &body) .iterate_to_fixpoint() .into_results_cursor(&body) }); @@ -163,7 +163,7 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> { hir::ConstContext::Const | hir::ConstContext::Static(_) => { let mut cursor = FlowSensitiveAnalysis::new(CustomEq, ccx) - .into_engine(ccx.tcx, &ccx.body, Blocks::All) + .into_engine(ccx.tcx, &ccx.body) .iterate_to_fixpoint() .into_results_cursor(&ccx.body); diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index 71bda6d486aa2..87cea20ca3d91 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -15,7 +15,7 @@ use rustc_middle::mir::{ use rustc_middle::ty::{self, InstanceDef, ParamEnv, Ty, TyCtxt, TypeVisitable}; use rustc_mir_dataflow::impls::MaybeStorageLive; use rustc_mir_dataflow::storage::always_storage_live_locals; -use rustc_mir_dataflow::{Analysis, Blocks, ResultsCursor}; +use rustc_mir_dataflow::{Analysis, ResultsCursor}; use rustc_target::abi::{Size, VariantIdx}; #[derive(Copy, Clone, Debug)] @@ -53,7 +53,7 @@ impl<'tcx> MirPass<'tcx> for Validator { let always_live_locals = always_storage_live_locals(body); let storage_liveness = MaybeStorageLive::new(std::borrow::Cow::Owned(always_live_locals)) - .into_engine(tcx, body, Blocks::All) + .into_engine(tcx, body) .iterate_to_fixpoint() .into_results_cursor(body); diff --git a/compiler/rustc_mir_dataflow/src/framework/engine.rs b/compiler/rustc_mir_dataflow/src/framework/engine.rs index 64d3eadc0016c..5fe4c7a3031d0 100644 --- a/compiler/rustc_mir_dataflow/src/framework/engine.rs +++ b/compiler/rustc_mir_dataflow/src/framework/engine.rs @@ -71,13 +71,6 @@ where } } -pub enum Blocks { - /// Consider all MIR blocks - All, - /// Consider only the MIR blocks reachable from the start - Reachable, -} - /// A solver for dataflow problems. pub struct Engine<'a, 'tcx, A> where @@ -97,7 +90,6 @@ where // performance in practice. I've tried a few ways to avoid this, but they have downsides. See // the message for the commit that added this FIXME for more information. apply_trans_for_block: Option>, - blocks: Blocks, } impl<'a, 'tcx, A, D, T> Engine<'a, 'tcx, A> @@ -107,18 +99,13 @@ where T: Idx, { /// Creates a new `Engine` to solve a gen-kill dataflow problem. - pub fn new_gen_kill( - tcx: TyCtxt<'tcx>, - body: &'a mir::Body<'tcx>, - analysis: A, - blocks: Blocks, - ) -> Self { + pub fn new_gen_kill(tcx: TyCtxt<'tcx>, body: &'a mir::Body<'tcx>, analysis: A) -> Self { // If there are no back-edges in the control-flow graph, we only ever need to apply the // transfer function for each block exactly once (assuming that we process blocks in RPO). // // In this case, there's no need to compute the block transfer functions ahead of time. if !body.basic_blocks.is_cfg_cyclic() { - return Self::new(tcx, body, analysis, None, blocks); + return Self::new(tcx, body, analysis, None); } // Otherwise, compute and store the cumulative transfer function for each block. @@ -135,7 +122,7 @@ where trans_for_block[bb].apply(state); }); - Self::new(tcx, body, analysis, Some(apply_trans as Box<_>), blocks) + Self::new(tcx, body, analysis, Some(apply_trans as Box<_>)) } } @@ -149,13 +136,8 @@ where /// /// Gen-kill problems should use `new_gen_kill`, which will coalesce transfer functions for /// better performance. - pub fn new_generic( - tcx: TyCtxt<'tcx>, - body: &'a mir::Body<'tcx>, - analysis: A, - blocks: Blocks, - ) -> Self { - Self::new(tcx, body, analysis, None, blocks) + pub fn new_generic(tcx: TyCtxt<'tcx>, body: &'a mir::Body<'tcx>, analysis: A) -> Self { + Self::new(tcx, body, analysis, None) } fn new( @@ -163,7 +145,6 @@ where body: &'a mir::Body<'tcx>, analysis: A, apply_trans_for_block: Option>, - blocks: Blocks, ) -> Self { let bottom_value = analysis.bottom_value(body); let mut entry_sets = IndexVec::from_elem(bottom_value.clone(), &body.basic_blocks); @@ -181,7 +162,6 @@ where pass_name: None, entry_sets, apply_trans_for_block, - blocks, } } @@ -217,7 +197,6 @@ where tcx, apply_trans_for_block, pass_name, - blocks, .. } = self; @@ -226,23 +205,13 @@ where let mut visited = BitSet::new_empty(body.basic_blocks.len()); if A::Direction::IS_FORWARD { - match blocks { - Blocks::All => { - for (bb, _) in traversal::reverse_postorder(body) { - dirty_queue.insert(bb); - } - } - Blocks::Reachable => { - dirty_queue.insert(mir::START_BLOCK); - } - } + dirty_queue.insert(mir::START_BLOCK); } else { // Reverse post-order on the reverse CFG may generate a better iteration order for // backward dataflow analyses, but probably not enough to matter. for (bb, _) in traversal::postorder(body) { dirty_queue.insert(bb); } - assert!(matches!(blocks, Blocks::All)); } // `state` is not actually used between iterations; diff --git a/compiler/rustc_mir_dataflow/src/framework/mod.rs b/compiler/rustc_mir_dataflow/src/framework/mod.rs index daf164c5175f2..d9aff94fef2f9 100644 --- a/compiler/rustc_mir_dataflow/src/framework/mod.rs +++ b/compiler/rustc_mir_dataflow/src/framework/mod.rs @@ -47,7 +47,7 @@ mod visitor; pub use self::cursor::{ResultsCursor, ResultsRefCursor}; pub use self::direction::{Backward, Direction, Forward}; -pub use self::engine::{Blocks, Engine, Results}; +pub use self::engine::{Engine, Results}; pub use self::lattice::{JoinSemiLattice, MeetSemiLattice}; pub use self::visitor::{visit_results, ResultsVisitable, ResultsVisitor}; @@ -261,12 +261,11 @@ pub trait Analysis<'tcx>: AnalysisDomain<'tcx> { self, tcx: TyCtxt<'tcx>, body: &'mir mir::Body<'tcx>, - blocks: Blocks, ) -> Engine<'mir, 'tcx, Self> where Self: Sized, { - Engine::new_generic(tcx, body, self, blocks) + Engine::new_generic(tcx, body, self) } } @@ -420,12 +419,11 @@ where self, tcx: TyCtxt<'tcx>, body: &'mir mir::Body<'tcx>, - blocks: Blocks, ) -> Engine<'mir, 'tcx, Self> where Self: Sized, { - Engine::new_gen_kill(tcx, body, self, blocks) + Engine::new_gen_kill(tcx, body, self) } } diff --git a/compiler/rustc_mir_dataflow/src/lib.rs b/compiler/rustc_mir_dataflow/src/lib.rs index e8bec506392cf..7f40cfca32fff 100644 --- a/compiler/rustc_mir_dataflow/src/lib.rs +++ b/compiler/rustc_mir_dataflow/src/lib.rs @@ -25,10 +25,9 @@ pub use self::drop_flag_effects::{ on_lookup_result_bits, }; pub use self::framework::{ - fmt, graphviz, lattice, visit_results, Analysis, AnalysisDomain, Backward, Blocks, - CallReturnPlaces, Direction, Engine, Forward, GenKill, GenKillAnalysis, JoinSemiLattice, - Results, ResultsCursor, ResultsRefCursor, ResultsVisitable, ResultsVisitor, - SwitchIntEdgeEffects, + fmt, graphviz, lattice, visit_results, Analysis, AnalysisDomain, Backward, CallReturnPlaces, + Direction, Engine, Forward, GenKill, GenKillAnalysis, JoinSemiLattice, Results, ResultsCursor, + ResultsRefCursor, ResultsVisitable, ResultsVisitor, SwitchIntEdgeEffects, }; use self::move_paths::MoveData; diff --git a/compiler/rustc_mir_dataflow/src/rustc_peek.rs b/compiler/rustc_mir_dataflow/src/rustc_peek.rs index 55b4b8f395b35..7cae68efbecc3 100644 --- a/compiler/rustc_mir_dataflow/src/rustc_peek.rs +++ b/compiler/rustc_mir_dataflow/src/rustc_peek.rs @@ -10,7 +10,7 @@ use crate::errors::{ PeekArgumentNotALocal, PeekArgumentUntracked, PeekBitNotSet, PeekMustBeNotTemporary, PeekMustBePlaceOrRefPlace, StopAfterDataFlowEndedCompilation, }; -use crate::framework::{BitSetExt, Blocks}; +use crate::framework::BitSetExt; use crate::impls::{ DefinitelyInitializedPlaces, MaybeInitializedPlaces, MaybeLiveLocals, MaybeUninitializedPlaces, }; @@ -39,7 +39,7 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { if has_rustc_mir_with(tcx, def_id, sym::rustc_peek_maybe_init).is_some() { let flow_inits = MaybeInitializedPlaces::new(tcx, body, &mdpe) - .into_engine(tcx, body, Blocks::All) + .into_engine(tcx, body) .iterate_to_fixpoint(); sanity_check_via_rustc_peek(tcx, body, &flow_inits); @@ -47,7 +47,7 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { if has_rustc_mir_with(tcx, def_id, sym::rustc_peek_maybe_uninit).is_some() { let flow_uninits = MaybeUninitializedPlaces::new(tcx, body, &mdpe) - .into_engine(tcx, body, Blocks::All) + .into_engine(tcx, body) .iterate_to_fixpoint(); sanity_check_via_rustc_peek(tcx, body, &flow_uninits); @@ -55,15 +55,14 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { if has_rustc_mir_with(tcx, def_id, sym::rustc_peek_definite_init).is_some() { let flow_def_inits = DefinitelyInitializedPlaces::new(tcx, body, &mdpe) - .into_engine(tcx, body, Blocks::All) + .into_engine(tcx, body) .iterate_to_fixpoint(); sanity_check_via_rustc_peek(tcx, body, &flow_def_inits); } if has_rustc_mir_with(tcx, def_id, sym::rustc_peek_liveness).is_some() { - let flow_liveness = - MaybeLiveLocals.into_engine(tcx, body, Blocks::All).iterate_to_fixpoint(); + let flow_liveness = MaybeLiveLocals.into_engine(tcx, body).iterate_to_fixpoint(); sanity_check_via_rustc_peek(tcx, body, &flow_liveness); } diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index 34938082beede..c75fe2327de3e 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -9,9 +9,7 @@ use rustc_middle::mir::visit::{MutVisitor, Visitor}; use rustc_middle::mir::*; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_mir_dataflow::value_analysis::{Map, State, TrackElem, ValueAnalysis, ValueOrPlace}; -use rustc_mir_dataflow::{ - lattice::FlatSet, Analysis, Blocks, ResultsVisitor, SwitchIntEdgeEffects, -}; +use rustc_mir_dataflow::{lattice::FlatSet, Analysis, ResultsVisitor, SwitchIntEdgeEffects}; use rustc_span::DUMMY_SP; use rustc_target::abi::Align; @@ -55,7 +53,7 @@ impl<'tcx> MirPass<'tcx> for DataflowConstProp { // Perform the actual dataflow analysis. let analysis = ConstAnalysis::new(tcx, body, map); let results = debug_span!("analyze") - .in_scope(|| analysis.wrap().into_engine(tcx, body, Blocks::All).iterate_to_fixpoint()); + .in_scope(|| analysis.wrap().into_engine(tcx, body).iterate_to_fixpoint()); // Collect results and patch the body afterwards. let mut visitor = CollectAndPatch::new(tcx, &results.analysis.0.map); diff --git a/compiler/rustc_mir_transform/src/dead_store_elimination.rs b/compiler/rustc_mir_transform/src/dead_store_elimination.rs index af756d32f4335..42c580c63f060 100644 --- a/compiler/rustc_mir_transform/src/dead_store_elimination.rs +++ b/compiler/rustc_mir_transform/src/dead_store_elimination.rs @@ -16,7 +16,7 @@ use rustc_index::bit_set::BitSet; use rustc_middle::mir::*; use rustc_middle::ty::TyCtxt; use rustc_mir_dataflow::impls::{borrowed_locals, MaybeTransitiveLiveLocals}; -use rustc_mir_dataflow::{Analysis, Blocks}; +use rustc_mir_dataflow::Analysis; /// Performs the optimization on the body /// @@ -24,7 +24,7 @@ use rustc_mir_dataflow::{Analysis, Blocks}; /// can be generated via the [`borrowed_locals`] function. pub fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, borrowed: &BitSet) { let mut live = MaybeTransitiveLiveLocals::new(borrowed) - .into_engine(tcx, body, Blocks::All) + .into_engine(tcx, body) .iterate_to_fixpoint() .into_results_cursor(body); diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index 5b51dfaf4b6f8..3a87c4d17f2bd 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -141,7 +141,7 @@ use rustc_middle::mir::{ }; use rustc_middle::ty::TyCtxt; use rustc_mir_dataflow::impls::MaybeLiveLocals; -use rustc_mir_dataflow::{Analysis, Blocks, ResultsCursor}; +use rustc_mir_dataflow::{Analysis, ResultsCursor}; pub struct DestinationPropagation; @@ -189,7 +189,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation { ); trace!(?candidates); let mut live = MaybeLiveLocals - .into_engine(tcx, body, Blocks::All) + .into_engine(tcx, body) .iterate_to_fixpoint() .into_results_cursor(body); dest_prop_mir_dump(tcx, body, &mut live, round_count); diff --git a/compiler/rustc_mir_transform/src/elaborate_drops.rs b/compiler/rustc_mir_transform/src/elaborate_drops.rs index 701d1c6d0224d..2ed4e225304b2 100644 --- a/compiler/rustc_mir_transform/src/elaborate_drops.rs +++ b/compiler/rustc_mir_transform/src/elaborate_drops.rs @@ -13,7 +13,7 @@ use rustc_mir_dataflow::on_lookup_result_bits; use rustc_mir_dataflow::un_derefer::UnDerefer; use rustc_mir_dataflow::MoveDataParamEnv; use rustc_mir_dataflow::{on_all_children_bits, on_all_drop_children_bits}; -use rustc_mir_dataflow::{Analysis, Blocks, ResultsCursor}; +use rustc_mir_dataflow::{Analysis, ResultsCursor}; use rustc_span::Span; use rustc_target::abi::VariantIdx; use std::fmt; @@ -43,7 +43,7 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops { let dead_unwinds = find_dead_unwinds(tcx, body, &env, &un_derefer); let inits = MaybeInitializedPlaces::new(tcx, body, &env) - .into_engine(tcx, body, Blocks::Reachable) + .into_engine(tcx, body) .dead_unwinds(&dead_unwinds) .pass_name("elaborate_drops") .iterate_to_fixpoint() @@ -51,7 +51,7 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops { let uninits = MaybeUninitializedPlaces::new(tcx, body, &env) .mark_inactive_variants_as_uninit() - .into_engine(tcx, body, Blocks::Reachable) + .into_engine(tcx, body) .dead_unwinds(&dead_unwinds) .pass_name("elaborate_drops") .iterate_to_fixpoint() @@ -87,7 +87,7 @@ fn find_dead_unwinds<'tcx>( // reach cleanup blocks, which can't have unwind edges themselves. let mut dead_unwinds = BitSet::new_empty(body.basic_blocks.len()); let mut flow_inits = MaybeInitializedPlaces::new(tcx, body, &env) - .into_engine(tcx, body, Blocks::Reachable) + .into_engine(tcx, body) .pass_name("find_dead_unwinds") .iterate_to_fixpoint() .into_results_cursor(body); diff --git a/compiler/rustc_mir_transform/src/generator.rs b/compiler/rustc_mir_transform/src/generator.rs index 2c4790f748c21..67a1bc341b4eb 100644 --- a/compiler/rustc_mir_transform/src/generator.rs +++ b/compiler/rustc_mir_transform/src/generator.rs @@ -69,7 +69,7 @@ use rustc_mir_dataflow::impls::{ MaybeBorrowedLocals, MaybeLiveLocals, MaybeRequiresStorage, MaybeStorageLive, }; use rustc_mir_dataflow::storage::always_storage_live_locals; -use rustc_mir_dataflow::{self, Analysis, Blocks}; +use rustc_mir_dataflow::{self, Analysis}; use rustc_target::abi::VariantIdx; use rustc_target::spec::PanicStrategy; use std::{iter, ops}; @@ -491,16 +491,14 @@ fn locals_live_across_suspend_points<'tcx>( // Calculate when MIR locals have live storage. This gives us an upper bound of their // lifetimes. let mut storage_live = MaybeStorageLive::new(std::borrow::Cow::Borrowed(always_live_locals)) - .into_engine(tcx, body_ref, Blocks::All) + .into_engine(tcx, body_ref) .iterate_to_fixpoint() .into_results_cursor(body_ref); // Calculate the MIR locals which have been previously // borrowed (even if they are still active). - let borrowed_locals_results = MaybeBorrowedLocals - .into_engine(tcx, body_ref, Blocks::All) - .pass_name("generator") - .iterate_to_fixpoint(); + let borrowed_locals_results = + MaybeBorrowedLocals.into_engine(tcx, body_ref).pass_name("generator").iterate_to_fixpoint(); let mut borrowed_locals_cursor = rustc_mir_dataflow::ResultsCursor::new(body_ref, &borrowed_locals_results); @@ -508,14 +506,14 @@ fn locals_live_across_suspend_points<'tcx>( // Calculate the MIR locals that we actually need to keep storage around // for. let requires_storage_results = MaybeRequiresStorage::new(body, &borrowed_locals_results) - .into_engine(tcx, body_ref, Blocks::All) + .into_engine(tcx, body_ref) .iterate_to_fixpoint(); let mut requires_storage_cursor = rustc_mir_dataflow::ResultsCursor::new(body_ref, &requires_storage_results); // Calculate the liveness of MIR locals ignoring borrows. let mut liveness = MaybeLiveLocals - .into_engine(tcx, body_ref, Blocks::All) + .into_engine(tcx, body_ref) .pass_name("generator") .iterate_to_fixpoint() .into_results_cursor(body_ref); diff --git a/compiler/rustc_mir_transform/src/remove_uninit_drops.rs b/compiler/rustc_mir_transform/src/remove_uninit_drops.rs index f62e0380dccf1..a9096ba3aad74 100644 --- a/compiler/rustc_mir_transform/src/remove_uninit_drops.rs +++ b/compiler/rustc_mir_transform/src/remove_uninit_drops.rs @@ -4,7 +4,7 @@ use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt, VariantDef}; use rustc_mir_dataflow::impls::MaybeInitializedPlaces; use rustc_mir_dataflow::move_paths::{LookupResult, MoveData, MovePathIndex}; -use rustc_mir_dataflow::{self, move_path_children_matching, Analysis, Blocks, MoveDataParamEnv}; +use rustc_mir_dataflow::{self, move_path_children_matching, Analysis, MoveDataParamEnv}; use crate::MirPass; @@ -30,7 +30,7 @@ impl<'tcx> MirPass<'tcx> for RemoveUninitDrops { let mdpe = MoveDataParamEnv { move_data, param_env }; let mut maybe_inits = MaybeInitializedPlaces::new(tcx, body, &mdpe) - .into_engine(tcx, body, Blocks::All) + .into_engine(tcx, body) .pass_name("remove_uninit_drops") .iterate_to_fixpoint() .into_results_cursor(body); From 1245c0c9c26312a3daadcd1affd1a2bc547c6bed Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Sat, 31 Dec 2022 13:47:17 +0100 Subject: [PATCH 04/13] Skip borrowck access check for a drop in a replace op --- compiler/rustc_borrowck/src/invalidation.rs | 47 ++++----------- compiler/rustc_borrowck/src/lib.rs | 63 ++++++++++----------- 2 files changed, 39 insertions(+), 71 deletions(-) diff --git a/compiler/rustc_borrowck/src/invalidation.rs b/compiler/rustc_borrowck/src/invalidation.rs index 943d5fd845264..bfae6cb4deb5f 100644 --- a/compiler/rustc_borrowck/src/invalidation.rs +++ b/compiler/rustc_borrowck/src/invalidation.rs @@ -10,8 +10,8 @@ use rustc_middle::ty::TyCtxt; use crate::{ borrow_set::BorrowSet, facts::AllFacts, location::LocationTable, path_utils::*, AccessDepth, - Activation, ArtificialField, BorrowIndex, Deep, FxHashSet, LocalMutationIsAllowed, Read, - ReadKind, ReadOrWrite, Reservation, Shallow, Write, WriteKind, + Activation, ArtificialField, BorrowIndex, Deep, LocalMutationIsAllowed, Read, ReadKind, + ReadOrWrite, Reservation, Shallow, Write, WriteKind, }; pub(super) fn generate_invalidates<'tcx>( @@ -36,7 +36,6 @@ pub(super) fn generate_invalidates<'tcx>( location_table, body: &body, dominators, - to_skip: Default::default(), }; ig.visit_body(body); } @@ -49,7 +48,6 @@ struct InvalidationGenerator<'cx, 'tcx> { body: &'cx Body<'tcx>, dominators: Dominators, borrow_set: &'cx BorrowSet<'tcx>, - to_skip: FxHashSet, } /// Visits the whole MIR and generates `invalidates()` facts. @@ -62,9 +60,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { StatementKind::Assign(box (lhs, rhs)) => { self.consume_rvalue(location, rhs); - if !self.to_skip.contains(&location) { - self.mutate_place(location, *lhs, Shallow(None)); - } + self.mutate_place(location, *lhs, Shallow(None)); } StatementKind::FakeRead(box (_, _)) => { // Only relevant for initialized/liveness/safety checks. @@ -113,36 +109,13 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { TerminatorKind::SwitchInt { discr, targets: _ } => { self.consume_operand(location, discr); } - TerminatorKind::Drop { place: drop_place, target, unwind, is_replace } => { - let next_statement = if *is_replace { - self.body - .basic_blocks - .get(*target) - .expect("MIR should be complete at this point") - .statements - .first() - } else { - None - }; - - match next_statement { - Some(Statement { kind: StatementKind::Assign(_), source_info: _ }) => { - // this is a drop from a replace operation, for better diagnostic report - // here possible conflicts and mute the assign statement errors - self.to_skip.insert(Location { block: *target, statement_index: 0 }); - self.to_skip - .insert(Location { block: unwind.unwrap(), statement_index: 0 }); - self.mutate_place(location, *drop_place, AccessDepth::Deep); - } - _ => { - self.access_place( - location, - *drop_place, - (AccessDepth::Drop, Write(WriteKind::StorageDeadOrDrop)), - LocalMutationIsAllowed::Yes, - ); - } - } + TerminatorKind::Drop { place: drop_place, target: _, unwind: _, is_replace: _ } => { + self.access_place( + location, + *drop_place, + (AccessDepth::Drop, Write(WriteKind::StorageDeadOrDrop)), + LocalMutationIsAllowed::Yes, + ); } TerminatorKind::Call { func, diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index ef0a03a546fe1..8890d54373cca 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -340,7 +340,7 @@ fn do_mir_borrowck<'tcx>( next_region_name: RefCell::new(1), polonius_output: None, errors, - to_skip: Default::default(), + replaces: Default::default(), }; promoted_mbcx.report_move_errors(move_errors); errors = promoted_mbcx.errors; @@ -372,7 +372,7 @@ fn do_mir_borrowck<'tcx>( next_region_name: RefCell::new(1), polonius_output, errors, - to_skip: Default::default(), + replaces: Default::default(), }; // Compute and report region errors, if any. @@ -556,7 +556,9 @@ struct MirBorrowckCtxt<'cx, 'tcx> { errors: error::BorrowckErrors<'tcx>, - to_skip: FxHashSet, + /// Record the places were a replace happens so that we can use the + /// correct access depth in the assignment for better diagnostic + replaces: FxHashSet, } // Check that: @@ -581,9 +583,10 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx match &stmt.kind { StatementKind::Assign(box (lhs, rhs)) => { self.consume_rvalue(location, (rhs, span), flow_state); - if !self.to_skip.contains(&location) { - self.mutate_place(location, (*lhs, span), Shallow(None), flow_state); - } + // In case of a replace the drop access check is skipped for better diagnostic but we need + // to use a stricter access depth here + let access_depth = if self.replaces.contains(&location) {AccessDepth::Drop} else {Shallow(None)}; + self.mutate_place(location, (*lhs, span), access_depth, flow_state); } StatementKind::FakeRead(box (_, place)) => { // Read for match doesn't access any memory and is used to @@ -656,36 +659,28 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx loc, term, place, span ); - let next_statement = if *is_replace { - self.body() - .basic_blocks - .get(*target) - .expect("MIR should be complete at this point") - .statements - .first() - } else { - None - }; - - match next_statement { - Some(Statement { kind: StatementKind::Assign(_), source_info: _ }) => { - // this is a drop from a replace operation, for better diagnostic report - // here possible conflicts and mute the assign statement errors - self.to_skip.insert(Location { block: *target, statement_index: 0 }); - self.to_skip - .insert(Location { block: unwind.unwrap(), statement_index: 0 }); - self.mutate_place(loc, (*place, span), AccessDepth::Deep, flow_state); - } - _ => { - self.access_place( - loc, - (*place, span), - (AccessDepth::Drop, Write(WriteKind::StorageDeadOrDrop)), - LocalMutationIsAllowed::Yes, - flow_state, - ); + // In case of a replace, it's more user friendly to report a problem with the explicit + // assignment than the implicit drop. + // Simply skip this access and rely on the assignment to report any error. + if *is_replace { + // An assignment `x = ...` is usually a shallow access, but in the case of a replace + // the drop could access internal references depending on the drop implementation. + // Since we're skipping the drop access, we need to mark the access depth + // of the assignment as AccessDepth::Drop. + self.replaces.insert(Location { block: *target, statement_index: 0 }); + if let Some(unwind) = unwind { + self.replaces.insert(Location { block: *unwind, statement_index: 0 }); } + return; } + + self.access_place( + loc, + (*place, span), + (AccessDepth::Drop, Write(WriteKind::StorageDeadOrDrop)), + LocalMutationIsAllowed::Yes, + flow_state, + ) } TerminatorKind::Call { func, From 59dcd933d65b1bfde8b72b3ecb5d45e442cd5cf4 Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Sat, 31 Dec 2022 13:48:44 +0100 Subject: [PATCH 05/13] use correct span for assign in a replace op --- compiler/rustc_mir_build/src/build/expr/stmt.rs | 11 +++++++++-- compiler/rustc_mir_build/src/build/scope.rs | 5 +++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/expr/stmt.rs b/compiler/rustc_mir_build/src/build/expr/stmt.rs index 243911de4fb98..bb3e8283edfaa 100644 --- a/compiler/rustc_mir_build/src/build/expr/stmt.rs +++ b/compiler/rustc_mir_build/src/build/expr/stmt.rs @@ -39,12 +39,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Generate better code for things that don't need to be // dropped. - let needs_drop = lhs.ty.needs_drop(this.tcx, this.param_env); let rhs = unpack!(block = this.as_local_rvalue(block, rhs)); let lhs = unpack!(block = this.as_place(block, lhs)); if needs_drop { - unpack!(block = this.build_drop_and_replace(block, lhs_span, lhs, rhs.clone())); + unpack!( + block = this.build_drop_and_replace( + block, + source_info, + lhs_span, + lhs, + rhs.clone() + ) + ); } this.cfg.push_assign(block, source_info, lhs, rhs); this.block_context.pop(); diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index a32cbc0271107..365808c6c1e1f 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -1125,6 +1125,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { pub(crate) fn build_drop_and_replace( &mut self, block: BasicBlock, + statement_source_info: SourceInfo, span: Span, place: Place<'tcx>, value: Rvalue<'tcx>, @@ -1133,8 +1134,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let next_target = self.cfg.start_new_block(); let assign = self.cfg.start_new_cleanup_block(); - self.cfg.push_assign(assign, source_info, place, value.clone()); - self.cfg.terminate(assign, source_info, TerminatorKind::Goto { target: block }); + self.cfg.push_assign(assign, statement_source_info, place, value.clone()); + self.cfg.terminate(assign, statement_source_info, TerminatorKind::Goto { target: block }); self.cfg.terminate( block, From 1bffc9f9caf167906601ced1577714ce519fe1d7 Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Sun, 1 Jan 2023 18:33:24 +0100 Subject: [PATCH 06/13] clarify terminator in replace unwind --- compiler/rustc_mir_build/src/build/scope.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index 365808c6c1e1f..774d79ff7ad1e 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -1135,7 +1135,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let assign = self.cfg.start_new_cleanup_block(); self.cfg.push_assign(assign, statement_source_info, place, value.clone()); - self.cfg.terminate(assign, statement_source_info, TerminatorKind::Goto { target: block }); + // We still have to build the scope drops so we don't know which block will follow. + // This terminator will be overwritten once the unwind drop tree builder runs. + self.cfg.terminate(assign, statement_source_info, TerminatorKind::Unreachable); self.cfg.terminate( block, From f40842a1bd73248831bee1ff789bca3701b085ff Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Sun, 1 Jan 2023 21:07:06 +0100 Subject: [PATCH 07/13] uncomment drop access kind panic --- .../rustc_borrowck/src/diagnostics/conflict_errors.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index aecbade6378ae..50cd13a2ccc8a 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -2364,10 +2364,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ProjectionElem::Deref => match kind { StorageDeadOrDrop::LocalStorageDead | StorageDeadOrDrop::BoxedStorageDead => { - // assert!( - // place_ty.ty.is_box(), - // "Drop of value behind a reference or raw pointer" - // ); + assert!( + place_ty.ty.is_box(), + "Drop of value behind a reference or raw pointer" + ); StorageDeadOrDrop::BoxedStorageDead } StorageDeadOrDrop::Destructor(_) => kind, From 63a692a7d45fdcf6173dbe9c3a941c0f5d57b617 Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Mon, 2 Jan 2023 16:02:42 +0100 Subject: [PATCH 08/13] Add relevant comments in the code * document is_replace field in Drop terminator * clarify drop and replace of untracked value --- compiler/rustc_middle/src/mir/syntax.rs | 9 ++++++++- compiler/rustc_mir_transform/src/elaborate_drops.rs | 4 ++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index eb704a439c24a..8a1d0fe4a18a1 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -577,7 +577,14 @@ pub enum TerminatorKind<'tcx> { /// > The drop glue is executed if, among all statements executed within this `Body`, an assignment to /// > the place or one of its "parents" occurred more recently than a move out of it. This does not /// > consider indirect assignments. - Drop { place: Place<'tcx>, target: BasicBlock, unwind: Option, is_replace: bool }, + Drop { + place: Place<'tcx>, + target: BasicBlock, + unwind: Option, + /// This field is only used for better diagnostic and to check MIR drop invariants. + /// It has no effect on the semantics of the `Drop` terminator. + is_replace: bool, + }, /// Roughly speaking, evaluates the `func` operand and the arguments, and starts execution of /// the referred to function. The operand types must match the argument types of the function. diff --git a/compiler/rustc_mir_transform/src/elaborate_drops.rs b/compiler/rustc_mir_transform/src/elaborate_drops.rs index 2ed4e225304b2..5ccec8bc5f480 100644 --- a/compiler/rustc_mir_transform/src/elaborate_drops.rs +++ b/compiler/rustc_mir_transform/src/elaborate_drops.rs @@ -386,8 +386,8 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { &format!("drop of untracked value {:?}", bb), ); } - // If we cannot find it, it means it's a drop followed by a replace - // -> unconditional dro + // drop and replace behind a pointer/array/whatever. The location + // must be initialized. Elaborator { ctxt: self }.patch().patch_terminator( bb, TerminatorKind::Drop { From 13fdf4ca18a40803a7d39cead619c3ecc6a7c723 Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Tue, 10 Jan 2023 15:13:51 +0100 Subject: [PATCH 09/13] Add DesugaringKind::Replace Add DesugaringKind::Replace to emit better diagnostic messages. At the moment it is used for drop and replaces and only the spans that actually contain a `Drop` terminator are marked as such. As there are no explicit drop correspondents in the HIR, marking happens during MIR build. --- compiler/rustc_borrowck/src/lib.rs | 54 +++++++------------ .../rustc_mir_build/src/build/expr/stmt.rs | 28 ++++++---- compiler/rustc_mir_build/src/build/scope.rs | 9 ++-- compiler/rustc_span/src/hygiene.rs | 2 + 4 files changed, 43 insertions(+), 50 deletions(-) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 8890d54373cca..c71412cb44a42 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -35,7 +35,7 @@ use rustc_middle::mir::{InlineAsmOperand, Terminator, TerminatorKind}; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{self, CapturedPlace, ParamEnv, RegionVid, TyCtxt}; use rustc_session::lint::builtin::UNUSED_MUT; -use rustc_span::{Span, Symbol}; +use rustc_span::{DesugaringKind, Span, Symbol}; use either::Either; use smallvec::SmallVec; @@ -340,7 +340,6 @@ fn do_mir_borrowck<'tcx>( next_region_name: RefCell::new(1), polonius_output: None, errors, - replaces: Default::default(), }; promoted_mbcx.report_move_errors(move_errors); errors = promoted_mbcx.errors; @@ -372,7 +371,6 @@ fn do_mir_borrowck<'tcx>( next_region_name: RefCell::new(1), polonius_output, errors, - replaces: Default::default(), }; // Compute and report region errors, if any. @@ -555,10 +553,6 @@ struct MirBorrowckCtxt<'cx, 'tcx> { polonius_output: Option>, errors: error::BorrowckErrors<'tcx>, - - /// Record the places were a replace happens so that we can use the - /// correct access depth in the assignment for better diagnostic - replaces: FxHashSet, } // Check that: @@ -583,10 +577,8 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx match &stmt.kind { StatementKind::Assign(box (lhs, rhs)) => { self.consume_rvalue(location, (rhs, span), flow_state); - // In case of a replace the drop access check is skipped for better diagnostic but we need - // to use a stricter access depth here - let access_depth = if self.replaces.contains(&location) {AccessDepth::Drop} else {Shallow(None)}; - self.mutate_place(location, (*lhs, span), access_depth, flow_state); + + self.mutate_place(location, (*lhs, span), AccessDepth::Shallow(None), flow_state); } StatementKind::FakeRead(box (_, place)) => { // Read for match doesn't access any memory and is used to @@ -652,28 +644,13 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx TerminatorKind::SwitchInt { discr, targets: _ } => { self.consume_operand(loc, (discr, span), flow_state); } - TerminatorKind::Drop { place, target, unwind, is_replace } => { + TerminatorKind::Drop { place, target: _, unwind: _, is_replace: _ } => { debug!( "visit_terminator_drop \ loc: {:?} term: {:?} place: {:?} span: {:?}", loc, term, place, span ); - // In case of a replace, it's more user friendly to report a problem with the explicit - // assignment than the implicit drop. - // Simply skip this access and rely on the assignment to report any error. - if *is_replace { - // An assignment `x = ...` is usually a shallow access, but in the case of a replace - // the drop could access internal references depending on the drop implementation. - // Since we're skipping the drop access, we need to mark the access depth - // of the assignment as AccessDepth::Drop. - self.replaces.insert(Location { block: *target, statement_index: 0 }); - if let Some(unwind) = unwind { - self.replaces.insert(Location { block: *unwind, statement_index: 0 }); - } - return; - } - self.access_place( loc, (*place, span), @@ -1112,13 +1089,22 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { this.report_conflicting_borrow(location, place_span, bk, borrow); this.buffer_error(err); } - WriteKind::StorageDeadOrDrop => this - .report_borrowed_value_does_not_live_long_enough( - location, - borrow, - place_span, - Some(kind), - ), + WriteKind::StorageDeadOrDrop => { + if let Some(DesugaringKind::Replace) = place_span.1.desugaring_kind() { + // If this is a drop triggered by a reassignment, it's more user friendly + // to report a problem with the explicit assignment than the implicit drop. + this.report_illegal_mutation_of_borrowed( + location, place_span, borrow, + ) + } else { + this.report_borrowed_value_does_not_live_long_enough( + location, + borrow, + place_span, + Some(kind), + ) + } + } WriteKind::Mutate => { this.report_illegal_mutation_of_borrowed(location, place_span, borrow) } diff --git a/compiler/rustc_mir_build/src/build/expr/stmt.rs b/compiler/rustc_mir_build/src/build/expr/stmt.rs index bb3e8283edfaa..9a3eacdf91943 100644 --- a/compiler/rustc_mir_build/src/build/expr/stmt.rs +++ b/compiler/rustc_mir_build/src/build/expr/stmt.rs @@ -3,6 +3,7 @@ use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder}; use rustc_middle::middle::region; use rustc_middle::mir::*; use rustc_middle::thir::*; +use rustc_span::DesugaringKind; impl<'a, 'tcx> Builder<'a, 'tcx> { /// Builds a block of MIR statements to evaluate the THIR `expr`. @@ -28,7 +29,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ExprKind::Assign { lhs, rhs } => { let lhs = &this.thir[lhs]; let rhs = &this.thir[rhs]; - let lhs_span = lhs.span; // Note: we evaluate assignments right-to-left. This // is better for borrowck interaction with overloaded @@ -42,17 +42,25 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let needs_drop = lhs.ty.needs_drop(this.tcx, this.param_env); let rhs = unpack!(block = this.as_local_rvalue(block, rhs)); let lhs = unpack!(block = this.as_place(block, lhs)); - if needs_drop { - unpack!( - block = this.build_drop_and_replace( - block, - source_info, - lhs_span, - lhs, - rhs.clone() + + let source_info = if needs_drop { + let span = this.tcx.with_stable_hashing_context(|hcx| { + expr_span.mark_with_reason( + None, + DesugaringKind::Replace, + this.tcx.sess.edition(), + hcx, ) + }); + let source_info = this.source_info(span); + unpack!( + block = this.build_drop_and_replace(block, source_info, lhs, rhs.clone()) ); - } + source_info + } else { + source_info + }; + this.cfg.push_assign(block, source_info, lhs, rhs); this.block_context.pop(); block.unit() diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index 774d79ff7ad1e..d76c520da1aab 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -1125,20 +1125,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { pub(crate) fn build_drop_and_replace( &mut self, block: BasicBlock, - statement_source_info: SourceInfo, - span: Span, + source_info: SourceInfo, place: Place<'tcx>, value: Rvalue<'tcx>, ) -> BlockAnd<()> { - let source_info = self.source_info(span); let next_target = self.cfg.start_new_block(); let assign = self.cfg.start_new_cleanup_block(); - self.cfg.push_assign(assign, statement_source_info, place, value.clone()); + self.cfg.push_assign(assign, source_info, place, value.clone()); // We still have to build the scope drops so we don't know which block will follow. // This terminator will be overwritten once the unwind drop tree builder runs. - self.cfg.terminate(assign, statement_source_info, TerminatorKind::Unreachable); - + self.cfg.terminate(assign, source_info, TerminatorKind::Unreachable); self.cfg.terminate( block, source_info, diff --git a/compiler/rustc_span/src/hygiene.rs b/compiler/rustc_span/src/hygiene.rs index 4e70dfb614782..7b20c6a926b0f 100644 --- a/compiler/rustc_span/src/hygiene.rs +++ b/compiler/rustc_span/src/hygiene.rs @@ -1151,6 +1151,7 @@ pub enum DesugaringKind { Await, ForLoop, WhileLoop, + Replace, } impl DesugaringKind { @@ -1166,6 +1167,7 @@ impl DesugaringKind { DesugaringKind::OpaqueTy => "`impl Trait`", DesugaringKind::ForLoop => "`for` loop", DesugaringKind::WhileLoop => "`while` loop", + DesugaringKind::Replace => "drop and replace", } } } From 1c1e795ba6b6589758fec4b6ae87be7a3717e935 Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Tue, 10 Jan 2023 16:15:58 +0100 Subject: [PATCH 10/13] remove is_replace field --- .../rustc_borrowck/src/diagnostics/mod.rs | 2 +- compiler/rustc_borrowck/src/invalidation.rs | 2 +- compiler/rustc_borrowck/src/lib.rs | 4 ++-- compiler/rustc_codegen_cranelift/src/base.rs | 2 +- compiler/rustc_codegen_ssa/src/mir/block.rs | 2 +- .../src/interpret/terminator.rs | 2 +- compiler/rustc_middle/src/mir/syntax.rs | 9 +------- compiler/rustc_middle/src/mir/visit.rs | 1 - .../src/build/custom/parse/instruction.rs | 1 - .../src/build/expr/as_rvalue.rs | 7 +----- compiler/rustc_mir_build/src/build/scope.rs | 15 ++----------- .../rustc_mir_dataflow/src/elaborate_drops.rs | 16 +++----------- .../src/add_moves_for_packed_drops.rs | 9 ++------ .../src/elaborate_drops.rs | 12 +++------- compiler/rustc_mir_transform/src/generator.rs | 18 +++++---------- compiler/rustc_mir_transform/src/shim.rs | 22 +++---------------- 16 files changed, 27 insertions(+), 97 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index d1fc66234b3fb..6ed9df6b75ae5 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -941,7 +941,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // drop and replace might have moved the assignment to the next block let maybe_additional_statement = if let Some(Terminator { - kind: TerminatorKind::Drop { target: drop_target, is_replace: true, .. }, + kind: TerminatorKind::Drop { target: drop_target, .. }, .. }) = self.body[location.block].terminator { diff --git a/compiler/rustc_borrowck/src/invalidation.rs b/compiler/rustc_borrowck/src/invalidation.rs index bfae6cb4deb5f..a7a8cfbc89869 100644 --- a/compiler/rustc_borrowck/src/invalidation.rs +++ b/compiler/rustc_borrowck/src/invalidation.rs @@ -109,7 +109,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { TerminatorKind::SwitchInt { discr, targets: _ } => { self.consume_operand(location, discr); } - TerminatorKind::Drop { place: drop_place, target: _, unwind: _, is_replace: _ } => { + TerminatorKind::Drop { place: drop_place, target: _, unwind: _ } => { self.access_place( location, *drop_place, diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index c71412cb44a42..7c8cd126e749b 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -578,7 +578,7 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx StatementKind::Assign(box (lhs, rhs)) => { self.consume_rvalue(location, (rhs, span), flow_state); - self.mutate_place(location, (*lhs, span), AccessDepth::Shallow(None), flow_state); + self.mutate_place(location, (*lhs, span), Shallow(None), flow_state); } StatementKind::FakeRead(box (_, place)) => { // Read for match doesn't access any memory and is used to @@ -644,7 +644,7 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx TerminatorKind::SwitchInt { discr, targets: _ } => { self.consume_operand(loc, (discr, span), flow_state); } - TerminatorKind::Drop { place, target: _, unwind: _, is_replace: _ } => { + TerminatorKind::Drop { place, target: _, unwind: _ } => { debug!( "visit_terminator_drop \ loc: {:?} term: {:?} place: {:?} span: {:?}", diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index 7a583684f832c..af74fccd354df 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -477,7 +477,7 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) { | TerminatorKind::GeneratorDrop => { bug!("shouldn't exist at codegen {:?}", bb_data.terminator()); } - TerminatorKind::Drop { place, target, unwind: _, is_replace: _ } => { + TerminatorKind::Drop { place, target, unwind: _ } => { let drop_place = codegen_place(fx, *place); crate::abi::codegen_drop(fx, source_info, drop_place); diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 7d3e8768bcc0f..d25812fd87f57 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -1305,7 +1305,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { MergingSucc::False } - mir::TerminatorKind::Drop { place, target, unwind, is_replace: _ } => { + mir::TerminatorKind::Drop { place, target, unwind } => { self.codegen_drop_terminator(helper, bx, place, target, unwind, mergeable_succ()) } diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index ba84d06ddaf86..63764926a7524 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -118,7 +118,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } - Drop { place, target, unwind, is_replace: _ } => { + Drop { place, target, unwind } => { let frame = self.frame(); let ty = place.ty(&frame.body.local_decls, *self.tcx).ty; let ty = self.subst_from_frame_and_normalize_erasing_regions(frame, ty)?; diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 8a1d0fe4a18a1..c6bd0eaf58e40 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -577,14 +577,7 @@ pub enum TerminatorKind<'tcx> { /// > The drop glue is executed if, among all statements executed within this `Body`, an assignment to /// > the place or one of its "parents" occurred more recently than a move out of it. This does not /// > consider indirect assignments. - Drop { - place: Place<'tcx>, - target: BasicBlock, - unwind: Option, - /// This field is only used for better diagnostic and to check MIR drop invariants. - /// It has no effect on the semantics of the `Drop` terminator. - is_replace: bool, - }, + Drop { place: Place<'tcx>, target: BasicBlock, unwind: Option }, /// Roughly speaking, evaluates the `func` operand and the arguments, and starts execution of /// the referred to function. The operand types must match the argument types of the function. diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index cf1d1da53abdf..b5e1dd8884dd7 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -486,7 +486,6 @@ macro_rules! make_mir_visitor { place, target: _, unwind: _, - is_replace: _, } => { self.visit_place( place, diff --git a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs index 1c13714d090d6..f27dd39c100c8 100644 --- a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs +++ b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs @@ -47,7 +47,6 @@ impl<'tcx, 'body> ParseCtxt<'tcx, 'body> { place: self.parse_place(args[0])?, target: self.parse_block(args[1])?, unwind: None, - is_replace: false }) }, @call("mir_call", args) => { 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 d02ccf03b82fc..c7b3eb44dc5fb 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -624,12 +624,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.cfg.terminate( block, outer_source_info, - TerminatorKind::Drop { - place: to_drop, - target: success, - unwind: None, - is_replace: false, - }, + TerminatorKind::Drop { place: to_drop, target: success, unwind: None }, ); this.diverge_from(block); block = success; diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index d76c520da1aab..25689b9527f42 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -371,7 +371,6 @@ impl DropTree { // The caller will handle this if needed. unwind: None, place: drop_data.0.local.into(), - is_replace: false, }; cfg.terminate(block, drop_data.0.source_info, terminator); } @@ -1139,12 +1138,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.cfg.terminate( block, source_info, - TerminatorKind::Drop { - place, - target: next_target, - unwind: Some(assign), - is_replace: true, - }, + TerminatorKind::Drop { place, target: next_target, unwind: Some(assign) }, ); self.diverge_from(block); @@ -1248,12 +1242,7 @@ fn build_scope_drops<'tcx>( cfg.terminate( block, source_info, - TerminatorKind::Drop { - place: local.into(), - target: next, - unwind: None, - is_replace: false, - }, + TerminatorKind::Drop { place: local.into(), target: next, unwind: None }, ); block = next; } diff --git a/compiler/rustc_mir_dataflow/src/elaborate_drops.rs b/compiler/rustc_mir_dataflow/src/elaborate_drops.rs index c844fa5541159..7836ae2e7b76f 100644 --- a/compiler/rustc_mir_dataflow/src/elaborate_drops.rs +++ b/compiler/rustc_mir_dataflow/src/elaborate_drops.rs @@ -164,7 +164,6 @@ where path: D::Path, succ: BasicBlock, unwind: Unwind, - is_replace: bool, } /// "Elaborates" a drop of `place`/`path` and patches `bb`'s terminator to execute it. @@ -183,12 +182,11 @@ pub fn elaborate_drop<'b, 'tcx, D>( succ: BasicBlock, unwind: Unwind, bb: BasicBlock, - is_replace: bool, ) where D: DropElaborator<'b, 'tcx>, 'tcx: 'b, { - DropCtxt { elaborator, source_info, place, path, succ, unwind, is_replace }.elaborate_drop(bb) + DropCtxt { elaborator, source_info, place, path, succ, unwind }.elaborate_drop(bb) } impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D> @@ -239,7 +237,6 @@ where place: self.place, target: self.succ, unwind: self.unwind.into_option(), - is_replace: self.is_replace, }, ); } @@ -301,7 +298,6 @@ where place, succ, unwind, - is_replace: self.is_replace, } .elaborated_drop_block() } else { @@ -316,7 +312,6 @@ where // Using `self.path` here to condition the drop on // our own drop flag. path: self.path, - is_replace: self.is_replace, } .complete_drop(succ, unwind) } @@ -735,7 +730,6 @@ where place: tcx.mk_place_deref(ptr), target: loop_block, unwind: unwind.into_option(), - is_replace: self.is_replace, }, ); @@ -997,12 +991,8 @@ where } fn drop_block(&mut self, target: BasicBlock, unwind: Unwind) -> BasicBlock { - let block = TerminatorKind::Drop { - place: self.place, - target, - unwind: unwind.into_option(), - is_replace: self.is_replace, - }; + let block = + TerminatorKind::Drop { place: self.place, target, unwind: unwind.into_option() }; self.new_block(unwind, block) } diff --git a/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs b/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs index 69198719ee2fb..896fcd9cdd608 100644 --- a/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs +++ b/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs @@ -80,7 +80,7 @@ fn add_move_for_packed_drop<'tcx>( is_cleanup: bool, ) { debug!("add_move_for_packed_drop({:?} @ {:?})", terminator, loc); - let TerminatorKind::Drop { ref place, target, unwind, is_replace: _ } = terminator.kind else { + let TerminatorKind::Drop { ref place, target, unwind } = terminator.kind else { unreachable!(); }; @@ -98,11 +98,6 @@ fn add_move_for_packed_drop<'tcx>( patch.add_assign(loc, Place::from(temp), Rvalue::Use(Operand::Move(*place))); patch.patch_terminator( loc.block, - TerminatorKind::Drop { - place: Place::from(temp), - target: storage_dead_block, - unwind, - is_replace: false, - }, + TerminatorKind::Drop { place: Place::from(temp), target: storage_dead_block, unwind }, ); } diff --git a/compiler/rustc_mir_transform/src/elaborate_drops.rs b/compiler/rustc_mir_transform/src/elaborate_drops.rs index 5ccec8bc5f480..3d2719629ff64 100644 --- a/compiler/rustc_mir_transform/src/elaborate_drops.rs +++ b/compiler/rustc_mir_transform/src/elaborate_drops.rs @@ -358,7 +358,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { let resume_block = self.patch.resume_block(); match terminator.kind { - TerminatorKind::Drop { mut place, target, unwind, is_replace } => { + TerminatorKind::Drop { mut place, target, unwind } => { if let Some(new_place) = self.un_derefer.derefer(place.as_ref(), self.body) { place = new_place; } @@ -377,17 +377,12 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { Unwind::To(Option::unwrap_or(unwind, resume_block)) }, bb, - is_replace, ), LookupResult::Parent(..) => { - if !is_replace { - self.tcx.sess.delay_span_bug( - terminator.source_info.span, - &format!("drop of untracked value {:?}", bb), - ); - } // drop and replace behind a pointer/array/whatever. The location // must be initialized. + // FIXME: should we check that it's actually the case and we did + // not wrongly emit a drop terminator somewhere / missed move data? Elaborator { ctxt: self }.patch().patch_terminator( bb, TerminatorKind::Drop { @@ -398,7 +393,6 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { } else { Some(Option::unwrap_or(unwind, resume_block)) }, - is_replace, }, ); } diff --git a/compiler/rustc_mir_transform/src/generator.rs b/compiler/rustc_mir_transform/src/generator.rs index 67a1bc341b4eb..87b298de72f8d 100644 --- a/compiler/rustc_mir_transform/src/generator.rs +++ b/compiler/rustc_mir_transform/src/generator.rs @@ -911,14 +911,11 @@ fn elaborate_generator_drops<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { let mut elaborator = DropShimElaborator { body, patch: MirPatch::new(body), tcx, param_env }; for (block, block_data) in body.basic_blocks.iter_enumerated() { - let (target, unwind, source_info, is_replace) = match block_data.terminator() { - Terminator { - source_info, - kind: TerminatorKind::Drop { place, target, unwind, is_replace }, - } => { + let (target, unwind, source_info) = match block_data.terminator() { + Terminator { source_info, kind: TerminatorKind::Drop { place, target, unwind } } => { if let Some(local) = place.as_local() { if local == SELF_ARG { - (target, unwind, source_info, is_replace) + (target, unwind, source_info) } else { continue; } @@ -941,7 +938,6 @@ fn elaborate_generator_drops<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { *target, unwind, block, - *is_replace, ); } elaborator.patch.apply(body); @@ -1166,12 +1162,8 @@ fn create_generator_resume_function<'tcx>( fn insert_clean_drop(body: &mut Body<'_>) -> BasicBlock { let return_block = insert_term_block(body, TerminatorKind::Return); - let term = TerminatorKind::Drop { - place: Place::from(SELF_ARG), - target: return_block, - unwind: None, - is_replace: false, - }; + let term = + TerminatorKind::Drop { place: Place::from(SELF_ARG), target: return_block, unwind: None }; let source_info = SourceInfo::outermost(body.span); // Create a block to destroy an unresumed generators. This can only destroy upvars. diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index 6b953e593ceb2..dace540fa29d2 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -218,7 +218,6 @@ fn build_drop_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, ty: Option>) return_block, elaborate_drops::Unwind::To(resume_block), START_BLOCK, - false, ); elaborator.patch }; @@ -512,12 +511,7 @@ impl<'tcx> CloneShimBuilder<'tcx> { self.make_clone_call(dest_field, src_field, ity, next_block, unwind); self.block( vec![], - TerminatorKind::Drop { - place: dest_field, - target: unwind, - unwind: None, - is_replace: false, - }, + TerminatorKind::Drop { place: dest_field, target: unwind, unwind: None }, true, ); unwind = next_unwind; @@ -769,12 +763,7 @@ fn build_call_shim<'tcx>( block( &mut blocks, vec![], - TerminatorKind::Drop { - place: rcvr_place(), - target: BasicBlock::new(2), - unwind: None, - is_replace: false, - }, + TerminatorKind::Drop { place: rcvr_place(), target: BasicBlock::new(2), unwind: None }, false, ); } @@ -785,12 +774,7 @@ fn build_call_shim<'tcx>( block( &mut blocks, vec![], - TerminatorKind::Drop { - place: rcvr_place(), - target: BasicBlock::new(4), - unwind: None, - is_replace: false, - }, + TerminatorKind::Drop { place: rcvr_place(), target: BasicBlock::new(4), unwind: None }, true, ); From 25cf2b648fb04f6d2bf82560a86f3a27bd6c4e68 Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Tue, 10 Jan 2023 17:20:25 +0100 Subject: [PATCH 11/13] remove resolved fixmes --- compiler/rustc_borrowck/src/used_muts.rs | 4 ---- .../src/transform/check_consts/resolver.rs | 13 ------------- compiler/rustc_const_eval/src/transform/validate.rs | 13 ------------- 3 files changed, 30 deletions(-) diff --git a/compiler/rustc_borrowck/src/used_muts.rs b/compiler/rustc_borrowck/src/used_muts.rs index 86c784da3a02f..10ade142b24dd 100644 --- a/compiler/rustc_borrowck/src/used_muts.rs +++ b/compiler/rustc_borrowck/src/used_muts.rs @@ -71,10 +71,6 @@ impl<'visit, 'cx, 'tcx> Visitor<'tcx> for GatherUsedMutsVisitor<'visit, 'cx, 'tc TerminatorKind::Call { destination, .. } => { self.remove_never_initialized_mut_locals(*destination); } - // FIXME: ?? - // TerminatorKind::DropAndReplace { place, .. } => { - // self.remove_never_initialized_mut_locals(*place); - // } _ => {} } diff --git a/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs b/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs index beb224d150bdf..148aff9be4b5b 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs @@ -222,19 +222,6 @@ where // The effect of assignment to the return place in `TerminatorKind::Call` is not applied // here; that occurs in `apply_call_return_effect`. - // FIXME ?? - // if let mir::TerminatorKind::DropAndReplace { value, place, .. } = &terminator.kind { - // let qualif = qualifs::in_operand::( - // self.ccx, - // &mut |l| self.state.qualif.contains(l), - // value, - // ); - - // if !place.is_indirect() { - // self.assign_qualif_direct(place, qualif); - // } - // } - // We ignore borrow on drop because custom drop impls are not allowed in consts. // FIXME: Reconsider if accounting for borrows in drops is necessary for const drop. diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index 87cea20ca3d91..0f0b663e91d2a 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -736,19 +736,6 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { self.check_edge(location, *unwind, EdgeKind::Unwind); } } - // FIXME ?? - // TerminatorKind::DropAndReplace { target, unwind, .. } => { - // if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { - // self.fail( - // location, - // "`DropAndReplace` should have been removed during drop elaboration", - // ); - // } - // self.check_edge(location, *target, EdgeKind::Normal); - // if let Some(unwind) = unwind { - // self.check_edge(location, *unwind, EdgeKind::Unwind); - // } - // } TerminatorKind::Call { func, args, destination, target, cleanup, .. } => { let func_ty = func.ty(&self.body.local_decls, self.tcx); match func_ty.kind() { From a473afe36a05f1c6de14168810196165a60e4a67 Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Wed, 11 Jan 2023 22:01:27 +0100 Subject: [PATCH 12/13] Fix map index suggestion DesugaringKind::Replace is added at MIR build and does not show up in the HIR. Remove the desugaring from the span when walking down the HIR in diagnostic so that the span from the HIR correctly matches the span from MIR. --- .../rustc_borrowck/src/diagnostics/mutability_errors.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 6f6d1b01bd429..b1bed65ab2054 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -688,6 +688,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { return; } }; + if let hir::ExprKind::Assign(place, rv, _sp) = expr.kind && let hir::ExprKind::Index(val, index) = place.kind && (expr.span == self.assign_span || place.span == self.assign_span) @@ -772,7 +773,13 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { let Some(hir::Node::Item(item)) = node else { return; }; let hir::ItemKind::Fn(.., body_id) = item.kind else { return; }; let body = self.infcx.tcx.hir().body(body_id); - let mut v = V { assign_span: span, err, ty, suggested: false }; + let mut assign_span = span; + // Drop desugaring is done at MIR build so it's not in the HIR + if let Some(DesugaringKind::Replace) = span.desugaring_kind() { + assign_span.remove_mark(); + } + + let mut v = V { assign_span, err, ty, suggested: false }; v.visit_body(body); if !v.suggested { err.help(&format!( From 110b922eb331a606fe2c51ef3433de0d4f6ab233 Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Thu, 12 Jan 2023 00:15:21 +0100 Subject: [PATCH 13/13] Postpone unwind block termination for replaces --- compiler/rustc_mir_build/src/build/scope.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index 25689b9527f42..29809cd13b1b9 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -1132,9 +1132,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let assign = self.cfg.start_new_cleanup_block(); self.cfg.push_assign(assign, source_info, place, value.clone()); - // We still have to build the scope drops so we don't know which block will follow. - // This terminator will be overwritten once the unwind drop tree builder runs. - self.cfg.terminate(assign, source_info, TerminatorKind::Unreachable); self.cfg.terminate( block, source_info, @@ -1423,8 +1420,8 @@ impl<'tcx> DropTreeBuilder<'tcx> for Unwind { match &mut term.kind { TerminatorKind::Drop { unwind, .. } => { if let Some(unwind) = unwind.clone() { - cfg.block_data_mut(unwind).terminator_mut().kind = - TerminatorKind::Goto { target: to }; + let source_info = term.source_info; + cfg.terminate(unwind, source_info, TerminatorKind::Goto { target: to }); } else { *unwind = Some(to); }