f8463896a9
Make operational semantics of pattern matching independent of crate and module
The question of "when does matching an enum against a pattern of one of its variants read its discriminant" is currently an underspecified part of the language, causing weird behavior around borrowck, drop order, and UB.
Of course, in the common cases, the discriminant must be read to distinguish the variant of the enum, but currently the following exceptions are implemented:
1. If the enum has only one variant, we currently skip the discriminant read.
- This has the advantage that single-variant enums behave the same way as structs in this regard.
- However, it means that if the discriminant exists in the layout, we can't say that this discriminant being invalid is UB. This makes me particularly uneasy in its interactions with niches – consider the following example ([playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=5904a6155cbdd39af4a2e7b1d32a9b1a)), where miri currently doesn't detect any UB (because the semantics don't specify any):
<details><summary>Example 1</summary>
```rust
#![allow(dead_code)]
use core::mem::{size_of, transmute};
#[repr(u8)]
enum Inner {
X(u8),
}
enum Outer {
A(Inner),
B(u8),
}
fn f(x: &Inner) {
match x {
Inner::X(v) => {
println!("{v}");
}
}
}
fn main() {
assert_eq!(size_of::<Inner>(), 2);
assert_eq!(size_of::<Outer>(), 2);
let x = Outer::B(42);
let y = &x;
f(unsafe { transmute(y) });
}
```
</details>
2. For the purpose of the above, enums with marked with `#[non_exhaustive]` are always considered to have multiple variants when observed from foreign crates, but the actual number of variants is considered in the current crate.
- This means that whether code has UB can depend on which crate it is in: https://github.com/rust-lang/rust/issues/147722
- In another case of `#[non_exhaustive]` affecting the runtime semantics, its presence or absence can change what gets captured by a closure, and by extension, the drop order: https://github.com/rust-lang/rust/issues/147722#issuecomment-3674554872
- Also at the above link, there is an example where removing `#[non_exhaustive]` can cause borrowck to suddenly start failing in another crate.
3. Moreover, we currently make a more specific check: we only read the discriminant if there is more than one *inhabited* variant in the enum.
- This means that the semantics can differ between `foo<!>`, and a copy of `foo` where `T` was manually replaced with `!`: rust-lang/rust#146803
- Moreover, due to the privacy rules for inhabitedness, it means that the semantics of code can depend on the *module* in which it is located.
- Additionally, this inhabitedness rule is even uglier due to the fact that closure capture analysis needs to happen before we can determine whether types are uninhabited, which means that whether the discriminant read happens has a different answer specifically for capture analysis.
- For the two above points, see the following example ([playground](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2024&gist=a07d8a3ec0b31953942e96e2130476d9)):
<details><summary>Example 2</summary>
```rust
#![allow(unused)]
mod foo {
enum Never {}
struct PrivatelyUninhabited(Never);
pub enum A {
V(String, String),
Y(PrivatelyUninhabited),
}
fn works(mut x: A) {
let a = match x {
A::V(ref mut a, _) => a,
_ => unreachable!(),
};
let b = match x {
A::V(_, ref mut b) => b,
_ => unreachable!(),
};
a.len(); b.len();
}
fn fails(mut x: A) {
let mut f = || match x {
A::V(ref mut a, _) => (),
_ => unreachable!(),
};
let mut g = || match x {
A::V(_, ref mut b) => (),
_ => unreachable!(),
};
f(); g();
}
}
use foo::A;
fn fails(mut x: A) {
let a = match x {
A::V(ref mut a, _) => a,
_ => unreachable!(),
};
let b = match x {
A::V(_, ref mut b) => b,
_ => unreachable!(),
};
a.len(); b.len();
}
fn fails2(mut x: A) {
let mut f = || match x {
A::V(ref mut a, _) => (),
_ => unreachable!(),
};
let mut g = || match x {
A::V(_, ref mut b) => (),
_ => unreachable!(),
};
f(); g();
}
```
</details>
In light of the above, and following the discussion at rust-lang/rust#138961 and rust-lang/rust#147722, this PR ~~makes it so that, operationally, matching on an enum *always* reads its discriminant.~~ introduces the following changes to this behavior:
- matching on a `#[non_exhaustive]` enum will always introduce a discriminant read, regardless of whether the enum is from an external crate
- uninhabited variants now count just like normal ones, and don't get skipped in the checks
As per the discussion below, the resolution for point (1) above is that it should land as part of a separate PR, so that the subtler decision can be more carefully considered.
Note that this is a breaking change, due to the aforementioned changes in borrow checking behavior, new UB (or at least UB newly detected by miri), as well as drop order around closure captures. However, it seems to me that the combination of this PR with rust-lang/rust#138961 should have smaller real-world impact than rust-lang/rust#138961 by itself.
Fixes rust-lang/rust#142394
Fixes rust-lang/rust#146590
Fixes rust-lang/rust#146803 (though already marked as duplicate)
Fixes parts of rust-lang/rust#147722
Fixes rust-lang/miri#4778
r? @Nadrieril @RalfJung
@rustbot label +A-closures +A-patterns +T-opsem +T-lang
|
||
|---|---|---|
| .. | ||
| chained_comparison.bitand.PreCodegen.after.mir | ||
| chained_comparison.naive.PreCodegen.after.mir | ||
| chained_comparison.returning.PreCodegen.after.mir | ||
| chained_comparison.rs | ||
| checked_ops.checked_shl.PreCodegen.after.panic-abort.mir | ||
| checked_ops.checked_shl.PreCodegen.after.panic-unwind.mir | ||
| checked_ops.rs | ||
| checked_ops.saturating_sub_at_home.PreCodegen.after.panic-abort.mir | ||
| checked_ops.saturating_sub_at_home.PreCodegen.after.panic-unwind.mir | ||
| checked_ops.step_forward.PreCodegen.after.panic-abort.mir | ||
| checked_ops.step_forward.PreCodegen.after.panic-unwind.mir | ||
| checked_ops.use_checked_sub.PreCodegen.after.panic-abort.mir | ||
| checked_ops.use_checked_sub.PreCodegen.after.panic-unwind.mir | ||
| clone_as_copy.clone_as_copy.PreCodegen.after.mir | ||
| clone_as_copy.enum_clone_as_copy.PreCodegen.after.mir | ||
| clone_as_copy.rs | ||
| const_promotion_option_ordering_eq.direct.PreCodegen.after.mir | ||
| const_promotion_option_ordering_eq.rs | ||
| const_promotion_option_ordering_eq.with_let.PreCodegen.after.mir | ||
| dead_on_invalid_place.invalid_place.PreCodegen.after.mir | ||
| dead_on_invalid_place.rs | ||
| deref_nested_borrows.rs | ||
| deref_nested_borrows.src.GVN.panic-abort.diff | ||
| deref_nested_borrows.src.GVN.panic-unwind.diff | ||
| deref_nested_borrows.src.PreCodegen.after.panic-abort.mir | ||
| deref_nested_borrows.src.PreCodegen.after.panic-unwind.mir | ||
| derived_ord.demo_le.PreCodegen.after.mir | ||
| derived_ord.rs | ||
| derived_ord.{impl#0}-partial_cmp.PreCodegen.after.mir | ||
| derived_ord_debug.rs | ||
| derived_ord_debug.{impl#0}-partial_cmp.PreCodegen.after.panic-abort.mir | ||
| derived_ord_debug.{impl#0}-partial_cmp.PreCodegen.after.panic-unwind.mir | ||
| derived_ord_debug.{impl#1}-cmp.PreCodegen.after.panic-abort.mir | ||
| derived_ord_debug.{impl#1}-cmp.PreCodegen.after.panic-unwind.mir | ||
| drop_boxed_slice.generic_in_place.PreCodegen.after.32bit.panic-abort.mir | ||
| drop_boxed_slice.generic_in_place.PreCodegen.after.32bit.panic-unwind.mir | ||
| drop_boxed_slice.generic_in_place.PreCodegen.after.64bit.panic-abort.mir | ||
| drop_boxed_slice.generic_in_place.PreCodegen.after.64bit.panic-unwind.mir | ||
| drop_boxed_slice.rs | ||
| duplicate_switch_targets.rs | ||
| duplicate_switch_targets.ub_if_b.PreCodegen.after.mir | ||
| intrinsics.f_u64.PreCodegen.after.mir | ||
| intrinsics.f_unit.PreCodegen.after.mir | ||
| intrinsics.rs | ||
| issue_117368_print_invalid_constant.main.GVN.32bit.panic-abort.diff | ||
| issue_117368_print_invalid_constant.main.GVN.32bit.panic-unwind.diff | ||
| issue_117368_print_invalid_constant.main.GVN.64bit.panic-abort.diff | ||
| issue_117368_print_invalid_constant.main.GVN.64bit.panic-unwind.diff | ||
| issue_117368_print_invalid_constant.rs | ||
| loops.filter_mapped.PreCodegen.after.mir | ||
| loops.int_range.PreCodegen.after.mir | ||
| loops.mapped.PreCodegen.after.mir | ||
| loops.rs | ||
| loops.vec_move.PreCodegen.after.mir | ||
| matchbr.match1.PreCodegen.after.mir | ||
| matchbr.rs | ||
| matches_macro.issue_77355_opt.PreCodegen.after.mir | ||
| matches_macro.rs | ||
| mem_replace.manual_replace.PreCodegen.after.panic-abort.mir | ||
| mem_replace.manual_replace.PreCodegen.after.panic-unwind.mir | ||
| mem_replace.mem_replace.PreCodegen.after.panic-abort.mir | ||
| mem_replace.mem_replace.PreCodegen.after.panic-unwind.mir | ||
| mem_replace.rs | ||
| no_inlined_clone.rs | ||
| no_inlined_clone.{impl#0}-clone.PreCodegen.after.mir | ||
| optimizes_into_variable.main.GVN.32bit.panic-abort.diff | ||
| optimizes_into_variable.main.GVN.32bit.panic-unwind.diff | ||
| optimizes_into_variable.main.GVN.64bit.panic-abort.diff | ||
| optimizes_into_variable.main.GVN.64bit.panic-unwind.diff | ||
| optimizes_into_variable.main.PreCodegen.after.32bit.panic-abort.mir | ||
| optimizes_into_variable.main.PreCodegen.after.32bit.panic-unwind.mir | ||
| optimizes_into_variable.main.PreCodegen.after.64bit.panic-abort.mir | ||
| optimizes_into_variable.main.PreCodegen.after.64bit.panic-unwind.mir | ||
| optimizes_into_variable.main.ScalarReplacementOfAggregates.32bit.panic-abort.diff | ||
| optimizes_into_variable.main.ScalarReplacementOfAggregates.32bit.panic-unwind.diff | ||
| optimizes_into_variable.main.ScalarReplacementOfAggregates.64bit.panic-abort.diff | ||
| optimizes_into_variable.main.ScalarReplacementOfAggregates.64bit.panic-unwind.diff | ||
| optimizes_into_variable.main.SimplifyLocals-final.after.32bit.panic-abort.mir | ||
| optimizes_into_variable.main.SimplifyLocals-final.after.32bit.panic-unwind.mir | ||
| optimizes_into_variable.main.SimplifyLocals-final.after.64bit.panic-abort.mir | ||
| optimizes_into_variable.main.SimplifyLocals-final.after.64bit.panic-unwind.mir | ||
| optimizes_into_variable.rs | ||
| option_bubble_debug.option_direct.PreCodegen.after.panic-abort.mir | ||
| option_bubble_debug.option_direct.PreCodegen.after.panic-unwind.mir | ||
| option_bubble_debug.option_traits.PreCodegen.after.panic-abort.mir | ||
| option_bubble_debug.option_traits.PreCodegen.after.panic-unwind.mir | ||
| option_bubble_debug.rs | ||
| ptr_offset.demo_byte_add_fat.PreCodegen.after.panic-abort.mir | ||
| ptr_offset.demo_byte_add_fat.PreCodegen.after.panic-unwind.mir | ||
| ptr_offset.demo_byte_add_thin.PreCodegen.after.panic-abort.mir | ||
| ptr_offset.demo_byte_add_thin.PreCodegen.after.panic-unwind.mir | ||
| ptr_offset.rs | ||
| range_iter.forward_loop.PreCodegen.after.panic-abort.mir | ||
| range_iter.forward_loop.PreCodegen.after.panic-unwind.mir | ||
| range_iter.inclusive_loop.PreCodegen.after.panic-abort.mir | ||
| range_iter.inclusive_loop.PreCodegen.after.panic-unwind.mir | ||
| range_iter.range_inclusive_iter_next.PreCodegen.after.panic-abort.mir | ||
| range_iter.range_inclusive_iter_next.PreCodegen.after.panic-unwind.mir | ||
| range_iter.range_iter_next.PreCodegen.after.panic-abort.mir | ||
| range_iter.range_iter_next.PreCodegen.after.panic-unwind.mir | ||
| range_iter.rs | ||
| README.md | ||
| simple_option_map.ezmap.PreCodegen.after.mir | ||
| simple_option_map.map_via_question_mark.PreCodegen.after.mir | ||
| simple_option_map.rs | ||
| slice_filter.rs | ||
| slice_filter.variant_a-{closure#0}.PreCodegen.after.mir | ||
| slice_filter.variant_b-{closure#0}.PreCodegen.after.mir | ||
| slice_index.rs | ||
| slice_index.slice_get_mut_usize.PreCodegen.after.panic-abort.mir | ||
| slice_index.slice_get_mut_usize.PreCodegen.after.panic-unwind.mir | ||
| slice_index.slice_get_unchecked_mut_range.PreCodegen.after.panic-abort.mir | ||
| slice_index.slice_get_unchecked_mut_range.PreCodegen.after.panic-unwind.mir | ||
| slice_index.slice_index_range.PreCodegen.after.panic-abort.mir | ||
| slice_index.slice_index_range.PreCodegen.after.panic-unwind.mir | ||
| slice_index.slice_index_usize.PreCodegen.after.panic-abort.mir | ||
| slice_index.slice_index_usize.PreCodegen.after.panic-unwind.mir | ||
| slice_index.slice_ptr_get_unchecked_range.PreCodegen.after.panic-abort.mir | ||
| slice_index.slice_ptr_get_unchecked_range.PreCodegen.after.panic-unwind.mir | ||
| slice_iter.enumerated_loop.PreCodegen.after.panic-abort.mir | ||
| slice_iter.enumerated_loop.PreCodegen.after.panic-unwind.mir | ||
| slice_iter.forward_loop.PreCodegen.after.panic-abort.mir | ||
| slice_iter.forward_loop.PreCodegen.after.panic-unwind.mir | ||
| slice_iter.range_loop.PreCodegen.after.panic-abort.mir | ||
| slice_iter.range_loop.PreCodegen.after.panic-unwind.mir | ||
| slice_iter.reverse_loop.PreCodegen.after.panic-abort.mir | ||
| slice_iter.reverse_loop.PreCodegen.after.panic-unwind.mir | ||
| slice_iter.rs | ||
| slice_iter.slice_iter_generic_is_empty.PreCodegen.after.panic-abort.mir | ||
| slice_iter.slice_iter_generic_is_empty.PreCodegen.after.panic-unwind.mir | ||
| slice_iter.slice_iter_mut_next_back.PreCodegen.after.panic-abort.mir | ||
| slice_iter.slice_iter_mut_next_back.PreCodegen.after.panic-unwind.mir | ||
| slice_iter.slice_iter_next.PreCodegen.after.panic-abort.mir | ||
| slice_iter.slice_iter_next.PreCodegen.after.panic-unwind.mir | ||
| spans.outer.PreCodegen.after.panic-abort.mir | ||
| spans.outer.PreCodegen.after.panic-unwind.mir | ||
| spans.rs | ||
| try_identity.new.PreCodegen.after.mir | ||
| try_identity.old.PreCodegen.after.mir | ||
| try_identity.rs | ||
| tuple_ord.demo_ge_partial.PreCodegen.after.mir | ||
| tuple_ord.demo_le_total.PreCodegen.after.mir | ||
| tuple_ord.rs | ||
| two_unwrap_unchecked.rs | ||
| two_unwrap_unchecked.two_unwrap_unchecked.GVN.diff | ||
| two_unwrap_unchecked.two_unwrap_unchecked.PreCodegen.after.mir | ||
| vec_deref.rs | ||
| vec_deref.vec_deref_to_slice.PreCodegen.after.panic-abort.mir | ||
| vec_deref.vec_deref_to_slice.PreCodegen.after.panic-unwind.mir | ||
The goal of this directory is to track the quality of MIR that is given to codegen in a standard -O configuration.
As such, feel free to --bless whatever changes you get here, so long as doing so doesn't add substantially more MIR.