Make operational semantics of pattern matching independent of crate and module

compiler/rustc_hir_typeck/src/expr_use_visitor.rs

@@ -818,14 +818,12 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx
     /// The core driver for walking a pattern
     ///
     /// This should mirror how pattern-matching gets lowered to MIR, as
-    /// otherwise lowering will ICE when trying to resolve the upvars.
+    /// otherwise said lowering will ICE when trying to resolve the upvars.
     ///
     /// However, it is okay to approximate it here by doing *more* accesses than
     /// the actual MIR builder will, which is useful when some checks are too
-    /// cumbersome to perform here. For example, if after typeck it becomes
-    /// clear that only one variant of an enum is inhabited, and therefore a
-    /// read of the discriminant is not necessary, `walk_pat` will have
-    /// over-approximated the necessary upvar capture granularity.
+    /// cumbersome to perform here, because e.g. they require more typeck results
+    /// than available.
     ///
     /// Do note that discrepancies like these do still create obscure corners
     /// in the semantics of the language, and should be avoided if possible.
@@ -1852,26 +1850,13 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx
     }
 
     /// Checks whether a type has multiple variants, and therefore, whether a
-    /// read of the discriminant might be necessary. Note that the actual MIR
-    /// builder code does a more specific check, filtering out variants that
-    /// happen to be uninhabited.
-    ///
-    /// Here, it is not practical to perform such a check, because inhabitedness
-    /// queries require typeck results, and typeck requires closure capture analysis.
-    ///
-    /// Moreover, the language is moving towards uninhabited variants still semantically
-    /// causing a discriminant read, so we *shouldn't* perform any such check.
-    ///
-    /// FIXME(never_patterns): update this comment once the aforementioned MIR builder
-    /// code is changed to be insensitive to inhhabitedness.
+    /// read of the discriminant might be necessary.
     #[instrument(skip(self, span), level = "debug")]
     fn is_multivariant_adt(&self, ty: Ty<'tcx>, span: Span) -> bool {
         if let ty::Adt(def, _) = self.cx.structurally_resolve_type(span, ty).kind() {
-            // Note that if a non-exhaustive SingleVariant is defined in another crate, we need
-            // to assume that more cases will be added to the variant in the future. This mean
-            // that we should handle non-exhaustive SingleVariant the same way we would handle
-            // a MultiVariant.
-            def.variants().len() > 1 || def.variant_list_has_applicable_non_exhaustive()
+            // We treat non-exhaustive enums the same independent of the crate they are
+            // defined in, to avoid differences in the operational semantics between crates.
+            def.variants().len() > 1 || def.is_variant_list_non_exhaustive()
         } else {
             false
         }

compiler/rustc_mir_build/src/builder/matches/match_pair.rs

@@ -295,22 +295,18 @@ impl<'tcx> MatchPairTree<'tcx> {
                 }
             }
 
-            PatKind::Variant { adt_def, variant_index, args, ref subpatterns } => {
+            PatKind::Variant { adt_def, variant_index, args: _, ref subpatterns } => {
                 let downcast_place = place_builder.downcast(adt_def, variant_index); // `(x as Variant)`
                 cx.field_match_pairs(&mut subpairs, extra_data, downcast_place, subpatterns);
 
-                let irrefutable = adt_def.variants().iter_enumerated().all(|(i, v)| {
-                    i == variant_index
-                        || !v.inhabited_predicate(cx.tcx, adt_def).instantiate(cx.tcx, args).apply(
-                            cx.tcx,
-                            cx.infcx.typing_env(cx.param_env),
-                            cx.def_id.into(),
-                        )
-                }) && !adt_def.variant_list_has_applicable_non_exhaustive();
-                if irrefutable {
-                    None
-                } else {
+                // We treat non-exhaustive enums the same independent of the crate they are
+                // defined in, to avoid differences in the operational semantics between crates.
+                let refutable =
+                    adt_def.variants().len() > 1 || adt_def.is_variant_list_non_exhaustive();
+                if refutable {
                     Some(TestableCase::Variant { adt_def, variant_index })
+                } else {
+                    None
                 }
             }
 

src/tools/miri/tests/fail/match/all_variants_uninhabited.rs

@@ -0,0 +1,25 @@
+#![allow(deref_nullptr)]
+
+enum Never {}
+
+fn main() {
+    unsafe {
+        match *std::ptr::null::<Result<Never, Never>>() {
+        //~^ ERROR: read discriminant of an uninhabited enum variant
+            Ok(_) => {
+                lol();
+            }
+            Err(_) => {
+                wut();
+            }
+        }
+    }
+}
+
+fn lol() {
+    println!("lol");
+}
+
+fn wut() {
+    println!("wut");
+}

src/tools/miri/tests/fail/match/all_variants_uninhabited.stderr

@@ -0,0 +1,13 @@
+error: Undefined Behavior: read discriminant of an uninhabited enum variant
+  --> tests/fail/match/all_variants_uninhabited.rs:LL:CC
+   |
+LL |         match *std::ptr::null::<Result<Never, Never>>() {
+   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

src/tools/miri/tests/fail/closures/deref-in-pattern.stderr → src/tools/miri/tests/fail/match/closures/deref-in-pattern.stderr

@@ -1,5 +1,5 @@
 error: Undefined Behavior: constructing invalid value: encountered a dangling reference (use-after-free)
-  --> tests/fail/closures/deref-in-pattern.rs:LL:CC
+  --> tests/fail/match/closures/deref-in-pattern.rs:LL:CC
    |
 LL |       let _ = || {
    |  _____________^

src/tools/miri/tests/fail/closures/partial-pattern.stderr → src/tools/miri/tests/fail/match/closures/partial-pattern.stderr

@@ -1,5 +1,5 @@
 error: Undefined Behavior: constructing invalid value: encountered a dangling reference (use-after-free)
-  --> tests/fail/closures/partial-pattern.rs:LL:CC
+  --> tests/fail/match/closures/partial-pattern.rs:LL:CC
    |
 LL |       let _ = || {
    |  _____________^

src/tools/miri/tests/fail/closures/uninhabited-variant.stderr → src/tools/miri/tests/fail/match/closures/uninhabited-variant1.stderr

@@ -1,5 +1,5 @@
 error: Undefined Behavior: read discriminant of an uninhabited enum variant
-  --> tests/fail/closures/uninhabited-variant.rs:LL:CC
+  --> tests/fail/match/closures/uninhabited-variant1.rs:LL:CC
    |
 LL |         match r {
    |               ^ Undefined Behavior occurred here
@@ -8,9 +8,9 @@ LL |         match r {
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
    = note: stack backtrace:
            0: main::{closure#0}
-               at tests/fail/closures/uninhabited-variant.rs:LL:CC
+               at tests/fail/match/closures/uninhabited-variant1.rs:LL:CC
            1: main
-               at tests/fail/closures/uninhabited-variant.rs:LL:CC
+               at tests/fail/match/closures/uninhabited-variant1.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
 

src/tools/miri/tests/fail/match/closures/uninhabited-variant2.rs

@@ -0,0 +1,32 @@
+// Motivated by rust-lang/rust#138961, this shows how invalid discriminants interact with
+// closure captures.
+//
+// Test case with only one inhabited variant, for which rustc used to not emit
+// a discriminant read in the first place. See: rust-lang/miri#4778
+#![feature(never_type)]
+
+#[repr(C)]
+#[allow(dead_code)]
+enum E {
+    V0,    // discriminant: 0
+    V1(!), // 1
+}
+
+fn main() {
+    assert_eq!(std::mem::size_of::<E>(), 4);
+
+    let val = 1u32;
+    let ptr = (&raw const val).cast::<E>();
+    let r = unsafe { &*ptr };
+    let f = || {
+        // After rust-lang/rust#138961, constructing the closure performs a reborrow of r.
+        // Nevertheless, the discriminant is only actually inspected when the closure
+        // is called.
+        match r { //~ ERROR: read discriminant of an uninhabited enum variant
+            E::V0 => {}
+            E::V1(_) => {}
+        }
+    };
+
+    f();
+}

src/tools/miri/tests/fail/match/closures/uninhabited-variant2.stderr

@@ -0,0 +1,18 @@
+error: Undefined Behavior: read discriminant of an uninhabited enum variant
+  --> tests/fail/match/closures/uninhabited-variant2.rs:LL:CC
+   |
+LL |         match r {
+   |               ^ Undefined Behavior occurred here
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: stack backtrace:
+           0: main::{closure#0}
+               at tests/fail/match/closures/uninhabited-variant2.rs:LL:CC
+           1: main
+               at tests/fail/match/closures/uninhabited-variant2.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

src/tools/miri/tests/fail/match/only_inhabited_variant.rs

@@ -0,0 +1,21 @@
+// rust-lang/miri#4778
+#![feature(never_type)]
+
+#[repr(C)]
+#[allow(dead_code)]
+enum E {
+  V0, // discriminant: 0
+  V1(!), // 1
+}
+
+fn main() {
+    assert_eq!(std::mem::size_of::<E>(), 4);
+
+    let val = 1u32;
+    let ptr = (&raw const val).cast::<E>();
+    let r = unsafe { &*ptr };
+    match r { //~ ERROR: read discriminant of an uninhabited enum variant
+        E::V0 => {}
+        E::V1(_) => {}
+    }
+}

src/tools/miri/tests/fail/match/only_inhabited_variant.stderr

@@ -0,0 +1,13 @@
+error: Undefined Behavior: read discriminant of an uninhabited enum variant
+  --> tests/fail/match/only_inhabited_variant.rs:LL:CC
+   |
+LL |     match r {
+   |           ^ Undefined Behavior occurred here
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

src/tools/miri/tests/fail/match/single_variant.rs

@@ -0,0 +1,31 @@
+// Ideally, this would be UB regardless of #[non_exhaustive]. For now,
+// at least the semantics don't depend on the crate you're in.
+//
+// See: rust-lang/rust#147722
+#![allow(dead_code)]
+
+#[repr(u8)]
+enum Exhaustive {
+    A(u8) = 42,
+}
+
+#[repr(u8)]
+#[non_exhaustive]
+enum NonExhaustive {
+    A(u8) = 42,
+}
+
+fn main() {
+    unsafe {
+        let x: &[u8; 2] = &[21, 37];
+        let y: &Exhaustive = std::mem::transmute(x);
+        match y {
+            Exhaustive::A(_) => {},
+        }
+
+        let y: &NonExhaustive = std::mem::transmute(x);
+        match y { //~ ERROR: enum value has invalid tag
+            NonExhaustive::A(_) => {},
+        }
+    }
+}

src/tools/miri/tests/fail/match/single_variant.stderr

@@ -0,0 +1,13 @@
+error: Undefined Behavior: enum value has invalid tag: 0x15
+  --> tests/fail/match/single_variant.rs:LL:CC
+   |
+LL |         match y {
+   |               ^ Undefined Behavior occurred here
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

src/tools/miri/tests/fail/match/single_variant_uninit.rs

@@ -0,0 +1,36 @@
+// Ideally, this would be UB regardless of #[non_exhaustive]. For now,
+// at least the semantics don't depend on the crate you're in.
+//
+// See: rust-lang/rust#147722
+#![allow(dead_code)]
+#![allow(unreachable_patterns)]
+
+#[repr(u8)]
+enum Exhaustive {
+    A(u8) = 0,
+}
+
+#[repr(u8)]
+#[non_exhaustive]
+enum NonExhaustive {
+    A(u8) = 0,
+}
+
+use std::mem::MaybeUninit;
+
+fn main() {
+    let buffer: [MaybeUninit<u8>; 2] = [MaybeUninit::uninit(), MaybeUninit::new(0u8)];
+    let exh: *const Exhaustive = (&raw const buffer).cast();
+    let nexh: *const NonExhaustive = (&raw const buffer).cast();
+    unsafe {
+        match *exh {
+            Exhaustive::A(ref _val) => {}
+            _ => {}
+        }
+
+        match *nexh { //~ ERROR: memory is uninitialized
+            NonExhaustive::A(ref _val) => {}
+            _ => {}
+        }
+    }
+}

src/tools/miri/tests/fail/match/single_variant_uninit.stderr

@@ -0,0 +1,18 @@
+error: Undefined Behavior: reading memory at ALLOC[0x0..0x1], but memory is uninitialized at [0x0..0x1], and this operation requires initialized memory
+  --> tests/fail/match/single_variant_uninit.rs:LL:CC
+   |
+LL |         match *nexh {
+   |               ^^^^^ Undefined Behavior occurred here
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+
+Uninitialized memory occurred at ALLOC[0x0..0x1], in this allocation:
+ALLOC (stack variable, size: 2, align: 1) {
+    __ 00                                           │ ░.
+}
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

tests/codegen-llvm/enum/enum-transparent-extract.rs

@@ -11,6 +11,8 @@ pub enum Never {}
 pub fn make_unmake_result_never(x: i32) -> i32 {
     // CHECK-LABEL: define i32 @make_unmake_result_never(i32{{( signext)?}} %x)
     // CHECK: start:
+    // CHECK-NEXT: br label %[[next:bb.*]]
+    // CHECK: [[next]]:
     // CHECK-NEXT: ret i32 %x
 
     let y: Result<i32, Never> = Ok(x);
@@ -22,6 +24,8 @@ pub fn make_unmake_result_never(x: i32) -> i32 {
 pub fn extract_control_flow_never(x: ControlFlow<&str, Never>) -> &str {
     // CHECK-LABEL: define { ptr, i64 } @extract_control_flow_never(ptr align 1 %x.0, i64 %x.1)
     // CHECK: start:
+    // CHECK-NEXT: br label %[[next:bb.*]]
+    // CHECK: [[next]]:
     // CHECK-NEXT: %[[P0:.+]] = insertvalue { ptr, i64 } poison, ptr %x.0, 0
     // CHECK-NEXT: %[[P1:.+]] = insertvalue { ptr, i64 } %[[P0]], i64 %x.1, 1
     // CHECK-NEXT: ret { ptr, i64 } %[[P1]]

tests/mir-opt/building/match/never_patterns.opt1.SimplifyCfg-initial.after.mir

@@ -13,17 +13,17 @@ fn opt1(_1: &Result<u32, Void>) -> &u32 {
 
     bb0: {
         PlaceMention(_1);
-        falseEdge -> [real: bb4, imaginary: bb1];
+        _2 = discriminant((*_1));
+        switchInt(move _2) -> [0: bb2, 1: bb3, otherwise: bb1];
     }
 
     bb1: {
-        _2 = discriminant((*_1));
-        switchInt(move _2) -> [1: bb3, otherwise: bb2];
+        FakeRead(ForMatchedPlace(None), _1);
+        unreachable;
     }
 
     bb2: {
-        FakeRead(ForMatchedPlace(None), _1);
-        unreachable;
+        falseEdge -> [real: bb4, imaginary: bb3];
     }
 
     bb3: {