Skip to content

Do not project to uninhabited variant in JumpThreading + const printing + GVN #120350

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions compiler/rustc_const_eval/src/const_eval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ pub(crate) fn try_destructure_mir_constant_for_user_output<'tcx>(
}
ty::Adt(def, _) => {
let variant = ecx.read_discriminant(&op).ok()?;
if op.layout.for_variant(&ecx, variant).abi.is_uninhabited() {
return None;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should have such checks everywhere. That's too fragile. We should just remove the assertion: #120367.

let down = ecx.project_downcast(&op, variant).ok()?;
(def.variants()[variant].fields.len(), Some(variant), down)
}
Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_mir_transform/src/gvn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,14 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
return None;
}
}
ProjectionElem::Downcast(name, index) => ProjectionElem::Downcast(name, index),
ProjectionElem::Downcast(name, index) => {
if let Some(ct) = self.eval_to_const(value)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't bail on the None case since GVN runs on polymorphic MIR. Is there a better uninhabited method for possibly polymorphic types?

&& ct.layout.for_variant(&self.ecx, index).abi.is_uninhabited()
{
return None;
}
ProjectionElem::Downcast(name, index)
}
ProjectionElem::Field(f, ty) => {
if let Value::Aggregate(_, _, fields) = self.get(value) {
return Some(fields[f.as_usize()]);
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_mir_transform/src/jump_threading.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,12 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> {
constant,
&mut |elem, op| match elem {
TrackElem::Field(idx) => self.ecx.project_field(op, idx.as_usize()).ok(),
TrackElem::Variant(idx) => self.ecx.project_downcast(op, idx).ok(),
TrackElem::Variant(idx) => {
if op.layout.for_variant(&self.ecx, idx).abi.is_uninhabited() {
return None;
}
self.ecx.project_downcast(op, idx).ok()
}
TrackElem::Discriminant => {
let variant = self.ecx.read_discriminant(op).ok()?;
let discr_value =
Expand Down
34 changes: 34 additions & 0 deletions tests/mir-opt/gvn_uninhabited.f.GVN.panic-abort.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
- // MIR for `f` before GVN
+ // MIR for `f` after GVN

fn f() -> u32 {
let mut _0: u32;
let _1: u32;
let mut _2: E;
let mut _3: &U;
let _4: U;
scope 1 {
debug i => _1;
}
scope 2 {
let mut _5: &U;
}

bb0: {
StorageLive(_2);
StorageLive(_3);
_5 = const _;
_3 = &(*_5);
_2 = ((*_3).1: E);
- StorageLive(_1);
+ nop;
_1 = ((_2 as A).1: u32);
StorageDead(_3);
StorageDead(_2);
_0 = _1;
- StorageDead(_1);
+ nop;
return;
}
}

34 changes: 34 additions & 0 deletions tests/mir-opt/gvn_uninhabited.f.GVN.panic-unwind.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
- // MIR for `f` before GVN
+ // MIR for `f` after GVN

fn f() -> u32 {
let mut _0: u32;
let _1: u32;
let mut _2: E;
let mut _3: &U;
let _4: U;
scope 1 {
debug i => _1;
}
scope 2 {
let mut _5: &U;
}

bb0: {
StorageLive(_2);
StorageLive(_3);
_5 = const _;
_3 = &(*_5);
_2 = ((*_3).1: E);
- StorageLive(_1);
+ nop;
_1 = ((_2 as A).1: u32);
StorageDead(_3);
StorageDead(_2);
_0 = _1;
- StorageDead(_1);
+ nop;
return;
}
}

24 changes: 24 additions & 0 deletions tests/mir-opt/gvn_uninhabited.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// unit-test: GVN
// compile-flags: -O
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
// skip-filecheck

#![feature(never_type)]

#[derive(Copy, Clone)]
pub enum E {
A(!, u32),
}

pub union U {
i: u32,
e: E,
}

// EMIT_MIR gvn_uninhabited.f.GVN.diff
pub const fn f() -> u32 {
let E::A(_, i) = unsafe { (&U { i: 0 }).e };
i
}

fn main() {}
16 changes: 16 additions & 0 deletions tests/mir-opt/jump_threading_uninhabited.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// unit-test: JumpThreading
// compile-flags: -Zmir-opt-level=3 -Zunsound-mir-opts
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
// skip-filecheck

pub enum HiddenType {}

pub struct Wrap<T>(T);

// EMIT_MIR jump_threading_uninhabited.test_questionmark.JumpThreading.diff
fn test_questionmark() -> Result<(), ()> {
Ok(Ok(()))??;
Ok(())
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
- // MIR for `test_questionmark` before JumpThreading
+ // MIR for `test_questionmark` after JumpThreading

fn test_questionmark() -> Result<(), ()> {
let mut _0: std::result::Result<(), ()>;
let mut _1: std::ops::ControlFlow<std::result::Result<std::convert::Infallible, ()>>;
let mut _2: std::result::Result<(), ()>;
let mut _3: std::ops::ControlFlow<std::result::Result<std::convert::Infallible, ()>, std::result::Result<(), ()>>;
let mut _4: std::result::Result<std::result::Result<(), ()>, ()>;
let mut _5: isize;
let _6: std::result::Result<(), ()>;
let mut _7: isize;
scope 1 {
debug residual => const Result::<Infallible, ()>::Err(());
scope 2 {
scope 15 (inlined #[track_caller] <Result<(), ()> as FromResidual<Result<Infallible, ()>>>::from_residual) {
debug residual => const Result::<Infallible, ()>::Err(());
scope 16 {
debug e => const ();
scope 17 (inlined <() as From<()>>::from) {
debug t => const ();
}
}
}
}
}
scope 3 {
debug val => const Result::<(), ()>::Ok(());
scope 4 {
}
}
scope 5 {
debug residual => const Result::<Infallible, ()>::Err(());
scope 6 {
scope 18 (inlined #[track_caller] <Result<(), ()> as FromResidual<Result<Infallible, ()>>>::from_residual) {
debug residual => const Result::<Infallible, ()>::Err(());
scope 19 {
debug e => const ();
scope 20 (inlined <() as From<()>>::from) {
debug t => const ();
}
}
}
}
}
scope 7 {
debug val => const ();
scope 8 {
}
}
scope 9 (inlined <Result<Result<(), ()>, ()> as Try>::branch) {
debug self => const Result::<Result<(), ()>, ()>::Ok(Result::<(), ()>::Ok(()));
let _8: std::result::Result<(), ()>;
scope 10 {
debug v => const Result::<(), ()>::Ok(());
}
scope 11 {
debug e => const ();
}
}
scope 12 (inlined <Result<(), ()> as Try>::branch) {
debug self => const Result::<(), ()>::Ok(());
let mut _9: isize;
scope 13 {
debug v => const ();
}
scope 14 {
debug e => const ();
}
}

bb0: {
StorageLive(_1);
StorageLive(_2);
StorageLive(_3);
StorageLive(_4);
_4 = const Result::<Result<(), ()>, ()>::Ok(Result::<(), ()>::Ok(()));
StorageLive(_8);
goto -> bb8;
}

bb1: {
_6 = const Result::<(), ()>::Ok(());
_2 = const Result::<(), ()>::Ok(());
StorageLive(_9);
_9 = const 0_isize;
goto -> bb10;
}

bb2: {
_0 = const Result::<(), ()>::Err(());
StorageDead(_2);
goto -> bb5;
}

bb3: {
StorageDead(_3);
StorageDead(_1);
_0 = const Result::<(), ()>::Ok(());
goto -> bb6;
}

bb4: {
_0 = const Result::<(), ()>::Err(());
goto -> bb5;
}

bb5: {
StorageDead(_3);
StorageDead(_1);
goto -> bb6;
}

bb6: {
return;
}

bb7: {
_3 = const ControlFlow::<Result<Infallible, ()>, Result<(), ()>>::Break(Result::<Infallible, ()>::Err(()));
StorageDead(_8);
StorageDead(_4);
_5 = discriminant(_3);
- switchInt(move _5) -> [0: bb1, 1: bb2, otherwise: bb11];
+ goto -> bb2;
}

bb8: {
_8 = const Result::<(), ()>::Ok(());
_3 = const ControlFlow::<Result<Infallible, ()>, Result<(), ()>>::Continue(Result::<(), ()>::Ok(()));
StorageDead(_8);
StorageDead(_4);
_5 = const 0_isize;
goto -> bb1;
}

bb9: {
_1 = const ControlFlow::<Result<Infallible, ()>>::Break(Result::<Infallible, ()>::Err(()));
StorageDead(_9);
StorageDead(_2);
_7 = discriminant(_1);
- switchInt(move _7) -> [0: bb3, 1: bb4, otherwise: bb11];
+ goto -> bb4;
}

bb10: {
_1 = const ControlFlow::<Result<Infallible, ()>>::Continue(());
StorageDead(_9);
StorageDead(_2);
_7 = const 0_isize;
goto -> bb3;
}

bb11: {
unreachable;
}
}

Loading