Auto merge of rust-lang#123332 - Nadrieril:testkind-never, r=matthewjasper

never patterns: lower never patterns to `Unreachable` in MIR

This lowers a `!` pattern to "goto Unreachable". Ideally I'd like to read from the place to make it clear that the UB is coming from an invalid value, but that's tricky so I'm leaving it for later.

r? `@compiler-errors` how do you feel about a lil bit of MIR lowering

Author: bors

compiler/rustc_ast_lowering/src/expr.rs

@@ -581,8 +581,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 self.dcx().emit_err(NeverPatternWithGuard { span: g.span });
             }
 
-            // We add a fake `loop {}` arm body so that it typecks to `!`.
-            // FIXME(never_patterns): Desugar into a call to `unreachable_unchecked`.
+            // We add a fake `loop {}` arm body so that it typecks to `!`. The mir lowering of never
+            // patterns ensures this loop is not reachable.
             let block = self.arena.alloc(hir::Block {
                 stmts: &[],
                 expr: None,

compiler/rustc_middle/src/thir.rs

@@ -682,6 +682,23 @@ impl<'tcx> Pat<'tcx> {
             true
         })
     }
+
+    /// Whether this a never pattern.
+    pub fn is_never_pattern(&self) -> bool {
+        let mut is_never_pattern = false;
+        self.walk(|pat| match &pat.kind {
+            PatKind::Never => {
+                is_never_pattern = true;
+                false
+            }
+            PatKind::Or { pats } => {
+                is_never_pattern = pats.iter().all(|p| p.is_never_pattern());
+                false
+            }
+            _ => true,
+        });
+        is_never_pattern
+    }
 }
 
 impl<'tcx> IntoDiagArg for Pat<'tcx> {

compiler/rustc_mir_build/src/build/matches/mod.rs

@@ -1016,6 +1016,9 @@ struct PatternExtraData<'tcx> {
 
     /// Types that must be asserted.
     ascriptions: Vec<Ascription<'tcx>>,
+
+    /// Whether this corresponds to a never pattern.
+    is_never: bool,
 }
 
 impl<'tcx> PatternExtraData<'tcx> {
@@ -1041,12 +1044,14 @@ impl<'tcx, 'pat> FlatPat<'pat, 'tcx> {
         pattern: &'pat Pat<'tcx>,
         cx: &mut Builder<'_, 'tcx>,
     ) -> Self {
+        let is_never = pattern.is_never_pattern();
         let mut flat_pat = FlatPat {
             match_pairs: vec![MatchPair::new(place, pattern, cx)],
             extra_data: PatternExtraData {
                 span: pattern.span,
                 bindings: Vec::new(),
                 ascriptions: Vec::new(),
+                is_never,
             },
         };
         cx.simplify_match_pairs(&mut flat_pat.match_pairs, &mut flat_pat.extra_data);
@@ -1062,6 +1067,8 @@ struct Candidate<'pat, 'tcx> {
     match_pairs: Vec<MatchPair<'pat, 'tcx>>,
 
     /// ...and if this is non-empty, one of these subcandidates also has to match...
+    // Invariant: at the end of the algorithm, this must never contain a `is_never` candidate
+    // because that would break binding consistency.
     subcandidates: Vec<Candidate<'pat, 'tcx>>,
 
     /// ...and the guard must be evaluated if there is one.
@@ -1172,6 +1179,7 @@ enum TestCase<'pat, 'tcx> {
     Range(&'pat PatRange<'tcx>),
     Slice { len: usize, variable_length: bool },
     Deref { temp: Place<'tcx>, mutability: Mutability },
+    Never,
     Or { pats: Box<[FlatPat<'pat, 'tcx>]> },
 }
 
@@ -1238,6 +1246,9 @@ enum TestKind<'tcx> {
         temp: Place<'tcx>,
         mutability: Mutability,
     },
+
+    /// Assert unreachability of never patterns.
+    Never,
 }
 
 /// A test to perform to determine which [`Candidate`] matches a value.
@@ -1662,6 +1673,27 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 self.cfg.goto(or_block, source_info, any_matches);
             }
             candidate.pre_binding_block = Some(any_matches);
+        } else {
+            // Never subcandidates may have a set of bindings inconsistent with their siblings,
+            // which would break later code. So we filter them out. Note that we can't filter out
+            // top-level candidates this way.
+            candidate.subcandidates.retain_mut(|candidate| {
+                if candidate.extra_data.is_never {
+                    candidate.visit_leaves(|subcandidate| {
+                        let block = subcandidate.pre_binding_block.unwrap();
+                        // That block is already unreachable but needs a terminator to make the MIR well-formed.
+                        let source_info = self.source_info(subcandidate.extra_data.span);
+                        self.cfg.terminate(block, source_info, TerminatorKind::Unreachable);
+                    });
+                    false
+                } else {
+                    true
+                }
+            });
+            if candidate.subcandidates.is_empty() {
+                // If `candidate` has become a leaf candidate, ensure it has a `pre_binding_block`.
+                candidate.pre_binding_block = Some(self.cfg.start_new_block());
+            }
         }
     }
 
@@ -2008,6 +2040,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             block = fresh_block;
         }
 
+        if candidate.extra_data.is_never {
+            // This arm has a dummy body, we don't need to generate code for it. `block` is already
+            // unreachable (except via false edge).
+            let source_info = self.source_info(candidate.extra_data.span);
+            self.cfg.terminate(block, source_info, TerminatorKind::Unreachable);
+            return self.cfg.start_new_block();
+        }
+
         self.ascribe_types(
             block,
             parent_data

compiler/rustc_mir_build/src/build/matches/test.rs

@@ -44,6 +44,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
             TestCase::Deref { temp, mutability } => TestKind::Deref { temp, mutability },
 
+            TestCase::Never => TestKind::Never,
+
             TestCase::Or { .. } => bug!("or-patterns should have already been handled"),
 
             TestCase::Irrefutable { .. } => span_bug!(
@@ -262,6 +264,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let target = target_block(TestBranch::Success);
                 self.call_deref(block, target, place, mutability, ty, temp, test.span);
             }
+
+            TestKind::Never => {
+                // Check that the place is initialized.
+                // FIXME(never_patterns): Also assert validity of the data at `place`.
+                self.cfg.push_fake_read(
+                    block,
+                    source_info,
+                    FakeReadCause::ForMatchedPlace(None),
+                    place,
+                );
+                // A never pattern is only allowed on an uninhabited type, so validity of the data
+                // implies unreachability.
+                self.cfg.terminate(block, source_info, TerminatorKind::Unreachable);
+            }
         }
     }
 
@@ -710,6 +726,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 Some(TestBranch::Success)
             }
 
+            (TestKind::Never, _) => {
+                fully_matched = true;
+                Some(TestBranch::Success)
+            }
+
             (
                 TestKind::Switch { .. }
                 | TestKind::SwitchInt { .. }

compiler/rustc_mir_build/src/build/matches/util.rs

@@ -124,7 +124,8 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
         let default_irrefutable = || TestCase::Irrefutable { binding: None, ascription: None };
         let mut subpairs = Vec::new();
         let test_case = match pattern.kind {
-            PatKind::Never | PatKind::Wild | PatKind::Error(_) => default_irrefutable(),
+            PatKind::Wild | PatKind::Error(_) => default_irrefutable(),
+
             PatKind::Or { ref pats } => TestCase::Or {
                 pats: pats.iter().map(|pat| FlatPat::new(place_builder.clone(), pat, cx)).collect(),
             },
@@ -260,6 +261,8 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
                 subpairs.push(MatchPair::new(PlaceBuilder::from(temp).deref(), subpattern, cx));
                 TestCase::Deref { temp, mutability }
             }
+
+            PatKind::Never => TestCase::Never,
         };
 
         MatchPair { place, test_case, subpairs, pattern }

tests/crashes/120421.rs

@@ -0,0 +1,41 @@
+// MIR for `opt1` after SimplifyCfg-initial
+
+fn opt1(_1: &Result<u32, Void>) -> &u32 {
+    debug res => _1;
+    let mut _0: &u32;
+    let mut _2: isize;
+    let _3: &u32;
+    let mut _4: !;
+    let mut _5: ();
+    scope 1 {
+        debug x => _3;
+    }
+
+    bb0: {
+        PlaceMention(_1);
+        _2 = discriminant((*_1));
+        switchInt(move _2) -> [0: bb2, 1: bb3, otherwise: bb1];
+    }
+
+    bb1: {
+        FakeRead(ForMatchedPlace(None), _1);
+        unreachable;
+    }
+
+    bb2: {
+        falseEdge -> [real: bb4, imaginary: bb3];
+    }
+
+    bb3: {
+        FakeRead(ForMatchedPlace(None), (((*_1) as Err).0: Void));
+        unreachable;
+    }
+
+    bb4: {
+        StorageLive(_3);
+        _3 = &(((*_1) as Ok).0: u32);
+        _0 = &(*_3);
+        StorageDead(_3);
+        return;
+    }
+}

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

@@ -0,0 +1,35 @@
+// MIR for `opt2` after SimplifyCfg-initial
+
+fn opt2(_1: &Result<u32, Void>) -> &u32 {
+    debug res => _1;
+    let mut _0: &u32;
+    let mut _2: isize;
+    let _3: &u32;
+    scope 1 {
+        debug x => _3;
+    }
+
+    bb0: {
+        PlaceMention(_1);
+        _2 = discriminant((*_1));
+        switchInt(move _2) -> [0: bb2, 1: bb3, otherwise: bb1];
+    }
+
+    bb1: {
+        FakeRead(ForMatchedPlace(None), _1);
+        unreachable;
+    }
+
+    bb2: {
+        StorageLive(_3);
+        _3 = &(((*_1) as Ok).0: u32);
+        _0 = &(*_3);
+        StorageDead(_3);
+        return;
+    }
+
+    bb3: {
+        FakeRead(ForMatchedPlace(None), (((*_1) as Err).0: Void));
+        unreachable;
+    }
+}

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

@@ -0,0 +1,35 @@
+// MIR for `opt3` after SimplifyCfg-initial
+
+fn opt3(_1: &Result<u32, Void>) -> &u32 {
+    debug res => _1;
+    let mut _0: &u32;
+    let mut _2: isize;
+    let _3: &u32;
+    scope 1 {
+        debug x => _3;
+    }
+
+    bb0: {
+        PlaceMention(_1);
+        _2 = discriminant((*_1));
+        switchInt(move _2) -> [0: bb3, 1: bb2, otherwise: bb1];
+    }
+
+    bb1: {
+        FakeRead(ForMatchedPlace(None), _1);
+        unreachable;
+    }
+
+    bb2: {
+        FakeRead(ForMatchedPlace(None), (((*_1) as Err).0: Void));
+        unreachable;
+    }
+
+    bb3: {
+        StorageLive(_3);
+        _3 = &(((*_1) as Ok).0: u32);
+        _0 = &(*_3);
+        StorageDead(_3);
+        return;
+    }
+}

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

@@ -0,0 +1,44 @@
+#![feature(never_patterns)]
+#![allow(incomplete_features)]
+
+enum Void {}
+
+// EMIT_MIR never_patterns.opt1.SimplifyCfg-initial.after.mir
+fn opt1(res: &Result<u32, Void>) -> &u32 {
+    // CHECK-LABEL: fn opt1(
+    // CHECK: bb0: {
+    // CHECK-NOT: {{bb.*}}: {
+    // CHECK: return;
+    match res {
+        Ok(x) => x,
+        Err(!),
+    }
+}
+
+// EMIT_MIR never_patterns.opt2.SimplifyCfg-initial.after.mir
+fn opt2(res: &Result<u32, Void>) -> &u32 {
+    // CHECK-LABEL: fn opt2(
+    // CHECK: bb0: {
+    // CHECK-NOT: {{bb.*}}: {
+    // CHECK: return;
+    match res {
+        Ok(x) | Err(!) => x,
+    }
+}
+
+// EMIT_MIR never_patterns.opt3.SimplifyCfg-initial.after.mir
+fn opt3(res: &Result<u32, Void>) -> &u32 {
+    // CHECK-LABEL: fn opt3(
+    // CHECK: bb0: {
+    // CHECK-NOT: {{bb.*}}: {
+    // CHECK: return;
+    match res {
+        Err(!) | Ok(x) => x,
+    }
+}
+
+fn main() {
+    assert_eq!(opt1(&Ok(0)), &0);
+    assert_eq!(opt2(&Ok(0)), &0);
+    assert_eq!(opt3(&Ok(0)), &0);
+}

tests/mir-opt/building/match/never_patterns.rs

@@ -1,3 +1,4 @@
+// Check that never patterns can't have bodies or guards.
 #![feature(never_patterns)]
 #![allow(incomplete_features)]