Skip to content

Commit ff1bd3d

Browse files
compiler-errorscompiler-errors
authored
Rollup merge of rust-lang#108208 - cjgillot:flood-enum, r=oli-obk
Correctly handle aggregates in DataflowConstProp The previous implementation from rust-lang#107411 flooded target of an aggregate assignment with `Bottom`, corresponding to the `deinit` that the interpreter does. As a consequence, when assigning `target = Enum::Variant#i(...)` all the `(target as Variant#j)` were at `Bottom` while they should have been `Top`. This PR replaces that flooding with `Top`. Aside, it corrects a second bug where the wrong place would be used to assign to enum variant fields, resulting to nothing happening. Fixes rust-lang#108166
2 parents ef0642a + efb4688 commit ff1bd3d

File tree

4 files changed

+120
-9
lines changed

4 files changed

+120
-9
lines changed

‎compiler/rustc_mir_transform/src/dataflow_const_prop.rs

+18-7
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,10 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> {
122122
) {
123123
match rvalue {
124124
Rvalue::Aggregate(kind, operands) => {
125-
state.flood_with(target.as_ref(), self.map(), FlatSet::Bottom);
125+
// If we assign `target = Enum::Variant#0(operand)`,
126+
// we must make sure that all `target as Variant#i` are `Top`.
127+
state.flood(target.as_ref(), self.map());
128+
126129
if let Some(target_idx) = self.map().find(target.as_ref()) {
127130
let (variant_target, variant_index) = match **kind {
128131
AggregateKind::Tuple | AggregateKind::Closure(..) => {
@@ -131,18 +134,21 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> {
131134
AggregateKind::Adt(def_id, variant_index, ..) => {
132135
match self.tcx.def_kind(def_id) {
133136
DefKind::Struct => (Some(target_idx), None),
134-
DefKind::Enum => (Some(target_idx), Some(variant_index)),
137+
DefKind::Enum => (
138+
self.map.apply(target_idx, TrackElem::Variant(variant_index)),
139+
Some(variant_index),
140+
),
135141
_ => (None, None),
136142
}
137143
}
138144
_ => (None, None),
139145
};
140-
if let Some(target) = variant_target {
146+
if let Some(variant_target_idx) = variant_target {
141147
for (field_index, operand) in operands.iter().enumerate() {
142-
if let Some(field) = self
143-
.map()
144-
.apply(target, TrackElem::Field(Field::from_usize(field_index)))
145-
{
148+
if let Some(field) = self.map().apply(
149+
variant_target_idx,
150+
TrackElem::Field(Field::from_usize(field_index)),
151+
) {
146152
let result = self.handle_operand(operand, state);
147153
state.insert_idx(field, result, self.map());
148154
}
@@ -151,6 +157,11 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> {
151157
if let Some(variant_index) = variant_index
152158
&& let Some(discr_idx) = self.map().apply(target_idx, TrackElem::Discriminant)
153159
{
160+
// We are assigning the discriminant as part of an aggregate.
161+
// This discriminant can only alias a variant field's value if the operand
162+
// had an invalid value for that type.
163+
// Using invalid values is UB, so we are allowed to perform the assignment
164+
// without extra flooding.
154165
let enum_ty = target.ty(self.local_decls, self.tcx).ty;
155166
if let Some(discr_val) = self.eval_discriminant(enum_ty, variant_index) {
156167
state.insert_value_idx(discr_idx, FlatSet::Elem(discr_val), &self.map);

‎tests/mir-opt/dataflow-const-prop/enum.multiple.DataflowConstProp.diff

+82
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
- // MIR for `multiple` before DataflowConstProp
2+
+ // MIR for `multiple` after DataflowConstProp
3+
4+
fn multiple(_1: bool, _2: u8) -> () {
5+
debug x => _1; // in scope 0 at $DIR/enum.rs:+0:13: +0:14
6+
debug i => _2; // in scope 0 at $DIR/enum.rs:+0:22: +0:23
7+
let mut _0: (); // return place in scope 0 at $DIR/enum.rs:+0:29: +0:29
8+
let _3: std::option::Option<u8>; // in scope 0 at $DIR/enum.rs:+1:9: +1:10
9+
let mut _4: bool; // in scope 0 at $DIR/enum.rs:+1:16: +1:17
10+
let mut _5: u8; // in scope 0 at $DIR/enum.rs:+2:14: +2:15
11+
let mut _7: isize; // in scope 0 at $DIR/enum.rs:+9:23: +9:30
12+
scope 1 {
13+
debug e => _3; // in scope 1 at $DIR/enum.rs:+1:9: +1:10
14+
let _6: u8; // in scope 1 at $DIR/enum.rs:+9:9: +9:10
15+
let _8: u8; // in scope 1 at $DIR/enum.rs:+9:28: +9:29
16+
scope 2 {
17+
debug x => _6; // in scope 2 at $DIR/enum.rs:+9:9: +9:10
18+
let _9: u8; // in scope 2 at $DIR/enum.rs:+11:9: +11:10
19+
scope 4 {
20+
debug y => _9; // in scope 4 at $DIR/enum.rs:+11:9: +11:10
21+
}
22+
}
23+
scope 3 {
24+
debug i => _8; // in scope 3 at $DIR/enum.rs:+9:28: +9:29
25+
}
26+
}
27+
28+
bb0: {
29+
StorageLive(_3); // scope 0 at $DIR/enum.rs:+1:9: +1:10
30+
StorageLive(_4); // scope 0 at $DIR/enum.rs:+1:16: +1:17
31+
_4 = _1; // scope 0 at $DIR/enum.rs:+1:16: +1:17
32+
switchInt(move _4) -> [0: bb2, otherwise: bb1]; // scope 0 at $DIR/enum.rs:+1:16: +1:17
33+
}
34+
35+
bb1: {
36+
StorageLive(_5); // scope 0 at $DIR/enum.rs:+2:14: +2:15
37+
_5 = _2; // scope 0 at $DIR/enum.rs:+2:14: +2:15
38+
_3 = Option::<u8>::Some(move _5); // scope 0 at $DIR/enum.rs:+2:9: +2:16
39+
StorageDead(_5); // scope 0 at $DIR/enum.rs:+2:15: +2:16
40+
goto -> bb3; // scope 0 at $DIR/enum.rs:+1:13: +5:6
41+
}
42+
43+
bb2: {
44+
_3 = Option::<u8>::None; // scope 0 at $DIR/enum.rs:+4:9: +4:13
45+
goto -> bb3; // scope 0 at $DIR/enum.rs:+1:13: +5:6
46+
}
47+
48+
bb3: {
49+
StorageDead(_4); // scope 0 at $DIR/enum.rs:+5:5: +5:6
50+
StorageLive(_6); // scope 1 at $DIR/enum.rs:+9:9: +9:10
51+
_7 = discriminant(_3); // scope 1 at $DIR/enum.rs:+9:19: +9:20
52+
switchInt(move _7) -> [0: bb4, 1: bb6, otherwise: bb5]; // scope 1 at $DIR/enum.rs:+9:13: +9:20
53+
}
54+
55+
bb4: {
56+
_6 = const 0_u8; // scope 1 at $DIR/enum.rs:+9:45: +9:46
57+
goto -> bb7; // scope 1 at $DIR/enum.rs:+9:45: +9:46
58+
}
59+
60+
bb5: {
61+
unreachable; // scope 1 at $DIR/enum.rs:+9:19: +9:20
62+
}
63+
64+
bb6: {
65+
StorageLive(_8); // scope 1 at $DIR/enum.rs:+9:28: +9:29
66+
_8 = ((_3 as Some).0: u8); // scope 1 at $DIR/enum.rs:+9:28: +9:29
67+
_6 = _8; // scope 3 at $DIR/enum.rs:+9:34: +9:35
68+
StorageDead(_8); // scope 1 at $DIR/enum.rs:+9:34: +9:35
69+
goto -> bb7; // scope 1 at $DIR/enum.rs:+9:34: +9:35
70+
}
71+
72+
bb7: {
73+
StorageLive(_9); // scope 2 at $DIR/enum.rs:+11:9: +11:10
74+
_9 = _6; // scope 2 at $DIR/enum.rs:+11:13: +11:14
75+
_0 = const (); // scope 0 at $DIR/enum.rs:+0:29: +12:2
76+
StorageDead(_9); // scope 2 at $DIR/enum.rs:+12:1: +12:2
77+
StorageDead(_6); // scope 1 at $DIR/enum.rs:+12:1: +12:2
78+
StorageDead(_3); // scope 0 at $DIR/enum.rs:+12:1: +12:2
79+
return; // scope 0 at $DIR/enum.rs:+12:2: +12:2
80+
}
81+
}
82+

‎tests/mir-opt/dataflow-const-prop/enum.rs

+16
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,23 @@ fn mutate_discriminant() -> u8 {
4646
)
4747
}
4848

49+
// EMIT_MIR enum.multiple.DataflowConstProp.diff
50+
fn multiple(x: bool, i: u8) {
51+
let e = if x {
52+
Some(i)
53+
} else {
54+
None
55+
};
56+
// The dataflow state must have:
57+
// discriminant(e) => Top
58+
// (e as Some).0 => Top
59+
let x = match e { Some(i) => i, None => 0 };
60+
// Therefore, `x` should be `Top` here, and no replacement shall happen.
61+
let y = x;
62+
}
63+
4964
fn main() {
5065
simple();
5166
mutate_discriminant();
67+
multiple(false, 5);
5268
}

‎tests/mir-opt/dataflow-const-prop/enum.simple.DataflowConstProp.diff

+4-2
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,10 @@
4545

4646
bb3: {
4747
StorageLive(_4); // scope 1 at $DIR/enum.rs:+2:29: +2:30
48-
_4 = ((_1 as V1).0: i32); // scope 1 at $DIR/enum.rs:+2:29: +2:30
49-
_2 = _4; // scope 3 at $DIR/enum.rs:+2:35: +2:36
48+
- _4 = ((_1 as V1).0: i32); // scope 1 at $DIR/enum.rs:+2:29: +2:30
49+
- _2 = _4; // scope 3 at $DIR/enum.rs:+2:35: +2:36
50+
+ _4 = const 0_i32; // scope 1 at $DIR/enum.rs:+2:29: +2:30
51+
+ _2 = const 0_i32; // scope 3 at $DIR/enum.rs:+2:35: +2:36
5052
StorageDead(_4); // scope 1 at $DIR/enum.rs:+2:35: +2:36
5153
goto -> bb4; // scope 1 at $DIR/enum.rs:+2:35: +2:36
5254
}

0 commit comments

Comments
 (0)