Skip to content

Commit 823ff8c

Browse files
committed
Auto merge of #69676 - ecstatic-morse:fix-enum-discr-effect, r=oli-obk
Pass correct place to `discriminant_switch_effect` PR #69562, which fixed a bug that was causing clippy to ICE, mistakenly passed the place holding the *result* of `Rvalue::Discriminant` instead of the place holding its *operand* to `apply_discriminant_switch_effect` as the enum place. As a result, no effect was applied at all, and we lost the perf benefits from marking inactive enum variants as uninitialized. **edit:** The regression test has been split into #69744. r? @oli-obk
2 parents a039217 + e82ec23 commit 823ff8c

File tree

1 file changed

+28
-20
lines changed

1 file changed

+28
-20
lines changed

src/librustc_mir/dataflow/generic/engine.rs

+28-20
Original file line numberDiff line numberDiff line change
@@ -239,23 +239,26 @@ where
239239
}
240240

241241
SwitchInt { ref targets, ref values, ref discr, .. } => {
242-
// If this is a switch on an enum discriminant, a custom effect may be applied
243-
// along each outgoing edge.
244-
if let Some(place) = discr.place() {
245-
let enum_def = switch_on_enum_discriminant(self.tcx, self.body, bb_data, place);
246-
if let Some(enum_def) = enum_def {
242+
let Engine { tcx, body, .. } = *self;
243+
let enum_ = discr
244+
.place()
245+
.and_then(|discr| switch_on_enum_discriminant(tcx, body, bb_data, discr));
246+
match enum_ {
247+
// If this is a switch on an enum discriminant, a custom effect may be applied
248+
// along each outgoing edge.
249+
Some((enum_place, enum_def)) => {
247250
self.propagate_bits_into_enum_discriminant_switch_successors(
248-
in_out, bb, enum_def, place, dirty_list, &*values, &*targets,
251+
in_out, bb, enum_def, enum_place, dirty_list, &*values, &*targets,
249252
);
250-
251-
return;
252253
}
253-
}
254254

255-
// Otherwise, it's just a normal `SwitchInt`, and every successor sees the same
256-
// exit state.
257-
for target in targets.iter().copied() {
258-
self.propagate_bits_into_entry_set_for(&in_out, target, dirty_list);
255+
// Otherwise, it's just a normal `SwitchInt`, and every successor sees the same
256+
// exit state.
257+
None => {
258+
for target in targets.iter().copied() {
259+
self.propagate_bits_into_entry_set_for(&in_out, target, dirty_list);
260+
}
261+
}
259262
}
260263
}
261264

@@ -342,22 +345,27 @@ where
342345
}
343346
}
344347

345-
/// Look at the last statement of a block that ends with to see if it is an assignment of an enum
346-
/// discriminant to the local that determines the target of a `SwitchInt` like so:
347-
/// _42 = discriminant(..)
348+
/// Inspect a `SwitchInt`-terminated basic block to see if the condition of that `SwitchInt` is
349+
/// an enum discriminant.
350+
///
351+
/// We expect such blocks to have a call to `discriminant` as their last statement like so:
352+
/// _42 = discriminant(_1)
348353
/// SwitchInt(_42, ..)
354+
///
355+
/// If the basic block matches this pattern, this function returns the place corresponding to the
356+
/// enum (`_1` in the example above) as well as the `AdtDef` of that enum.
349357
fn switch_on_enum_discriminant(
350358
tcx: TyCtxt<'tcx>,
351-
body: &mir::Body<'tcx>,
352-
block: &mir::BasicBlockData<'tcx>,
359+
body: &'mir mir::Body<'tcx>,
360+
block: &'mir mir::BasicBlockData<'tcx>,
353361
switch_on: &mir::Place<'tcx>,
354-
) -> Option<&'tcx ty::AdtDef> {
362+
) -> Option<(&'mir mir::Place<'tcx>, &'tcx ty::AdtDef)> {
355363
match block.statements.last().map(|stmt| &stmt.kind) {
356364
Some(mir::StatementKind::Assign(box (lhs, mir::Rvalue::Discriminant(discriminated))))
357365
if lhs == switch_on =>
358366
{
359367
match &discriminated.ty(body, tcx).ty.kind {
360-
ty::Adt(def, _) => Some(def),
368+
ty::Adt(def, _) => Some((discriminated, def)),
361369

362370
// `Rvalue::Discriminant` is also used to get the active yield point for a
363371
// generator, but we do not need edge-specific effects in that case. This may

0 commit comments

Comments
 (0)