Skip to content

Commit

Permalink
Remove DropAndReplace terminator
Browse files Browse the repository at this point in the history
  • Loading branch information
zeegomo committed Nov 16, 2022
1 parent 63c748e commit aec002f
Show file tree
Hide file tree
Showing 51 changed files with 328 additions and 404 deletions.
8 changes: 4 additions & 4 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2200,10 +2200,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,
Expand Down
17 changes: 16 additions & 1 deletion compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -937,7 +937,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(ref kind, ref places))) =
stmt.kind
{
Expand Down
57 changes: 37 additions & 20 deletions compiler/rustc_borrowck/src/invalidation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand All @@ -36,6 +36,7 @@ pub(super) fn generate_invalidates<'tcx>(
location_table,
body: &body,
dominators,
to_skip: Default::default(),
};
ig.visit_body(body);
}
Expand All @@ -48,6 +49,7 @@ struct InvalidationGenerator<'cx, 'tcx> {
body: &'cx Body<'tcx>,
dominators: Dominators<BasicBlock>,
borrow_set: &'cx BorrowSet<'tcx>,
to_skip: FxHashSet<Location>,
}

/// Visits the whole MIR and generates `invalidates()` facts.
Expand All @@ -59,8 +61,9 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> {
match &statement.kind {
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.
Expand Down Expand Up @@ -109,22 +112,36 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> {
TerminatorKind::SwitchInt { ref discr, switch_ty: _, 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: ref 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 {
ref func,
Expand Down
61 changes: 40 additions & 21 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,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;
Expand Down Expand Up @@ -374,6 +375,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.
Expand Down Expand Up @@ -556,6 +558,8 @@ struct MirBorrowckCtxt<'cx, 'tcx> {
polonius_output: Option<Rc<PoloniusOutput>>,

errors: error::BorrowckErrors<'tcx>,

to_skip: FxHashSet<Location>,
}

// Check that:
Expand All @@ -580,8 +584,9 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx
match &stmt.kind {
StatementKind::Assign(box (lhs, ref 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 (_, ref place)) => {
// Read for match doesn't access any memory and is used to
Expand Down Expand Up @@ -647,29 +652,43 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx
TerminatorKind::SwitchInt { ref discr, switch_ty: _, 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: ref 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 {
ref func,
Expand Down Expand Up @@ -785,7 +804,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 { .. }
Expand Down Expand Up @@ -1627,7 +1645,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).
//
Expand Down
20 changes: 0 additions & 20 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1348,25 +1348,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
| TerminatorKind::InlineAsm { .. } => {
// no checks needed for these
}

TerminatorKind::DropAndReplace { ref place, ref 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 { ref discr, switch_ty, .. } => {
self.check_operand(discr, term_location);

Expand Down Expand Up @@ -1665,7 +1646,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 {
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_borrowck/src/used_muts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
// }
_ => {}
}

Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_codegen_cranelift/src/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,8 +541,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!(),
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_codegen_ssa/src/mir/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,6 @@ pub fn cleanup_kinds(mir: &mir::Body<'_>) -> IndexVec<mir::BasicBlock, CleanupKi
TerminatorKind::Call { cleanup: unwind, .. }
| TerminatorKind::InlineAsm { cleanup: unwind, .. }
| TerminatorKind::Assert { cleanup: unwind, .. }
| TerminatorKind::DropAndReplace { unwind, .. }
| TerminatorKind::Drop { unwind, .. } => {
if let Some(unwind) = unwind {
debug!(
Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1207,7 +1207,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bx.unreachable();
}

mir::TerminatorKind::Drop { place, target, unwind } => {
mir::TerminatorKind::Drop { place, target, unwind, is_replace: _ } => {
self.codegen_drop_terminator(helper, bx, place, target, unwind);
}

Expand All @@ -1217,10 +1217,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
);
}

mir::TerminatorKind::DropAndReplace { .. } => {
bug!("undesugared DropAndReplace in codegen: {:?}", terminator);
}

mir::TerminatorKind::Call {
ref func,
ref args,
Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_const_eval/src/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
}

Drop { place, target, unwind } => {
Drop { place, target, unwind, is_replace: _ } => {
let place = self.eval_place(place)?;
let ty = place.layout.ty;
trace!("TerminatorKind::drop: {:?}, type {}", place, ty);
Expand Down Expand Up @@ -156,11 +156,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -965,8 +965,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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit aec002f

Please sign in to comment.