Skip to content

Commit 68c1e2f

Browse files
committed
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 a) A dropped value won't be used again b) 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: (a) as there is no way of reaching the old value. drop-elaboration will also remove any uninitialized drop. (b) 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.
1 parent c8e6a9e commit 68c1e2f

File tree

4 files changed

+46
-9
lines changed

4 files changed

+46
-9
lines changed

compiler/rustc_mir_dataflow/src/drop_flag_effects.rs

+12-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::elaborate_drops::DropFlagState;
2-
use rustc_middle::mir::{self, Body, Location};
2+
use rustc_middle::mir::{self, Body, Location, Terminator, TerminatorKind};
33
use rustc_middle::ty::{self, TyCtxt};
44
use rustc_target::abi::VariantIdx;
55

@@ -194,6 +194,17 @@ pub fn drop_flag_effects_for_location<'tcx, F>(
194194
on_all_children_bits(tcx, body, move_data, path, |mpi| callback(mpi, DropFlagState::Absent))
195195
}
196196

197+
// Drop does not count as a move but we should still consider the variable uninitialized.
198+
if let Some(Terminator { kind: TerminatorKind::Drop { place, .. }, .. }) =
199+
body.stmt_at(loc).right()
200+
{
201+
if let LookupResult::Exact(mpi) = move_data.rev_lookup.find(place.as_ref()) {
202+
on_all_children_bits(tcx, body, move_data, mpi, |mpi| {
203+
callback(mpi, DropFlagState::Absent)
204+
})
205+
}
206+
}
207+
197208
debug!("drop_flag_effects: assignment for location({:?})", loc);
198209

199210
for_location_inits(tcx, body, move_data, loc, |mpi| callback(mpi, DropFlagState::Present));

compiler/rustc_mir_dataflow/src/move_paths/builder.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,8 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
375375
| TerminatorKind::Resume
376376
| TerminatorKind::Abort
377377
| TerminatorKind::GeneratorDrop
378-
| TerminatorKind::Unreachable => {}
378+
| TerminatorKind::Unreachable
379+
| TerminatorKind::Drop { .. } => {}
379380

380381
TerminatorKind::Assert { ref cond, .. } => {
381382
self.gather_operand(cond);
@@ -390,10 +391,6 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
390391
self.create_move_path(place);
391392
self.gather_init(place.as_ref(), InitKind::Deep);
392393
}
393-
394-
TerminatorKind::Drop { place, target: _, unwind: _ } => {
395-
self.gather_move(place);
396-
}
397394
TerminatorKind::DropAndReplace { place, ref value, .. } => {
398395
self.create_move_path(place);
399396
self.gather_operand(value);

compiler/rustc_mir_dataflow/src/value_analysis.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -222,13 +222,13 @@ pub trait ValueAnalysis<'tcx> {
222222
self.super_terminator(terminator, state)
223223
}
224224

225-
fn super_terminator(&self, terminator: &Terminator<'tcx>, _state: &mut State<Self::Value>) {
225+
fn super_terminator(&self, terminator: &Terminator<'tcx>, state: &mut State<Self::Value>) {
226226
match &terminator.kind {
227227
TerminatorKind::Call { .. } | TerminatorKind::InlineAsm { .. } => {
228228
// Effect is applied by `handle_call_return`.
229229
}
230-
TerminatorKind::Drop { .. } => {
231-
// We don't track dropped places.
230+
TerminatorKind::Drop { place, .. } => {
231+
state.flood_with(place.as_ref(), self.map(), Self::Value::bottom());
232232
}
233233
TerminatorKind::DropAndReplace { .. } | TerminatorKind::Yield { .. } => {
234234
// They would have an effect, but are not allowed in this phase.

compiler/rustc_mir_transform/src/elaborate_drops.rs

+29
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,35 @@ use rustc_span::Span;
1818
use rustc_target::abi::VariantIdx;
1919
use std::fmt;
2020

21+
/// During MIR building, Drop and DropAndReplace terminators are inserted in every place where a drop may occur.
22+
/// However, in this phase, the presence of these terminators does not guarantee that a destructor will run,
23+
/// as the target of the drop may be uninitialized.
24+
/// In general, the compiler cannot determine at compile time whether a destructor will run or not.
25+
///
26+
/// At a high level, this pass refines Drop and DropAndReplace to only run the destructor if the
27+
/// target is initialized. The way this is achievied is by inserting drop flags for every variable
28+
/// that may be dropped, and then using those flags to determine whether a destructor should run.
29+
/// This pass also removes DropAndReplace, replacing it with a Drop paired with an assign statement.
30+
/// Once this is complete, Drop terminators in the MIR correspond to a call to the "drop glue" or
31+
/// "drop shim" for the type of the dropped place.
32+
///
33+
/// This pass relies on dropped places having an associated move path, which is then used to determine
34+
/// the initialization status of the place and its descendants.
35+
/// It's worth noting that a MIR containing a Drop without an associated move path is probably ill formed,
36+
/// as it would allow running a destructor on a place behind a reference:
37+
///
38+
/// ```text
39+
// fn drop_term<T>(t: &mut T) {
40+
// mir!(
41+
// {
42+
// Drop(*t, exit)
43+
// }
44+
// exit = {
45+
// Return()
46+
// }
47+
// )
48+
// }
49+
/// ```
2150
pub struct ElaborateDrops;
2251

2352
impl<'tcx> MirPass<'tcx> for ElaborateDrops {

0 commit comments

Comments
 (0)