about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-12-22 12:12:12 +0000
committerbors <bors@rust-lang.org>2023-12-22 12:12:12 +0000
commitc1fc1d18cd38cab44696a9b0e0d52633863308fd (patch)
tree99838f0d4ebe1bbdf1f31a5a042b6c6ea0045419
parentef1b78eabe713a2068a1b0451102853dd2475a7b (diff)
parent2a87bae48d415b9ced69ae52513f004d06e34283 (diff)
downloadrust-c1fc1d18cd38cab44696a9b0e0d52633863308fd.tar.gz
rust-c1fc1d18cd38cab44696a9b0e0d52633863308fd.zip
Auto merge of #116821 - Nadrieril:fix-opaque-ice, r=compiler-errors
Exhaustiveness: reveal opaque types properly

Previously, exhaustiveness had no clear policy around opaque types. In this PR I propose the following policy: within the body of an item that defines the hidden type of some opaque type, exhaustiveness checking on a value of that opaque type is performed using the concrete hidden type inferred in this body.

I'm not sure how consistent this is with other operations allowed on opaque types; I believe this will require FCP.

From what I can tell, this doesn't change anything for non-empty types.

The observable changes are:
- when the real type is uninhabited, matches within the defining scopes can now rely on that for exhaustiveness, e.g.:

```rust
#[derive(Copy, Clone)]
enum Void {}
fn return_never_rpit(x: Void) -> impl Copy {
    if false {
        match return_never_rpit(x) {}
    }
    x
}
```
- this properly fixes ICEs like https://github.com/rust-lang/rust/issues/117100 that occurred because a same match could have some patterns where the type is revealed and some where it is not.

Bonus subtle point: if `x` is opaque, a match like `match x { ("", "") => {} ... }` will constrain its type ([playground](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=901d715330eac40339b4016ac566d6c3)). This is not the case for `match x {}`: this will not constain the type, and will only compile if something else constrains the type to be empty.

Fixes https://github.com/rust-lang/rust/issues/117100

r? `@oli-obk`

Edited for precision of the wording

[Included](https://github.com/rust-lang/rust/pull/116821#issuecomment-1813171764) in the FCP on this PR is this rule:

> Within the body of an item that defines the hidden type of some opaque type, exhaustiveness checking on a value of that opaque type is performed using the concrete hidden type inferred in this body.
-rw-r--r--compiler/rustc_mir_build/src/thir/pattern/check_match.rs4
-rw-r--r--compiler/rustc_pattern_analysis/src/lib.rs3
-rw-r--r--compiler/rustc_pattern_analysis/src/lints.rs22
-rw-r--r--compiler/rustc_pattern_analysis/src/rustc.rs21
-rw-r--r--compiler/rustc_pattern_analysis/src/usefulness.rs20
-rw-r--r--tests/ui/pattern/usefulness/impl-trait.rs146
-rw-r--r--tests/ui/pattern/usefulness/impl-trait.stderr101
7 files changed, 284 insertions, 33 deletions
diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs
index c435f4023af..d67dc59c618 100644
--- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs
+++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs
@@ -28,6 +28,7 @@ use rustc_span::hygiene::DesugaringKind;
 use rustc_span::Span;
 
 pub(crate) fn check_match(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Result<(), ErrorGuaranteed> {
+    let typeck_results = tcx.typeck(def_id);
     let (thir, expr) = tcx.thir_body(def_id)?;
     let thir = thir.borrow();
     let pattern_arena = TypedArena::default();
@@ -35,6 +36,7 @@ pub(crate) fn check_match(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Result<(), Err
     let mut visitor = MatchVisitor {
         tcx,
         thir: &*thir,
+        typeck_results,
         param_env: tcx.param_env(def_id),
         lint_level: tcx.local_def_id_to_hir_id(def_id),
         let_source: LetSource::None,
@@ -80,6 +82,7 @@ enum LetSource {
 struct MatchVisitor<'thir, 'p, 'tcx> {
     tcx: TyCtxt<'tcx>,
     param_env: ty::ParamEnv<'tcx>,
+    typeck_results: &'tcx ty::TypeckResults<'tcx>,
     thir: &'thir Thir<'tcx>,
     lint_level: HirId,
     let_source: LetSource,
@@ -382,6 +385,7 @@ impl<'thir, 'p, 'tcx> MatchVisitor<'thir, 'p, 'tcx> {
             scrutinee.map(|scrut| self.is_known_valid_scrutinee(scrut)).unwrap_or(true);
         MatchCheckCtxt {
             tcx: self.tcx,
+            typeck_results: self.typeck_results,
             param_env: self.param_env,
             module: self.tcx.parent_module(self.lint_level).to_def_id(),
             pattern_arena: self.pattern_arena,
diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs
index 785a60e9978..f00e6f18617 100644
--- a/compiler/rustc_pattern_analysis/src/lib.rs
+++ b/compiler/rustc_pattern_analysis/src/lib.rs
@@ -62,7 +62,8 @@ pub trait TypeCx: Sized + Clone + fmt::Debug {
     /// patterns during analysis.
     type PatData: Clone + Default;
 
-    fn is_opaque_ty(ty: Self::Ty) -> bool;
+    /// FIXME(Nadrieril): `Cx` should only give us revealed types.
+    fn reveal_opaque_ty(&self, ty: Self::Ty) -> Self::Ty;
     fn is_exhaustive_patterns_feature_on(&self) -> bool;
 
     /// The number of fields for this constructor.
diff --git a/compiler/rustc_pattern_analysis/src/lints.rs b/compiler/rustc_pattern_analysis/src/lints.rs
index 072ef4836a8..450a5cb0a10 100644
--- a/compiler/rustc_pattern_analysis/src/lints.rs
+++ b/compiler/rustc_pattern_analysis/src/lints.rs
@@ -48,22 +48,14 @@ impl<'a, 'p, 'tcx> PatternColumn<'a, 'p, 'tcx> {
     fn is_empty(&self) -> bool {
         self.patterns.is_empty()
     }
-    fn head_ty(&self) -> Option<Ty<'tcx>> {
+    fn head_ty(&self, cx: MatchCtxt<'a, 'p, 'tcx>) -> Option<Ty<'tcx>> {
         if self.patterns.len() == 0 {
             return None;
         }
-        // If the type is opaque and it is revealed anywhere in the column, we take the revealed
-        // version. Otherwise we could encounter constructors for the revealed type and crash.
-        let first_ty = self.patterns[0].ty();
-        if RustcMatchCheckCtxt::is_opaque_ty(first_ty) {
-            for pat in &self.patterns {
-                let ty = pat.ty();
-                if !RustcMatchCheckCtxt::is_opaque_ty(ty) {
-                    return Some(ty);
-                }
-            }
-        }
-        Some(first_ty)
+
+        let ty = self.patterns[0].ty();
+        // FIXME(Nadrieril): `Cx` should only give us revealed types.
+        Some(cx.tycx.reveal_opaque_ty(ty))
     }
 
     /// Do constructor splitting on the constructors of the column.
@@ -125,7 +117,7 @@ fn collect_nonexhaustive_missing_variants<'a, 'p, 'tcx>(
     cx: MatchCtxt<'a, 'p, 'tcx>,
     column: &PatternColumn<'a, 'p, 'tcx>,
 ) -> Vec<WitnessPat<'p, 'tcx>> {
-    let Some(ty) = column.head_ty() else {
+    let Some(ty) = column.head_ty(cx) else {
         return Vec::new();
     };
     let pcx = &PlaceCtxt::new_dummy(cx, ty);
@@ -226,7 +218,7 @@ pub(crate) fn lint_overlapping_range_endpoints<'a, 'p, 'tcx>(
     cx: MatchCtxt<'a, 'p, 'tcx>,
     column: &PatternColumn<'a, 'p, 'tcx>,
 ) {
-    let Some(ty) = column.head_ty() else {
+    let Some(ty) = column.head_ty(cx) else {
         return;
     };
     let pcx = &PlaceCtxt::new_dummy(cx, ty);
diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs
index 65c90aa9f1d..d2cfb9a8b06 100644
--- a/compiler/rustc_pattern_analysis/src/rustc.rs
+++ b/compiler/rustc_pattern_analysis/src/rustc.rs
@@ -44,6 +44,7 @@ pub type WitnessPat<'p, 'tcx> = crate::pat::WitnessPat<RustcMatchCheckCtxt<'p, '
 #[derive(Clone)]
 pub struct RustcMatchCheckCtxt<'p, 'tcx> {
     pub tcx: TyCtxt<'tcx>,
+    pub typeck_results: &'tcx ty::TypeckResults<'tcx>,
     /// The module in which the match occurs. This is necessary for
     /// checking inhabited-ness of types because whether a type is (visibly)
     /// inhabited can depend on whether it was defined in the current module or
@@ -101,6 +102,21 @@ impl<'p, 'tcx> RustcMatchCheckCtxt<'p, 'tcx> {
         }
     }
 
+    /// Type inference occasionally gives us opaque types in places where corresponding patterns
+    /// have more specific types. To avoid inconsistencies as well as detect opaque uninhabited
+    /// types, we use the corresponding concrete type if possible.
+    fn reveal_opaque_ty(&self, ty: Ty<'tcx>) -> Ty<'tcx> {
+        if let ty::Alias(ty::Opaque, alias_ty) = ty.kind() {
+            if let Some(local_def_id) = alias_ty.def_id.as_local() {
+                let key = ty::OpaqueTypeKey { def_id: local_def_id, args: alias_ty.args };
+                if let Some(real_ty) = self.typeck_results.concrete_opaque_types.get(&key) {
+                    return real_ty.ty;
+                }
+            }
+        }
+        ty
+    }
+
     // In the cases of either a `#[non_exhaustive]` field list or a non-public field, we hide
     // uninhabited fields in order not to reveal the uninhabitedness of the whole variant.
     // This lists the fields we keep along with their types.
@@ -873,8 +889,9 @@ impl<'p, 'tcx> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> {
     fn is_exhaustive_patterns_feature_on(&self) -> bool {
         self.tcx.features().exhaustive_patterns
     }
-    fn is_opaque_ty(ty: Self::Ty) -> bool {
-        matches!(ty.kind(), ty::Alias(ty::Opaque, ..))
+
+    fn reveal_opaque_ty(&self, ty: Ty<'tcx>) -> Ty<'tcx> {
+        self.reveal_opaque_ty(ty)
     }
 
     fn ctor_arity(&self, ctor: &crate::constructor::Constructor<Self>, ty: Self::Ty) -> usize {
diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs
index 6b1de807797..6b9fbd73003 100644
--- a/compiler/rustc_pattern_analysis/src/usefulness.rs
+++ b/compiler/rustc_pattern_analysis/src/usefulness.rs
@@ -865,24 +865,14 @@ impl<'a, 'p, Cx: TypeCx> Matrix<'a, 'p, Cx> {
         matrix
     }
 
-    fn head_ty(&self) -> Option<Cx::Ty> {
+    fn head_ty(&self, mcx: MatchCtxt<'a, 'p, Cx>) -> Option<Cx::Ty> {
         if self.column_count() == 0 {
             return None;
         }
 
-        let mut ty = self.wildcard_row.head().ty();
-        // If the type is opaque and it is revealed anywhere in the column, we take the revealed
-        // version. Otherwise we could encounter constructors for the revealed type and crash.
-        if Cx::is_opaque_ty(ty) {
-            for pat in self.heads() {
-                let pat_ty = pat.ty();
-                if !Cx::is_opaque_ty(pat_ty) {
-                    ty = pat_ty;
-                    break;
-                }
-            }
-        }
-        Some(ty)
+        let ty = self.wildcard_row.head().ty();
+        // FIXME(Nadrieril): `Cx` should only give us revealed types.
+        Some(mcx.tycx.reveal_opaque_ty(ty))
     }
     fn column_count(&self) -> usize {
         self.wildcard_row.len()
@@ -1181,7 +1171,7 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>(
 ) -> WitnessMatrix<Cx> {
     debug_assert!(matrix.rows().all(|r| r.len() == matrix.column_count()));
 
-    let Some(ty) = matrix.head_ty() else {
+    let Some(ty) = matrix.head_ty(mcx) else {
         // The base case: there are no columns in the matrix. We are morally pattern-matching on ().
         // A row is useful iff it has no (unguarded) rows above it.
         for row in matrix.rows_mut() {
diff --git a/tests/ui/pattern/usefulness/impl-trait.rs b/tests/ui/pattern/usefulness/impl-trait.rs
new file mode 100644
index 00000000000..6e8c0feb710
--- /dev/null
+++ b/tests/ui/pattern/usefulness/impl-trait.rs
@@ -0,0 +1,146 @@
+#![feature(never_type)]
+#![feature(exhaustive_patterns)]
+#![feature(type_alias_impl_trait)]
+#![feature(non_exhaustive_omitted_patterns_lint)]
+#![deny(unreachable_patterns)]
+// Test that the lint traversal handles opaques correctly
+#![deny(non_exhaustive_omitted_patterns)]
+
+fn main() {}
+
+#[derive(Copy, Clone)]
+enum Void {}
+
+fn return_never_rpit(x: Void) -> impl Copy {
+    if false {
+        match return_never_rpit(x) {
+            _ => {} //~ ERROR unreachable
+        }
+    }
+    x
+}
+fn friend_of_return_never_rpit(x: Void) {
+    match return_never_rpit(x) {}
+    //~^ ERROR non-empty
+}
+
+type T = impl Copy;
+fn return_never_tait(x: Void) -> T {
+    if false {
+        match return_never_tait(x) {
+            _ => {} //~ ERROR unreachable
+        }
+    }
+    x
+}
+fn friend_of_return_never_tait(x: Void) {
+    match return_never_tait(x) {}
+    //~^ ERROR non-empty
+}
+
+fn option_never(x: Void) -> Option<impl Copy> {
+    if false {
+        match option_never(x) {
+            None => {}
+            Some(_) => {} //~ ERROR unreachable
+        }
+        match option_never(x) {
+            None => {}
+            // FIXME: Unreachable not detected because `is_uninhabited` does not look into opaque
+            // types.
+            _ => {}
+        }
+    }
+    Some(x)
+}
+
+fn option_never2(x: Void) -> impl Copy {
+    if false {
+        match option_never2(x) {
+            None => {}
+            Some(_) => {} //~ ERROR unreachable
+        }
+        match option_never2(x) {
+            None => {}
+            _ => {} //~ ERROR unreachable
+        }
+        match option_never2(x) {
+            None => {}
+        }
+    }
+    Some(x)
+}
+
+fn inner_never(x: Void) {
+    type T = impl Copy;
+    let y: T = x;
+    match y {
+        _ => {} //~ ERROR unreachable
+    }
+}
+
+// This one caused ICE https://github.com/rust-lang/rust/issues/117100.
+fn inner_tuple() {
+    type T = impl Copy;
+    let foo: T = Some((1u32, 2u32));
+    match foo {
+        _ => {}
+        Some((a, b)) => {} //~ ERROR unreachable
+    }
+}
+
+type U = impl Copy;
+fn unify_never(x: Void, u: U) -> U {
+    if false {
+        match u {
+            _ => {} //~ ERROR unreachable
+        }
+    }
+    x
+}
+
+type V = impl Copy;
+fn infer_in_match(x: Option<V>) {
+    match x {
+        None => {}
+        Some((a, b)) => {}
+        Some((mut x, mut y)) => {
+            //~^ ERROR unreachable
+            x = 42;
+            y = "foo";
+        }
+    }
+}
+
+type W = impl Copy;
+#[derive(Copy, Clone)]
+struct Rec<'a> {
+    n: u32,
+    w: Option<&'a W>,
+}
+fn recursive_opaque() -> W {
+    if false {
+        match recursive_opaque() {
+            // Check for the ol' ICE when the type is recursively opaque.
+            _ => {}
+            Rec { n: 0, w: Some(Rec { n: 0, w: _ }) } => {} //~ ERROR unreachable
+        }
+    }
+    let w: Option<&'static W> = None;
+    Rec { n: 0, w }
+}
+
+type X = impl Copy;
+struct SecretelyVoid(X);
+fn nested_empty_opaque(x: Void) -> X {
+    if false {
+        let opaque_void = nested_empty_opaque(x);
+        let secretely_void = SecretelyVoid(opaque_void);
+        match secretely_void {
+            // FIXME: Unreachable not detected because `is_uninhabited` does not look into opaque
+            // types.
+            _ => {}
+        }
+    }
+    x
+}
diff --git a/tests/ui/pattern/usefulness/impl-trait.stderr b/tests/ui/pattern/usefulness/impl-trait.stderr
new file mode 100644
index 00000000000..df6d43dbb3f
--- /dev/null
+++ b/tests/ui/pattern/usefulness/impl-trait.stderr
@@ -0,0 +1,101 @@
+error: unreachable pattern
+  --> $DIR/impl-trait.rs:17:13
+   |
+LL |             _ => {}
+   |             ^
+   |
+note: the lint level is defined here
+  --> $DIR/impl-trait.rs:5:9
+   |
+LL | #![deny(unreachable_patterns)]
+   |         ^^^^^^^^^^^^^^^^^^^^
+
+error: unreachable pattern
+  --> $DIR/impl-trait.rs:31:13
+   |
+LL |             _ => {}
+   |             ^
+
+error: unreachable pattern
+  --> $DIR/impl-trait.rs:45:13
+   |
+LL |             Some(_) => {}
+   |             ^^^^^^^
+
+error: unreachable pattern
+  --> $DIR/impl-trait.rs:61:13
+   |
+LL |             Some(_) => {}
+   |             ^^^^^^^
+
+error: unreachable pattern
+  --> $DIR/impl-trait.rs:65:13
+   |
+LL |             _ => {}
+   |             ^
+
+error: unreachable pattern
+  --> $DIR/impl-trait.rs:78:9
+   |
+LL |         _ => {}
+   |         ^
+
+error: unreachable pattern
+  --> $DIR/impl-trait.rs:88:9
+   |
+LL |         _ => {}
+   |         - matches any value
+LL |         Some((a, b)) => {}
+   |         ^^^^^^^^^^^^ unreachable pattern
+
+error: unreachable pattern
+  --> $DIR/impl-trait.rs:96:13
+   |
+LL |             _ => {}
+   |             ^
+
+error: unreachable pattern
+  --> $DIR/impl-trait.rs:107:9
+   |
+LL |         Some((mut x, mut y)) => {
+   |         ^^^^^^^^^^^^^^^^^^^^
+
+error: unreachable pattern
+  --> $DIR/impl-trait.rs:126:13
+   |
+LL |             _ => {}
+   |             - matches any value
+LL |             Rec { n: 0, w: Some(Rec { n: 0, w: _ }) } => {}
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unreachable pattern
+
+error[E0004]: non-exhaustive patterns: type `impl Copy` is non-empty
+  --> $DIR/impl-trait.rs:23:11
+   |
+LL |     match return_never_rpit(x) {}
+   |           ^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the matched value is of type `impl Copy`
+help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown
+   |
+LL ~     match return_never_rpit(x) {
+LL +         _ => todo!(),
+LL +     }
+   |
+
+error[E0004]: non-exhaustive patterns: type `T` is non-empty
+  --> $DIR/impl-trait.rs:37:11
+   |
+LL |     match return_never_tait(x) {}
+   |           ^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the matched value is of type `T`
+help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown
+   |
+LL ~     match return_never_tait(x) {
+LL +         _ => todo!(),
+LL +     }
+   |
+
+error: aborting due to 12 previous errors
+
+For more information about this error, try `rustc --explain E0004`.