Review

Nadrieril · Nadrieril · commit bc31c3987f0e · 2023-11-29T03:07:03.000+01:00
diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs
@@ -178,7 +178,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         let ty = match pat.kind {
             PatKind::Wild => expected,
-            PatKind::Never => expected, // FIXME(never_patterns): check the type is uninhabited
+            // FIXME(never_patterns): check the type is uninhabited. If that is not possible within
+            // typeck, do that in a later phase.
+            PatKind::Never => expected,
             PatKind::Lit(lt) => self.check_pat_lit(pat.span, lt, expected, ti),
             PatKind::Range(lhs, rhs, _) => self.check_pat_range(pat.span, lhs, rhs, expected, ti),
             PatKind::Binding(ba, var_id, _, sub) => {
@@ -288,6 +290,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             | PatKind::Box(_)
             | PatKind::Range(..)
             | PatKind::Slice(..) => AdjustMode::Peel,
+            // A never pattern behaves somewhat like a literal or unit variant.
+            PatKind::Never => AdjustMode::Peel,
             // String and byte-string literals result in types `&str` and `&[u8]` respectively.
             // All other literals result in non-reference types.
             // As a result, we allow `if let 0 = &&0 {}` but not `if let "foo" = &&"foo" {}`.
@@ -308,8 +312,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 // a reference type wherefore peeling doesn't give up any expressiveness.
                 _ => AdjustMode::Peel,
             },
-            // A never pattern behaves somewhat like a literal or unit variant.
-            PatKind::Never => AdjustMode::Peel,
             // When encountering a `& mut? pat` pattern, reset to "by value".
             // This is so that `x` and `y` here are by value, as they appear to be:
             //
diff --git a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs
@@ -1558,7 +1558,9 @@ impl<'p, 'tcx> DeconstructedPat<'p, 'tcx> {
                 fields = Fields::from_iter(cx, pats.into_iter().map(mkpat));
             }
             PatKind::Never => {
-                ctor = Wildcard; // FIXME(never_patterns)
+                // FIXME(never_patterns): handle `!` in exhaustiveness. This is a sane default
+                // in the meantime.
+                ctor = Wildcard;
                 fields = Fields::empty();
             }
             PatKind::Error(_) => {
diff --git a/src/tools/rustfmt/src/patterns.rs b/src/tools/rustfmt/src/patterns.rs
@@ -40,21 +40,23 @@ pub(crate) fn is_short_pattern(pat: &ast::Pat, pat_str: &str) -> bool {
 
 fn is_short_pattern_inner(pat: &ast::Pat) -> bool {
     match pat.kind {
-        PatKind::Rest | PatKind::Never | PatKind::Wild | PatKind::Lit(_) => true,
-        PatKind::Ident(_, _, ref pat) => pat.is_none(),
-        PatKind::Struct(..)
-        | PatKind::MacCall(..)
-        | PatKind::Slice(..)
-        | PatKind::Path(..)
-        | PatKind::Range(..) => false,
-        PatKind::Tuple(ref subpats) => subpats.len() <= 1,
-        PatKind::TupleStruct(_, ref path, ref subpats) => {
+        ast::PatKind::Rest | ast::PatKind::Never | ast::PatKind::Wild | ast::PatKind::Lit(_) => {
+            true
+        }
+        ast::PatKind::Ident(_, _, ref pat) => pat.is_none(),
+        ast::PatKind::Struct(..)
+        | ast::PatKind::MacCall(..)
+        | ast::PatKind::Slice(..)
+        | ast::PatKind::Path(..)
+        | ast::PatKind::Range(..) => false,
+        ast::PatKind::Tuple(ref subpats) => subpats.len() <= 1,
+        ast::PatKind::TupleStruct(_, ref path, ref subpats) => {
             path.segments.len() <= 1 && subpats.len() <= 1
         }
-        PatKind::Box(ref p) | PatKind::Ref(ref p, _) | PatKind::Paren(ref p) => {
+        ast::PatKind::Box(ref p) | ast::PatKind::Ref(ref p, _) | ast::PatKind::Paren(ref p) => {
             is_short_pattern_inner(&*p)
         }
-        PatKind::Or(ref pats) => pats.iter().all(|p| is_short_pattern_inner(p)),
+        ast::PatKind::Or(ref pats) => pats.iter().all(|p| is_short_pattern_inner(p)),
     }
 }
 
@@ -193,13 +195,7 @@ impl Rewrite for Pat {
                     None
                 }
             }
-            PatKind::Never => {
-                if 1 <= shape.width {
-                    Some("!".to_owned())
-                } else {
-                    None
-                }
-            }
+            PatKind::Never => None,
             PatKind::Range(ref lhs, ref rhs, ref end_kind) => {
                 let infix = match end_kind.node {
                     RangeEnd::Included(RangeSyntax::DotDotDot) => "...",
diff --git a/tests/ui/feature-gates/feature-gate-never_patterns.rs b/tests/ui/feature-gates/feature-gate-never_patterns.rs
@@ -15,4 +15,13 @@ fn main() {
             ! => {} //~ ERROR `!` patterns are experimental
         }
     }
+
+    // Check that the gate operates even behind `cfg`.
+    #[cfg(FALSE)]
+    unsafe {
+        let ptr: *const Void = NonNull::dangling().as_ptr();
+        match *ptr {
+            ! => {} //~ ERROR `!` patterns are experimental
+        }
+    }
 }
diff --git a/tests/ui/feature-gates/feature-gate-never_patterns.stderr b/tests/ui/feature-gates/feature-gate-never_patterns.stderr
@@ -24,7 +24,16 @@ LL |             ! => {}
    = note: see issue #118155 <https://github.com/rust-lang/rust/issues/118155> for more information
    = help: add `#![feature(never_patterns)]` to the crate attributes to enable
 
-error: aborting due to 3 previous errors
+error[E0658]: `!` patterns are experimental
+  --> $DIR/feature-gate-never_patterns.rs:24:13
+   |
+LL |             ! => {}
+   |             ^
+   |
+   = note: see issue #118155 <https://github.com/rust-lang/rust/issues/118155> for more information
+   = help: add `#![feature(never_patterns)]` to the crate attributes to enable
+
+error: aborting due to 4 previous errors
 
 Some errors have detailed explanations: E0408, E0658.
 For more information about an error, try `rustc --explain E0408`.
diff --git a/tests/ui/pattern/never_patterns.rs b/tests/ui/pattern/never_patterns.rs
@@ -8,45 +8,59 @@ fn main() {}
 // The classic use for empty types.
 fn safe_unwrap_result<T>(res: Result<T, Void>) {
     let Ok(_x) = res;
+    // FIXME(never_patterns): These should be allowed
     let (Ok(_x) | Err(!)) = &res;
     //~^ ERROR: is not bound in all patterns
     let (Ok(_x) | Err(&!)) = res.as_ref();
     //~^ ERROR: is not bound in all patterns
 }
 
-// Check we only accept `!` where we want.
-fn never_pattern_location() {
-    // Don't accept on a non-empty type.
+// Check we only accept `!` where we want to.
+fn never_pattern_location(void: Void) {
+    // FIXME(never_patterns): Don't accept on a non-empty type.
     match Some(0) {
         None => {}
         Some(!) => {}
     }
-    // Don't accept on an arbitrary type, even if there are no more branches.
+    // FIXME(never_patterns): Don't accept on an arbitrary type, even if there are no more branches.
     match () {
         () => {}
         ! => {}
     }
-    // Don't accept even on an empty branch.
+    // FIXME(never_patterns): Don't accept even on an empty branch.
     match None::<Void> {
         None => {}
         ! => {}
     }
-    // Let alone if the emptiness is behind a reference.
+    // FIXME(never_patterns): Let alone if the emptiness is behind a reference.
     match None::<&Void> {
         None => {}
         ! => {}
     }
-    // Don't participate in match ergonomics.
+    // Participate in match ergonomics.
+    match &void {
+        ! => {}
+    }
+    match &&void {
+        ! => {}
+    }
+    match &&void {
+        &! => {}
+    }
+    match &None::<Void> {
+        None => {}
+        Some(!) => {}
+    }
     match None::<&Void> {
         None => {}
         Some(!) => {}
     }
-    // Accept on a nested empty type.
+    // Accept on a composite empty type.
     match None::<&(u32, Void)> {
         None => {}
         Some(&!) => {}
     }
-    // Accept on an basic empty type.
+    // Accept on an simple empty type.
     match None::<Void> {
         None => {}
         Some(!) => {}
@@ -64,15 +78,15 @@ fn never_pattern_location() {
 fn never_and_bindings() {
     let x: Result<bool, &(u32, Void)> = Ok(false);
 
-    // Never patterns in or-patterns don't need to share the same bindings.
+    // FIXME(never_patterns): Never patterns in or-patterns don't need to share the same bindings.
     match x {
         Ok(_x) | Err(&!) => {}
         //~^ ERROR: is not bound in all patterns
     }
     let (Ok(_x) | Err(&!)) = x;
         //~^ ERROR: is not bound in all patterns
 
-    // A never pattern mustn't have bindings.
+    // FIXME(never_patterns): A never pattern mustn't have bindings.
     match x {
         Ok(_) => {}
         Err(&(_b, !)) => {}
diff --git a/tests/ui/pattern/never_patterns.stderr b/tests/ui/pattern/never_patterns.stderr
@@ -1,45 +1,45 @@
 error[E0408]: variable `_x` is not bound in all patterns
-  --> $DIR/never_patterns.rs:11:19
+  --> $DIR/never_patterns.rs:12:19
    |
 LL |     let (Ok(_x) | Err(!)) = &res;
    |             --    ^^^^^^ pattern doesn't bind `_x`
    |             |
    |             variable not in all patterns
 
 error[E0408]: variable `_x` is not bound in all patterns
-  --> $DIR/never_patterns.rs:13:19
+  --> $DIR/never_patterns.rs:14:19
    |
 LL |     let (Ok(_x) | Err(&!)) = res.as_ref();
    |             --    ^^^^^^^ pattern doesn't bind `_x`
    |             |
    |             variable not in all patterns
 
 error[E0408]: variable `_x` is not bound in all patterns
-  --> $DIR/never_patterns.rs:69:18
+  --> $DIR/never_patterns.rs:83:18
    |
 LL |         Ok(_x) | Err(&!) => {}
    |            --    ^^^^^^^ pattern doesn't bind `_x`
    |            |
    |            variable not in all patterns
 
 error[E0408]: variable `_x` is not bound in all patterns
-  --> $DIR/never_patterns.rs:72:19
+  --> $DIR/never_patterns.rs:86:19
    |
 LL |     let (Ok(_x) | Err(&!)) = x;
    |             --    ^^^^^^^ pattern doesn't bind `_x`
    |             |
    |             variable not in all patterns
 
 error[E0408]: variable `_b` is not bound in all patterns
-  --> $DIR/never_patterns.rs:81:9
+  --> $DIR/never_patterns.rs:95:9
    |
 LL |         Ok(_a) | Err(&(_b, !)) => {}
    |         ^^^^^^         -- variable not in all patterns
    |         |
    |         pattern doesn't bind `_b`
 
 error[E0408]: variable `_a` is not bound in all patterns
-  --> $DIR/never_patterns.rs:81:18
+  --> $DIR/never_patterns.rs:95:18
    |
 LL |         Ok(_a) | Err(&(_b, !)) => {}
    |            --    ^^^^^^^^^^^^^ pattern doesn't bind `_a`

Original file line number	Diff line number	Diff line change
`@@ -1558,7 +1558,9 @@ impl<'p, 'tcx> DeconstructedPat<'p, 'tcx> {`
`1558`	`1558`	`fields = Fields::from_iter(cx, pats.into_iter().map(mkpat));`
`1559`	`1559`	`}`
`1560`	`1560`	`PatKind::Never => {`
`1561`		`- ctor = Wildcard; // FIXME(never_patterns)`
	`1561`	+ // FIXME(never_patterns): handle `!` in exhaustiveness. This is a sane default
	`1562`	`+ // in the meantime.`
	`1563`	`+ ctor = Wildcard;`
`1562`	`1564`	`fields = Fields::empty();`
`1563`	`1565`	`}`
`1564`	`1566`	`PatKind::Error(_) => {`
Original file line number	Diff line number	Diff line change
`@@ -15,4 +15,13 @@ fn main() {`
`15`	`15`	! => {} //~ ERROR `!` patterns are experimental
`16`	`16`	`}`
`17`	`17`	`}`
	`18`	`+`
	`19`	+ // Check that the gate operates even behind `cfg`.
	`20`	`+ #[cfg(FALSE)]`
	`21`	`+ unsafe {`
	`22`	`+ let ptr: *const Void = NonNull::dangling().as_ptr();`
	`23`	`+ match *ptr {`
	`24`	+ ! => {} //~ ERROR `!` patterns are experimental
	`25`	`+ }`
	`26`	`+ }`
`18`	`27`	`}`