From 7e4924b55d25847bad6d4760a8484456c758ff52 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 29 Oct 2023 21:14:52 +0100 Subject: [PATCH 1/2] Add tests --- tests/ui/pattern/usefulness/impl-trait.rs | 146 ++++++++++++++++++ tests/ui/pattern/usefulness/impl-trait.stderr | 71 +++++++++ 2 files changed, 217 insertions(+) create mode 100644 tests/ui/pattern/usefulness/impl-trait.rs create mode 100644 tests/ui/pattern/usefulness/impl-trait.stderr diff --git a/tests/ui/pattern/usefulness/impl-trait.rs b/tests/ui/pattern/usefulness/impl-trait.rs new file mode 100644 index 0000000000000..6fa28877b3e03 --- /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) { + _ => {} + } + } + 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) { + _ => {} + } + } + x +} +fn friend_of_return_never_tait(x: Void) { + match return_never_tait(x) {} + //~^ ERROR non-empty +} + +fn option_never(x: Void) -> Option { + if false { + match option_never(x) { + None => {} + Some(_) => {} + } + 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 { + _ => {} + } +} + +// 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 { + _ => {} + } + } + x +} + +type V = impl Copy; +fn infer_in_match(x: Option) { + 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 0000000000000..6d7cef88d4f70 --- /dev/null +++ b/tests/ui/pattern/usefulness/impl-trait.stderr @@ -0,0 +1,71 @@ +error: unreachable pattern + --> $DIR/impl-trait.rs:61:13 + | +LL | Some(_) => {} + | ^^^^^^^ + | +note: the lint level is defined here + --> $DIR/impl-trait.rs:5:9 + | +LL | #![deny(unreachable_patterns)] + | ^^^^^^^^^^^^^^^^^^^^ + +error: unreachable pattern + --> $DIR/impl-trait.rs:65:13 + | +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: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 7 previous errors + +For more information about this error, try `rustc --explain E0004`. From 2a87bae48d415b9ced69ae52513f004d06e34283 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 16 Nov 2023 04:28:22 +0100 Subject: [PATCH 2/2] Reveal opaque types in exhaustiveness checking --- .../src/thir/pattern/check_match.rs | 4 ++ compiler/rustc_pattern_analysis/src/lib.rs | 3 +- compiler/rustc_pattern_analysis/src/lints.rs | 22 ++++------- compiler/rustc_pattern_analysis/src/rustc.rs | 21 +++++++++- .../rustc_pattern_analysis/src/usefulness.rs | 20 +++------- tests/ui/pattern/usefulness/impl-trait.rs | 10 ++--- tests/ui/pattern/usefulness/impl-trait.stderr | 38 +++++++++++++++++-- 7 files changed, 76 insertions(+), 42 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 c435f4023af68..d67dc59c6185f 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 785a60e997842..f00e6f18617b9 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 072ef4836a84e..450a5cb0a105a 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> { + fn head_ty(&self, cx: MatchCtxt<'a, 'p, 'tcx>) -> Option> { 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> { - 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 65c90aa9f1d23..d2cfb9a8b068c 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 { 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, ty: Self::Ty) -> usize { diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 6b1de807797cf..6b9fbd7300327 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 { + fn head_ty(&self, mcx: MatchCtxt<'a, 'p, Cx>) -> Option { 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 { 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 index 6fa28877b3e03..6e8c0feb71030 100644 --- a/tests/ui/pattern/usefulness/impl-trait.rs +++ b/tests/ui/pattern/usefulness/impl-trait.rs @@ -14,7 +14,7 @@ enum Void {} fn return_never_rpit(x: Void) -> impl Copy { if false { match return_never_rpit(x) { - _ => {} + _ => {} //~ ERROR unreachable } } x @@ -28,7 +28,7 @@ type T = impl Copy; fn return_never_tait(x: Void) -> T { if false { match return_never_tait(x) { - _ => {} + _ => {} //~ ERROR unreachable } } x @@ -42,7 +42,7 @@ fn option_never(x: Void) -> Option { if false { match option_never(x) { None => {} - Some(_) => {} + Some(_) => {} //~ ERROR unreachable } match option_never(x) { None => {} @@ -75,7 +75,7 @@ fn inner_never(x: Void) { type T = impl Copy; let y: T = x; match y { - _ => {} + _ => {} //~ ERROR unreachable } } @@ -93,7 +93,7 @@ type U = impl Copy; fn unify_never(x: Void, u: U) -> U { if false { match u { - _ => {} + _ => {} //~ ERROR unreachable } } x diff --git a/tests/ui/pattern/usefulness/impl-trait.stderr b/tests/ui/pattern/usefulness/impl-trait.stderr index 6d7cef88d4f70..df6d43dbb3f28 100644 --- a/tests/ui/pattern/usefulness/impl-trait.stderr +++ b/tests/ui/pattern/usefulness/impl-trait.stderr @@ -1,8 +1,8 @@ error: unreachable pattern - --> $DIR/impl-trait.rs:61:13 + --> $DIR/impl-trait.rs:17:13 | -LL | Some(_) => {} - | ^^^^^^^ +LL | _ => {} + | ^ | note: the lint level is defined here --> $DIR/impl-trait.rs:5:9 @@ -10,12 +10,36 @@ note: the lint level is defined here 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 | @@ -24,6 +48,12 @@ LL | _ => {} 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 | @@ -66,6 +96,6 @@ LL + _ => todo!(), LL + } | -error: aborting due to 7 previous errors +error: aborting due to 12 previous errors For more information about this error, try `rustc --explain E0004`.