Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First attempt at removing DropAndReplace #104488

Closed
wants to merge 13 commits into from
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 @@ -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, .. },
..
}) = 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),
Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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!(
Expand Down
9 changes: 0 additions & 9 deletions compiler/rustc_borrowck/src/invalidation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,6 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> {
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::Call {
func,
args,
Expand Down
40 changes: 20 additions & 20 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -657,16 +657,7 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx
(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);
)
}
TerminatorKind::Call {
func,
Expand Down Expand Up @@ -782,7 +773,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 @@ -1099,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)
}
Expand Down Expand Up @@ -1592,7 +1591,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 @@ -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);

Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 0 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,6 @@ 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);
}
_ => {}
}

Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,6 @@ 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());
}
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 @@ -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!(),
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
4 changes: 0 additions & 4 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_const_eval/src/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
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
12 changes: 0 additions & 12 deletions compiler/rustc_const_eval/src/transform/check_consts/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,18 +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`.

if let mir::TerminatorKind::DropAndReplace { value, place, .. } = &terminator.kind {
let qualif = qualifs::in_operand::<Q, _>(
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.

Expand Down
12 changes: 0 additions & 12 deletions compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -736,18 +736,6 @@ 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);
}
}
TerminatorKind::Call { func, args, destination, target, cleanup, .. } => {
let func_ty = func.ty(&self.body.local_decls, self.tcx);
match func_ty.kind() {
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/mir/spanview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
38 changes: 0 additions & 38 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -580,43 +579,6 @@ pub enum TerminatorKind<'tcx> {
/// > consider indirect assignments.
Drop { place: Place<'tcx>, target: BasicBlock, unwind: Option<BasicBlock> },

/// 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<BasicBlock>,
},

/// 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.
/// The return place type must match the return type. The type of the `func` operand must be
Expand Down
Loading