Skip to content

Commit 816f2d6

Browse files
authored
Rollup merge of rust-lang#91410 - ecstatic-morse:const-precise-live-drops-take-2, r=oli-obk
Move `#![feature(const_precise_live_drops)]` checks earlier in the pipeline Should mitigate the issues found during MCP on rust-lang#73255. Once this is done, we should clean up the queries a bit, since I think `mir_drops_elaborated_and_const_checked` can be merged back into `mir_promoted`. Fixes rust-lang#90770. cc `@rust-lang/wg-const-eval` r? `@nikomatsakis` (since they reviewed rust-lang#71824)
2 parents c8bee63 + 37fa925 commit 816f2d6

15 files changed

+275
-32
lines changed

compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ impl Visitor<'tcx> for CheckLiveDrops<'mir, 'tcx> {
8080
trace!("visit_terminator: terminator={:?} location={:?}", terminator, location);
8181

8282
match &terminator.kind {
83-
mir::TerminatorKind::Drop { place: dropped_place, .. } => {
83+
mir::TerminatorKind::Drop { place: dropped_place, .. }
84+
| mir::TerminatorKind::DropAndReplace { place: dropped_place, .. } => {
8485
let dropped_ty = dropped_place.ty(self.body, self.tcx).ty;
8586
if !NeedsNonConstDrop::in_any_value_of_ty(self.ccx, dropped_ty) {
8687
// Instead of throwing a bug, we just return here. This is because we have to
@@ -104,11 +105,6 @@ impl Visitor<'tcx> for CheckLiveDrops<'mir, 'tcx> {
104105
}
105106
}
106107

107-
mir::TerminatorKind::DropAndReplace { .. } => span_bug!(
108-
terminator.source_info.span,
109-
"`DropAndReplace` should be removed by drop elaboration",
110-
),
111-
112108
mir::TerminatorKind::Abort
113109
| mir::TerminatorKind::Call { .. }
114110
| mir::TerminatorKind::Assert { .. }

compiler/rustc_middle/src/mir/mod.rs

+10
Original file line numberDiff line numberDiff line change
@@ -1803,6 +1803,16 @@ impl<V, T> ProjectionElem<V, T> {
18031803
| Self::Downcast(_, _) => false,
18041804
}
18051805
}
1806+
1807+
/// Returns `true` if this is a `Downcast` projection with the given `VariantIdx`.
1808+
pub fn is_downcast_to(&self, v: VariantIdx) -> bool {
1809+
matches!(*self, Self::Downcast(_, x) if x == v)
1810+
}
1811+
1812+
/// Returns `true` if this is a `Field` projection with the given index.
1813+
pub fn is_field_to(&self, f: Field) -> bool {
1814+
matches!(*self, Self::Field(x, _) if x == f)
1815+
}
18061816
}
18071817

18081818
/// Alias for projections as they appear in places, where the base is a place

compiler/rustc_mir_transform/src/lib.rs

+19-5
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,10 @@ mod match_branches;
6060
mod multiple_return_terminators;
6161
mod normalize_array_len;
6262
mod nrvo;
63+
mod remove_false_edges;
6364
mod remove_noop_landing_pads;
6465
mod remove_storage_markers;
66+
mod remove_uninit_drops;
6567
mod remove_unneeded_drops;
6668
mod remove_zsts;
6769
mod required_consts;
@@ -75,7 +77,7 @@ mod simplify_try;
7577
mod uninhabited_enum_branching;
7678
mod unreachable_prop;
7779

78-
use rustc_const_eval::transform::check_consts;
80+
use rustc_const_eval::transform::check_consts::{self, ConstCx};
7981
use rustc_const_eval::transform::promote_consts;
8082
use rustc_const_eval::transform::validate;
8183
use rustc_mir_dataflow::rustc_peek;
@@ -444,8 +446,20 @@ fn mir_drops_elaborated_and_const_checked<'tcx>(
444446
let (body, _) = tcx.mir_promoted(def);
445447
let mut body = body.steal();
446448

449+
// IMPORTANT
450+
remove_false_edges::RemoveFalseEdges.run_pass(tcx, &mut body);
451+
452+
// Do a little drop elaboration before const-checking if `const_precise_live_drops` is enabled.
453+
//
454+
// FIXME: Can't use `run_passes` for these, since `run_passes` SILENTLY DOES NOTHING IF THE MIR
455+
// PHASE DOESN'T CHANGE.
456+
if check_consts::post_drop_elaboration::checking_enabled(&ConstCx::new(tcx, &body)) {
457+
simplify::SimplifyCfg::new("remove-false-edges").run_pass(tcx, &mut body);
458+
remove_uninit_drops::RemoveUninitDrops.run_pass(tcx, &mut body);
459+
check_consts::post_drop_elaboration::check_live_drops(tcx, &body);
460+
}
461+
447462
run_post_borrowck_cleanup_passes(tcx, &mut body);
448-
check_consts::post_drop_elaboration::check_live_drops(tcx, &body);
449463
tcx.alloc_steal_mir(body)
450464
}
451465

@@ -455,7 +469,7 @@ fn run_post_borrowck_cleanup_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tc
455469

456470
let post_borrowck_cleanup: &[&dyn MirPass<'tcx>] = &[
457471
// Remove all things only needed by analysis
458-
&simplify_branches::SimplifyBranches::new("initial"),
472+
&simplify_branches::SimplifyConstCondition::new("initial"),
459473
&remove_noop_landing_pads::RemoveNoopLandingPads,
460474
&cleanup_post_borrowck::CleanupNonCodegenStatements,
461475
&simplify::SimplifyCfg::new("early-opt"),
@@ -514,13 +528,13 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
514528
&instcombine::InstCombine,
515529
&separate_const_switch::SeparateConstSwitch,
516530
&const_prop::ConstProp,
517-
&simplify_branches::SimplifyBranches::new("after-const-prop"),
531+
&simplify_branches::SimplifyConstCondition::new("after-const-prop"),
518532
&early_otherwise_branch::EarlyOtherwiseBranch,
519533
&simplify_comparison_integral::SimplifyComparisonIntegral,
520534
&simplify_try::SimplifyArmIdentity,
521535
&simplify_try::SimplifyBranchSame,
522536
&dest_prop::DestinationPropagation,
523-
&simplify_branches::SimplifyBranches::new("final"),
537+
&simplify_branches::SimplifyConstCondition::new("final"),
524538
&remove_noop_landing_pads::RemoveNoopLandingPads,
525539
&simplify::SimplifyCfg::new("final"),
526540
&nrvo::RenameReturnPlace,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
use rustc_middle::mir::{Body, TerminatorKind};
2+
use rustc_middle::ty::TyCtxt;
3+
4+
use crate::MirPass;
5+
6+
/// Removes `FalseEdge` and `FalseUnwind` terminators from the MIR.
7+
///
8+
/// These are only needed for borrow checking, and can be removed afterwards.
9+
///
10+
/// FIXME: This should probably have its own MIR phase.
11+
pub struct RemoveFalseEdges;
12+
13+
impl<'tcx> MirPass<'tcx> for RemoveFalseEdges {
14+
fn run_pass(&self, _: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
15+
for block in body.basic_blocks_mut() {
16+
let terminator = block.terminator_mut();
17+
terminator.kind = match terminator.kind {
18+
TerminatorKind::FalseEdge { real_target, .. } => {
19+
TerminatorKind::Goto { target: real_target }
20+
}
21+
TerminatorKind::FalseUnwind { real_target, .. } => {
22+
TerminatorKind::Goto { target: real_target }
23+
}
24+
25+
_ => continue,
26+
}
27+
}
28+
}
29+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
use rustc_index::bit_set::BitSet;
2+
use rustc_middle::mir::{Body, Field, Rvalue, Statement, StatementKind, TerminatorKind};
3+
use rustc_middle::ty::subst::SubstsRef;
4+
use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt, VariantDef};
5+
use rustc_mir_dataflow::impls::MaybeInitializedPlaces;
6+
use rustc_mir_dataflow::move_paths::{LookupResult, MoveData, MovePathIndex};
7+
use rustc_mir_dataflow::{self, move_path_children_matching, Analysis, MoveDataParamEnv};
8+
9+
use crate::MirPass;
10+
11+
/// Removes `Drop` and `DropAndReplace` terminators whose target is known to be uninitialized at
12+
/// that point.
13+
///
14+
/// This is redundant with drop elaboration, but we need to do it prior to const-checking, and
15+
/// running const-checking after drop elaboration makes it opimization dependent, causing issues
16+
/// like [#90770].
17+
///
18+
/// [#90770]: https://github.com/rust-lang/rust/issues/90770
19+
pub struct RemoveUninitDrops;
20+
21+
impl<'tcx> MirPass<'tcx> for RemoveUninitDrops {
22+
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
23+
let param_env = tcx.param_env(body.source.def_id());
24+
let Ok(move_data) = MoveData::gather_moves(body, tcx, param_env) else {
25+
// We could continue if there are move errors, but there's not much point since our
26+
// init data isn't complete.
27+
return;
28+
};
29+
30+
let mdpe = MoveDataParamEnv { move_data, param_env };
31+
let mut maybe_inits = MaybeInitializedPlaces::new(tcx, body, &mdpe)
32+
.into_engine(tcx, body)
33+
.pass_name("remove_uninit_drops")
34+
.iterate_to_fixpoint()
35+
.into_results_cursor(body);
36+
37+
let mut to_remove = vec![];
38+
for (bb, block) in body.basic_blocks().iter_enumerated() {
39+
let terminator = block.terminator();
40+
let (TerminatorKind::Drop { place, .. } | TerminatorKind::DropAndReplace { place, .. })
41+
= &terminator.kind
42+
else { continue };
43+
44+
maybe_inits.seek_before_primary_effect(body.terminator_loc(bb));
45+
46+
// If there's no move path for the dropped place, it's probably a `Deref`. Let it alone.
47+
let LookupResult::Exact(mpi) = mdpe.move_data.rev_lookup.find(place.as_ref()) else {
48+
continue;
49+
};
50+
51+
let should_keep = is_needs_drop_and_init(
52+
tcx,
53+
param_env,
54+
maybe_inits.get(),
55+
&mdpe.move_data,
56+
place.ty(body, tcx).ty,
57+
mpi,
58+
);
59+
if !should_keep {
60+
to_remove.push(bb)
61+
}
62+
}
63+
64+
for bb in to_remove {
65+
let block = &mut body.basic_blocks_mut()[bb];
66+
67+
let (TerminatorKind::Drop { target, .. } | TerminatorKind::DropAndReplace { target, .. })
68+
= &block.terminator().kind
69+
else { unreachable!() };
70+
71+
// Replace block terminator with `Goto`.
72+
let target = *target;
73+
let old_terminator_kind = std::mem::replace(
74+
&mut block.terminator_mut().kind,
75+
TerminatorKind::Goto { target },
76+
);
77+
78+
// If this is a `DropAndReplace`, we need to emulate the assignment to the return place.
79+
if let TerminatorKind::DropAndReplace { place, value, .. } = old_terminator_kind {
80+
block.statements.push(Statement {
81+
source_info: block.terminator().source_info,
82+
kind: StatementKind::Assign(Box::new((place, Rvalue::Use(value)))),
83+
});
84+
}
85+
}
86+
}
87+
}
88+
89+
fn is_needs_drop_and_init(
90+
tcx: TyCtxt<'tcx>,
91+
param_env: ParamEnv<'tcx>,
92+
maybe_inits: &BitSet<MovePathIndex>,
93+
move_data: &MoveData<'tcx>,
94+
ty: Ty<'tcx>,
95+
mpi: MovePathIndex,
96+
) -> bool {
97+
// No need to look deeper if the root is definitely uninit or if it has no `Drop` impl.
98+
if !maybe_inits.contains(mpi) || !ty.needs_drop(tcx, param_env) {
99+
return false;
100+
}
101+
102+
let field_needs_drop_and_init = |(f, f_ty, mpi)| {
103+
let child = move_path_children_matching(move_data, mpi, |x| x.is_field_to(f));
104+
let Some(mpi) = child else {
105+
return f_ty.needs_drop(tcx, param_env);
106+
};
107+
108+
is_needs_drop_and_init(tcx, param_env, maybe_inits, move_data, f_ty, mpi)
109+
};
110+
111+
// This pass is only needed for const-checking, so it doesn't handle as many cases as
112+
// `DropCtxt::open_drop`, since they aren't relevant in a const-context.
113+
match ty.kind() {
114+
ty::Adt(adt, substs) => {
115+
let dont_elaborate = adt.is_union() || adt.is_manually_drop() || adt.has_dtor(tcx);
116+
if dont_elaborate {
117+
return true;
118+
}
119+
120+
// Look at all our fields, or if we are an enum all our variants and their fields.
121+
//
122+
// If a field's projection *is not* present in `MoveData`, it has the same
123+
// initializedness as its parent (maybe init).
124+
//
125+
// If its projection *is* present in `MoveData`, then the field may have been moved
126+
// from separate from its parent. Recurse.
127+
adt.variants.iter_enumerated().any(|(vid, variant)| {
128+
// Enums have multiple variants, which are discriminated with a `Downcast` projection.
129+
// Structs have a single variant, and don't use a `Downcast` projection.
130+
let mpi = if adt.is_enum() {
131+
let downcast =
132+
move_path_children_matching(move_data, mpi, |x| x.is_downcast_to(vid));
133+
let Some(dc_mpi) = downcast else {
134+
return variant_needs_drop(tcx, param_env, substs, variant);
135+
};
136+
137+
dc_mpi
138+
} else {
139+
mpi
140+
};
141+
142+
variant
143+
.fields
144+
.iter()
145+
.enumerate()
146+
.map(|(f, field)| (Field::from_usize(f), field.ty(tcx, substs), mpi))
147+
.any(field_needs_drop_and_init)
148+
})
149+
}
150+
151+
ty::Tuple(_) => ty
152+
.tuple_fields()
153+
.enumerate()
154+
.map(|(f, f_ty)| (Field::from_usize(f), f_ty, mpi))
155+
.any(field_needs_drop_and_init),
156+
157+
_ => true,
158+
}
159+
}
160+
161+
fn variant_needs_drop(
162+
tcx: TyCtxt<'tcx>,
163+
param_env: ParamEnv<'tcx>,
164+
substs: SubstsRef<'tcx>,
165+
variant: &VariantDef,
166+
) -> bool {
167+
variant.fields.iter().any(|field| {
168+
let f_ty = field.ty(tcx, substs);
169+
f_ty.needs_drop(tcx, param_env)
170+
})
171+
}

compiler/rustc_mir_transform/src/remove_unneeded_drops.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
//! This pass replaces a drop of a type that does not need dropping, with a goto
1+
//! This pass replaces a drop of a type that does not need dropping, with a goto.
2+
//!
3+
//! When the MIR is built, we check `needs_drop` before emitting a `Drop` for a place. This pass is
4+
//! useful because (unlike MIR building) it runs after type checking, so it can make use of
5+
//! `Reveal::All` to provide more precies type information.
26
37
use crate::MirPass;
48
use rustc_middle::mir::*;

compiler/rustc_mir_transform/src/simplify_branches.rs

+5-12
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,21 @@
1-
//! A pass that simplifies branches when their condition is known.
2-
31
use crate::MirPass;
42
use rustc_middle::mir::*;
53
use rustc_middle::ty::TyCtxt;
64

75
use std::borrow::Cow;
86

9-
pub struct SimplifyBranches {
7+
/// A pass that replaces a branch with a goto when its condition is known.
8+
pub struct SimplifyConstCondition {
109
label: String,
1110
}
1211

13-
impl SimplifyBranches {
12+
impl SimplifyConstCondition {
1413
pub fn new(label: &str) -> Self {
15-
SimplifyBranches { label: format!("SimplifyBranches-{}", label) }
14+
SimplifyConstCondition { label: format!("SimplifyConstCondition-{}", label) }
1615
}
1716
}
1817

19-
impl<'tcx> MirPass<'tcx> for SimplifyBranches {
18+
impl<'tcx> MirPass<'tcx> for SimplifyConstCondition {
2019
fn name(&self) -> Cow<'_, str> {
2120
Cow::Borrowed(&self.label)
2221
}
@@ -53,12 +52,6 @@ impl<'tcx> MirPass<'tcx> for SimplifyBranches {
5352
Some(v) if v == expected => TerminatorKind::Goto { target },
5453
_ => continue,
5554
},
56-
TerminatorKind::FalseEdge { real_target, .. } => {
57-
TerminatorKind::Goto { target: real_target }
58-
}
59-
TerminatorKind::FalseUnwind { real_target, .. } => {
60-
TerminatorKind::Goto { target: real_target }
61-
}
6255
_ => continue,
6356
};
6457
}

src/test/mir-opt/const_prop/switch_int.main.SimplifyBranches-after-const-prop.diff src/test/mir-opt/const_prop/switch_int.main.SimplifyConstCondition-after-const-prop.diff

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
- // MIR for `main` before SimplifyBranches-after-const-prop
2-
+ // MIR for `main` after SimplifyBranches-after-const-prop
1+
- // MIR for `main` before SimplifyConstCondition-after-const-prop
2+
+ // MIR for `main` after SimplifyConstCondition-after-const-prop
33

44
fn main() -> () {
55
let mut _0: (); // return place in scope 0 at $DIR/switch_int.rs:6:11: 6:11

src/test/mir-opt/const_prop/switch_int.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
fn foo(_: i32) { }
33

44
// EMIT_MIR switch_int.main.ConstProp.diff
5-
// EMIT_MIR switch_int.main.SimplifyBranches-after-const-prop.diff
5+
// EMIT_MIR switch_int.main.SimplifyConstCondition-after-const-prop.diff
66
fn main() {
77
match 1 {
88
1 => foo(0),

src/test/mir-opt/early_otherwise_branch_68867.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ pub enum ViewportPercentageLength {
1111
}
1212

1313
// EMIT_MIR early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.diff
14-
// EMIT_MIR early_otherwise_branch_68867.try_sum EarlyOtherwiseBranch.before SimplifyBranches-final.after
14+
// EMIT_MIR early_otherwise_branch_68867.try_sum EarlyOtherwiseBranch.before SimplifyConstCondition-final.after
1515
#[no_mangle]
1616
pub extern "C" fn try_sum(
1717
x: &ViewportPercentageLength,

src/test/mir-opt/early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.before-SimplifyBranches-final.after.diff src/test/mir-opt/early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.before-SimplifyConstCondition-final.after.diff

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
- // MIR for `try_sum` before EarlyOtherwiseBranch
2-
+ // MIR for `try_sum` after SimplifyBranches-final
2+
+ // MIR for `try_sum` after SimplifyConstCondition-final
33

44
fn try_sum(_1: &ViewportPercentageLength, _2: &ViewportPercentageLength) -> Result<ViewportPercentageLength, ()> {
55
debug x => _1; // in scope 0 at $DIR/early_otherwise_branch_68867.rs:17:5: 17:6

src/test/mir-opt/simplify_if.main.SimplifyBranches-after-const-prop.diff src/test/mir-opt/simplify_if.main.SimplifyConstCondition-after-const-prop.diff

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
- // MIR for `main` before SimplifyBranches-after-const-prop
2-
+ // MIR for `main` after SimplifyBranches-after-const-prop
1+
- // MIR for `main` before SimplifyConstCondition-after-const-prop
2+
+ // MIR for `main` after SimplifyConstCondition-after-const-prop
33

44
fn main() -> () {
55
let mut _0: (); // return place in scope 0 at $DIR/simplify_if.rs:5:11: 5:11

0 commit comments

Comments
 (0)