From 27704c7f9eea6e8ffaedac04ae1c932da7422fb7 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 31 Mar 2024 23:57:53 +0200 Subject: [PATCH] Fix union handling in exhaustiveness --- compiler/rustc_middle/src/thir.rs | 3 +- .../rustc_pattern_analysis/src/constructor.rs | 28 +++++++++++++++ .../rustc_pattern_analysis/src/usefulness.rs | 35 +++++++++++++++---- tests/ui/pattern/usefulness/unions.rs | 5 +-- tests/ui/pattern/usefulness/unions.stderr | 32 ++++++++--------- 5 files changed, 78 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index 05f6fbbbfa3..5a0cf524e26 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -1120,7 +1120,8 @@ impl<'tcx> fmt::Display for Pat<'tcx> { printed += 1; } - if printed < variant.fields.len() { + let is_union = self.ty.ty_adt_def().is_some_and(|adt| adt.is_union()); + if printed < variant.fields.len() && (!is_union || printed == 0) { write!(f, "{}..", start_or_comma())?; } diff --git a/compiler/rustc_pattern_analysis/src/constructor.rs b/compiler/rustc_pattern_analysis/src/constructor.rs index 49b2e510642..44f09b66bf6 100644 --- a/compiler/rustc_pattern_analysis/src/constructor.rs +++ b/compiler/rustc_pattern_analysis/src/constructor.rs @@ -140,6 +140,34 @@ //! [`ConstructorSet::split`]. The invariants of [`SplitConstructorSet`] are also of interest. //! //! +//! ## Unions +//! +//! Unions allow us to match a value via several overlapping representations at the same time. For +//! example, the following is exhaustive because when seeing the value as a boolean we handled all +//! possible cases (other cases such as `n == 3` would trigger UB). +//! +//! ```rust +//! # fn main() { +//! union U8AsBool { +//! n: u8, +//! b: bool, +//! } +//! let x = U8AsBool { n: 1 }; +//! unsafe { +//! match x { +//! U8AsBool { n: 2 } => {} +//! U8AsBool { b: true } => {} +//! U8AsBool { b: false } => {} +//! } +//! } +//! # } +//! ``` +//! +//! Pattern-matching has no knowledge that e.g. `false as u8 == 0`, so the values we consider in the +//! algorithm look like `U8AsBool { b: true, n: 2 }`. In other words, for the most part a union is +//! treated like a struct with the same fields. The difference lies in how we construct witnesses of +//! non-exhaustiveness. +//! //! //! ## Opaque patterns //! diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 1b5dd8478d6..7246039174a 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -1384,12 +1384,35 @@ impl WitnessStack { /// pats: [(false, "foo"), _, true] /// result: [Enum::Variant { a: (false, "foo"), b: _ }, true] /// ``` - fn apply_constructor(&mut self, pcx: &PlaceCtxt<'_, Cx>, ctor: &Constructor) { + fn apply_constructor( + mut self, + pcx: &PlaceCtxt<'_, Cx>, + ctor: &Constructor, + ) -> SmallVec<[Self; 1]> { let len = self.0.len(); let arity = pcx.ctor_arity(ctor); - let fields = self.0.drain((len - arity)..).rev().collect(); - let pat = WitnessPat::new(ctor.clone(), fields, pcx.ty.clone()); - self.0.push(pat); + let fields: Vec<_> = self.0.drain((len - arity)..).rev().collect(); + if matches!(ctor, Constructor::UnionField) + && fields.iter().filter(|p| !matches!(p.ctor(), Constructor::Wildcard)).count() >= 2 + { + // Convert a `Union { a: p, b: q }` witness into `Union { a: p }` and `Union { b: q }`. + // First add `Union { .. }` to `self`. + self.0.push(WitnessPat::wild_from_ctor(pcx.cx, ctor.clone(), pcx.ty.clone())); + fields + .into_iter() + .enumerate() + .filter(|(_, p)| !matches!(p.ctor(), Constructor::Wildcard)) + .map(|(i, p)| { + let mut ret = self.clone(); + // Fill the `i`th field of the union with `p`. + ret.0.last_mut().unwrap().fields[i] = p; + ret + }) + .collect() + } else { + self.0.push(WitnessPat::new(ctor.clone(), fields, pcx.ty.clone())); + smallvec![self] + } } } @@ -1462,8 +1485,8 @@ impl WitnessMatrix { *self = ret; } else { // Any other constructor we unspecialize as expected. - for witness in self.0.iter_mut() { - witness.apply_constructor(pcx, ctor) + for witness in std::mem::take(&mut self.0) { + self.0.extend(witness.apply_constructor(pcx, ctor)); } } } diff --git a/tests/ui/pattern/usefulness/unions.rs b/tests/ui/pattern/usefulness/unions.rs index 32fe9b4c04d..80a7f36a09a 100644 --- a/tests/ui/pattern/usefulness/unions.rs +++ b/tests/ui/pattern/usefulness/unions.rs @@ -20,12 +20,13 @@ fn main() { U8AsBool { n: 1.. } => {} } match x { - //~^ ERROR non-exhaustive patterns: `U8AsBool { n: 0_u8, b: false }` not covered + //~^ ERROR non-exhaustive patterns: `U8AsBool { n: 0_u8 }` and `U8AsBool { b: false }` not covered U8AsBool { b: true } => {} U8AsBool { n: 1.. } => {} } + // Our approach can report duplicate witnesses sometimes. match (x, true) { - //~^ ERROR non-exhaustive patterns: `(U8AsBool { n: 0_u8, b: false }, false)` and `(U8AsBool { n: 0_u8, b: true }, false)` not covered + //~^ ERROR non-exhaustive patterns: `(U8AsBool { n: 0_u8 }, false)`, `(U8AsBool { b: false }, false)`, `(U8AsBool { n: 0_u8 }, false)` and 1 more not covered (U8AsBool { b: true }, true) => {} (U8AsBool { b: false }, true) => {} (U8AsBool { n: 1.. }, true) => {} diff --git a/tests/ui/pattern/usefulness/unions.stderr b/tests/ui/pattern/usefulness/unions.stderr index d824a031cd3..4b397dc25db 100644 --- a/tests/ui/pattern/usefulness/unions.stderr +++ b/tests/ui/pattern/usefulness/unions.stderr @@ -1,8 +1,8 @@ -error[E0004]: non-exhaustive patterns: `U8AsBool { n: 0_u8, b: false }` not covered +error[E0004]: non-exhaustive patterns: `U8AsBool { n: 0_u8 }` and `U8AsBool { b: false }` not covered --> $DIR/unions.rs:22:15 | LL | match x { - | ^ pattern `U8AsBool { n: 0_u8, b: false }` not covered + | ^ patterns `U8AsBool { n: 0_u8 }` and `U8AsBool { b: false }` not covered | note: `U8AsBool` defined here --> $DIR/unions.rs:3:11 @@ -10,23 +10,23 @@ note: `U8AsBool` defined here LL | union U8AsBool { | ^^^^^^^^ = note: the matched value is of type `U8AsBool` -help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown - | -LL ~ U8AsBool { n: 1.. } => {}, -LL + U8AsBool { n: 0_u8, b: false } => todo!() - | - -error[E0004]: non-exhaustive patterns: `(U8AsBool { n: 0_u8, b: false }, false)` and `(U8AsBool { n: 0_u8, b: true }, false)` not covered - --> $DIR/unions.rs:27:15 - | -LL | match (x, true) { - | ^^^^^^^^^ patterns `(U8AsBool { n: 0_u8, b: false }, false)` and `(U8AsBool { n: 0_u8, b: true }, false)` not covered - | - = note: the matched value is of type `(U8AsBool, bool)` help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms | +LL ~ U8AsBool { n: 1.. } => {}, +LL + U8AsBool { n: 0_u8 } | U8AsBool { b: false } => todo!() + | + +error[E0004]: non-exhaustive patterns: `(U8AsBool { n: 0_u8 }, false)`, `(U8AsBool { b: false }, false)`, `(U8AsBool { n: 0_u8 }, false)` and 1 more not covered + --> $DIR/unions.rs:28:15 + | +LL | match (x, true) { + | ^^^^^^^^^ patterns `(U8AsBool { n: 0_u8 }, false)`, `(U8AsBool { b: false }, false)`, `(U8AsBool { n: 0_u8 }, false)` and 1 more not covered + | + = note: the matched value is of type `(U8AsBool, bool)` +help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown, or multiple match arms + | LL ~ (U8AsBool { n: 1.. }, true) => {}, -LL + (U8AsBool { n: 0_u8, b: false }, false) | (U8AsBool { n: 0_u8, b: true }, false) => todo!() +LL + _ => todo!() | error: aborting due to 2 previous errors