Rollup merge of #107271 - Zeegomo:drop-rmw, r=oli-obk
Treat Drop as a rmw operation Previously, a Drop terminator was considered a move in MIR. This commit changes the behavior to only treat Drop as a mutable access to the dropped place. In order for this change to be correct, we need to guarantee that 1. A dropped value won't be used again 2. Places that appear in a drop won't be used again before a subsequent initialization. We can ensure this to be correct at MIR construction because Drop will only be emitted when a variable goes out of scope, thus having: * (1) as there is no way of reaching the old value. drop-elaboration will also remove any uninitialized drop. * (2) as the place can't be named following the end of the scope. However, the initialization status, previously tracked by moves, should also be tied to the execution of a Drop, hence the additional logic in the dataflow analyses. From discussion in [this thread](https://rust-lang.zulipchat.com/#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/.60DROP.60.20to.20.60DROP_IF.60.20compiler-team.23558), originating from https://github.com/rust-lang/compiler-team/issues/558. See also https://github.com/rust-lang/rust/pull/104488#discussion_r1085556010
This commit is contained in:
commit
05748c66a0
@ -1,5 +1,5 @@
|
||||
use crate::elaborate_drops::DropFlagState;
|
||||
use rustc_middle::mir::{self, Body, Location};
|
||||
use rustc_middle::mir::{self, Body, Location, Terminator, TerminatorKind};
|
||||
use rustc_middle::ty::{self, TyCtxt};
|
||||
use rustc_target::abi::VariantIdx;
|
||||
|
||||
@ -194,6 +194,17 @@ pub fn drop_flag_effects_for_location<'tcx, F>(
|
||||
on_all_children_bits(tcx, body, move_data, path, |mpi| callback(mpi, DropFlagState::Absent))
|
||||
}
|
||||
|
||||
// Drop does not count as a move but we should still consider the variable uninitialized.
|
||||
if let Some(Terminator { kind: TerminatorKind::Drop { place, .. }, .. }) =
|
||||
body.stmt_at(loc).right()
|
||||
{
|
||||
if let LookupResult::Exact(mpi) = move_data.rev_lookup.find(place.as_ref()) {
|
||||
on_all_children_bits(tcx, body, move_data, mpi, |mpi| {
|
||||
callback(mpi, DropFlagState::Absent)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
debug!("drop_flag_effects: assignment for location({:?})", loc);
|
||||
|
||||
for_location_inits(tcx, body, move_data, loc, |mpi| callback(mpi, DropFlagState::Present));
|
||||
|
@ -376,7 +376,8 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
|
||||
| TerminatorKind::Resume
|
||||
| TerminatorKind::Abort
|
||||
| TerminatorKind::GeneratorDrop
|
||||
| TerminatorKind::Unreachable => {}
|
||||
| TerminatorKind::Unreachable
|
||||
| TerminatorKind::Drop { .. } => {}
|
||||
|
||||
TerminatorKind::Assert { ref cond, .. } => {
|
||||
self.gather_operand(cond);
|
||||
@ -391,10 +392,6 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
|
||||
self.create_move_path(place);
|
||||
self.gather_init(place.as_ref(), InitKind::Deep);
|
||||
}
|
||||
|
||||
TerminatorKind::Drop { place, target: _, unwind: _ } => {
|
||||
self.gather_move(place);
|
||||
}
|
||||
TerminatorKind::DropAndReplace { place, ref value, .. } => {
|
||||
self.create_move_path(place);
|
||||
self.gather_operand(value);
|
||||
|
@ -223,13 +223,13 @@ pub trait ValueAnalysis<'tcx> {
|
||||
self.super_terminator(terminator, state)
|
||||
}
|
||||
|
||||
fn super_terminator(&self, terminator: &Terminator<'tcx>, _state: &mut State<Self::Value>) {
|
||||
fn super_terminator(&self, terminator: &Terminator<'tcx>, state: &mut State<Self::Value>) {
|
||||
match &terminator.kind {
|
||||
TerminatorKind::Call { .. } | TerminatorKind::InlineAsm { .. } => {
|
||||
// Effect is applied by `handle_call_return`.
|
||||
}
|
||||
TerminatorKind::Drop { .. } => {
|
||||
// We don't track dropped places.
|
||||
TerminatorKind::Drop { place, .. } => {
|
||||
state.flood_with(place.as_ref(), self.map(), Self::Value::bottom());
|
||||
}
|
||||
TerminatorKind::DropAndReplace { .. } | TerminatorKind::Yield { .. } => {
|
||||
// They would have an effect, but are not allowed in this phase.
|
||||
|
@ -18,6 +18,35 @@ use rustc_span::Span;
|
||||
use rustc_target::abi::VariantIdx;
|
||||
use std::fmt;
|
||||
|
||||
/// During MIR building, Drop and DropAndReplace terminators are inserted in every place where a drop may occur.
|
||||
/// However, in this phase, the presence of these terminators does not guarantee that a destructor will run,
|
||||
/// as the target of the drop may be uninitialized.
|
||||
/// In general, the compiler cannot determine at compile time whether a destructor will run or not.
|
||||
///
|
||||
/// At a high level, this pass refines Drop and DropAndReplace to only run the destructor if the
|
||||
/// target is initialized. The way this is achievied is by inserting drop flags for every variable
|
||||
/// that may be dropped, and then using those flags to determine whether a destructor should run.
|
||||
/// This pass also removes DropAndReplace, replacing it with a Drop paired with an assign statement.
|
||||
/// Once this is complete, Drop terminators in the MIR correspond to a call to the "drop glue" or
|
||||
/// "drop shim" for the type of the dropped place.
|
||||
///
|
||||
/// This pass relies on dropped places having an associated move path, which is then used to determine
|
||||
/// the initialization status of the place and its descendants.
|
||||
/// It's worth noting that a MIR containing a Drop without an associated move path is probably ill formed,
|
||||
/// as it would allow running a destructor on a place behind a reference:
|
||||
///
|
||||
/// ```text
|
||||
// fn drop_term<T>(t: &mut T) {
|
||||
// mir!(
|
||||
// {
|
||||
// Drop(*t, exit)
|
||||
// }
|
||||
// exit = {
|
||||
// Return()
|
||||
// }
|
||||
// )
|
||||
// }
|
||||
/// ```
|
||||
pub struct ElaborateDrops;
|
||||
|
||||
impl<'tcx> MirPass<'tcx> for ElaborateDrops {
|
||||
|
Loading…
x
Reference in New Issue
Block a user