From 3b37d941a29d91afbfa06afcb63f702ee7b810d6 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 17 Oct 2020 19:01:10 +0100 Subject: [PATCH 01/13] Add some tests --- .../const_in_pattern}/issue-44333.rs | 0 .../const_in_pattern}/issue-44333.stderr | 0 .../ui/consts/const_in_pattern/issue-78057.rs | 17 ++ .../const_in_pattern/issue-78057.stderr | 20 +++ .../ui/pattern/usefulness/consts-opaque.rs | 113 +++++++++++++ .../pattern/usefulness/consts-opaque.stderr | 154 ++++++++++++++++++ 6 files changed, 304 insertions(+) rename src/test/ui/{issues => consts/const_in_pattern}/issue-44333.rs (100%) rename src/test/ui/{issues => consts/const_in_pattern}/issue-44333.stderr (100%) create mode 100644 src/test/ui/consts/const_in_pattern/issue-78057.rs create mode 100644 src/test/ui/consts/const_in_pattern/issue-78057.stderr create mode 100644 src/test/ui/pattern/usefulness/consts-opaque.rs create mode 100644 src/test/ui/pattern/usefulness/consts-opaque.stderr diff --git a/src/test/ui/issues/issue-44333.rs b/src/test/ui/consts/const_in_pattern/issue-44333.rs similarity index 100% rename from src/test/ui/issues/issue-44333.rs rename to src/test/ui/consts/const_in_pattern/issue-44333.rs diff --git a/src/test/ui/issues/issue-44333.stderr b/src/test/ui/consts/const_in_pattern/issue-44333.stderr similarity index 100% rename from src/test/ui/issues/issue-44333.stderr rename to src/test/ui/consts/const_in_pattern/issue-44333.stderr diff --git a/src/test/ui/consts/const_in_pattern/issue-78057.rs b/src/test/ui/consts/const_in_pattern/issue-78057.rs new file mode 100644 index 0000000000000..69cf8404da18e --- /dev/null +++ b/src/test/ui/consts/const_in_pattern/issue-78057.rs @@ -0,0 +1,17 @@ +#![deny(unreachable_patterns)] + +#[derive(PartialEq)] +struct Opaque(i32); + +impl Eq for Opaque {} + +const FOO: Opaque = Opaque(42); + +fn main() { + match FOO { + FOO => {}, + //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` + _ => {} + //~^ ERROR unreachable pattern + } +} diff --git a/src/test/ui/consts/const_in_pattern/issue-78057.stderr b/src/test/ui/consts/const_in_pattern/issue-78057.stderr new file mode 100644 index 0000000000000..0d49d0e96c854 --- /dev/null +++ b/src/test/ui/consts/const_in_pattern/issue-78057.stderr @@ -0,0 +1,20 @@ +error: to use a constant of type `Opaque` in a pattern, `Opaque` must be annotated with `#[derive(PartialEq, Eq)]` + --> $DIR/issue-78057.rs:12:9 + | +LL | FOO => {}, + | ^^^ + +error: unreachable pattern + --> $DIR/issue-78057.rs:14:9 + | +LL | _ => {} + | ^ + | +note: the lint level is defined here + --> $DIR/issue-78057.rs:1:9 + | +LL | #![deny(unreachable_patterns)] + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/pattern/usefulness/consts-opaque.rs b/src/test/ui/pattern/usefulness/consts-opaque.rs new file mode 100644 index 0000000000000..928756723a6c9 --- /dev/null +++ b/src/test/ui/pattern/usefulness/consts-opaque.rs @@ -0,0 +1,113 @@ +// This file tests the exhaustiveness algorithm on opaque constants. Most of the examples give +// unnecessary warnings because const_to_pat.rs converts a constant pattern to a wildcard when the +// constant is not allowed as a pattern. This is an edge case so we may not care to fix it. +// See also https://github.com/rust-lang/rust/issues/78057 + +#![deny(unreachable_patterns)] + +#[derive(PartialEq)] +struct Foo(i32); +impl Eq for Foo {} +const FOO: Foo = Foo(42); +const FOO_REF: &Foo = &Foo(42); +const FOO_REF_REF: &&Foo = &&Foo(42); + +#[derive(PartialEq)] +struct Bar; +impl Eq for Bar {} +const BAR: Bar = Bar; + +#[derive(PartialEq)] +enum Baz { + Baz1, + Baz2 +} +impl Eq for Baz {} +const BAZ: Baz = Baz::Baz1; + +type Quux = fn(usize, usize) -> usize; +fn quux(a: usize, b: usize) -> usize { a + b } +const QUUX: Quux = quux; + +fn main() { + match FOO { + FOO => {} + //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` + _ => {} // should not be emitting unreachable warning + //~^ ERROR unreachable pattern + } + + match FOO_REF { + FOO_REF => {} + //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` + Foo(_) => {} // should not be emitting unreachable warning + //~^ ERROR unreachable pattern + } + + // FIXME: this causes an ICE (https://github.com/rust-lang/rust/issues/78071) + //match FOO_REF_REF { + // FOO_REF_REF => {} + // Foo(_) => {} + //} + + match BAR { + Bar => {} + BAR => {} // should not be emitting unreachable warning + //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` + //~| ERROR unreachable pattern + _ => {} + //~^ ERROR unreachable pattern + } + + match BAR { + BAR => {} + //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` + Bar => {} // should not be emitting unreachable warning + //~^ ERROR unreachable pattern + _ => {} + //~^ ERROR unreachable pattern + } + + match BAR { + BAR => {} + //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` + BAR => {} // should not be emitting unreachable warning + //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` + //~| ERROR unreachable pattern + _ => {} // should not be emitting unreachable warning + //~^ ERROR unreachable pattern + } + + match BAZ { + BAZ => {} + //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` + Baz::Baz1 => {} // should not be emitting unreachable warning + //~^ ERROR unreachable pattern + _ => {} + //~^ ERROR unreachable pattern + } + + match BAZ { + Baz::Baz1 => {} + BAZ => {} + //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` + _ => {} + //~^ ERROR unreachable pattern + } + + match BAZ { + BAZ => {} + //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` + Baz::Baz2 => {} // should not be emitting unreachable warning + //~^ ERROR unreachable pattern + _ => {} // should not be emitting unreachable warning + //~^ ERROR unreachable pattern + } + + match QUUX { + QUUX => {} + QUUX => {} // should not be emitting unreachable warning + //~^ ERROR unreachable pattern + _ => {} + } +} diff --git a/src/test/ui/pattern/usefulness/consts-opaque.stderr b/src/test/ui/pattern/usefulness/consts-opaque.stderr new file mode 100644 index 0000000000000..07cdc1c95fad0 --- /dev/null +++ b/src/test/ui/pattern/usefulness/consts-opaque.stderr @@ -0,0 +1,154 @@ +error: to use a constant of type `Foo` in a pattern, `Foo` must be annotated with `#[derive(PartialEq, Eq)]` + --> $DIR/consts-opaque.rs:34:9 + | +LL | FOO => {} + | ^^^ + +error: unreachable pattern + --> $DIR/consts-opaque.rs:36:9 + | +LL | _ => {} // should not be emitting unreachable warning + | ^ + | +note: the lint level is defined here + --> $DIR/consts-opaque.rs:6:9 + | +LL | #![deny(unreachable_patterns)] + | ^^^^^^^^^^^^^^^^^^^^ + +error: to use a constant of type `Foo` in a pattern, `Foo` must be annotated with `#[derive(PartialEq, Eq)]` + --> $DIR/consts-opaque.rs:41:9 + | +LL | FOO_REF => {} + | ^^^^^^^ + +error: unreachable pattern + --> $DIR/consts-opaque.rs:43:9 + | +LL | Foo(_) => {} // should not be emitting unreachable warning + | ^^^^^^ + +error: to use a constant of type `Bar` in a pattern, `Bar` must be annotated with `#[derive(PartialEq, Eq)]` + --> $DIR/consts-opaque.rs:55:9 + | +LL | BAR => {} // should not be emitting unreachable warning + | ^^^ + +error: unreachable pattern + --> $DIR/consts-opaque.rs:55:9 + | +LL | Bar => {} + | --- matches any value +LL | BAR => {} // should not be emitting unreachable warning + | ^^^ unreachable pattern + +error: unreachable pattern + --> $DIR/consts-opaque.rs:58:9 + | +LL | Bar => {} + | --- matches any value +... +LL | _ => {} + | ^ unreachable pattern + +error: to use a constant of type `Bar` in a pattern, `Bar` must be annotated with `#[derive(PartialEq, Eq)]` + --> $DIR/consts-opaque.rs:63:9 + | +LL | BAR => {} + | ^^^ + +error: unreachable pattern + --> $DIR/consts-opaque.rs:65:9 + | +LL | Bar => {} // should not be emitting unreachable warning + | ^^^ + +error: unreachable pattern + --> $DIR/consts-opaque.rs:67:9 + | +LL | Bar => {} // should not be emitting unreachable warning + | --- matches any value +LL | +LL | _ => {} + | ^ unreachable pattern + +error: to use a constant of type `Bar` in a pattern, `Bar` must be annotated with `#[derive(PartialEq, Eq)]` + --> $DIR/consts-opaque.rs:72:9 + | +LL | BAR => {} + | ^^^ + +error: to use a constant of type `Bar` in a pattern, `Bar` must be annotated with `#[derive(PartialEq, Eq)]` + --> $DIR/consts-opaque.rs:74:9 + | +LL | BAR => {} // should not be emitting unreachable warning + | ^^^ + +error: unreachable pattern + --> $DIR/consts-opaque.rs:74:9 + | +LL | BAR => {} // should not be emitting unreachable warning + | ^^^ + +error: unreachable pattern + --> $DIR/consts-opaque.rs:77:9 + | +LL | _ => {} // should not be emitting unreachable warning + | ^ + +error: to use a constant of type `Baz` in a pattern, `Baz` must be annotated with `#[derive(PartialEq, Eq)]` + --> $DIR/consts-opaque.rs:82:9 + | +LL | BAZ => {} + | ^^^ + +error: unreachable pattern + --> $DIR/consts-opaque.rs:84:9 + | +LL | Baz::Baz1 => {} // should not be emitting unreachable warning + | ^^^^^^^^^ + +error: unreachable pattern + --> $DIR/consts-opaque.rs:86:9 + | +LL | _ => {} + | ^ + +error: to use a constant of type `Baz` in a pattern, `Baz` must be annotated with `#[derive(PartialEq, Eq)]` + --> $DIR/consts-opaque.rs:92:9 + | +LL | BAZ => {} + | ^^^ + +error: unreachable pattern + --> $DIR/consts-opaque.rs:94:9 + | +LL | _ => {} + | ^ + +error: to use a constant of type `Baz` in a pattern, `Baz` must be annotated with `#[derive(PartialEq, Eq)]` + --> $DIR/consts-opaque.rs:99:9 + | +LL | BAZ => {} + | ^^^ + +error: unreachable pattern + --> $DIR/consts-opaque.rs:101:9 + | +LL | Baz::Baz2 => {} // should not be emitting unreachable warning + | ^^^^^^^^^ + +error: unreachable pattern + --> $DIR/consts-opaque.rs:103:9 + | +LL | _ => {} // should not be emitting unreachable warning + | ^ + +error: unreachable pattern + --> $DIR/consts-opaque.rs:109:9 + | +LL | QUUX => {} // should not be emitting unreachable warning + | ^^^^ + +error: aborting due to 23 previous errors + From bb8111069ed38455d915f7f8f0e050bb3c3720cf Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 29 Sep 2020 18:28:39 +0200 Subject: [PATCH 02/13] Destructure byte array constants to array patterns instead of keeping them opaque --- .../src/thir/pattern/_match.rs | 32 +++---------------- .../src/thir/pattern/const_to_pat.rs | 11 ++++--- .../match-byte-array-patterns-2.stderr | 4 +-- 3 files changed, 13 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/_match.rs b/compiler/rustc_mir_build/src/thir/pattern/_match.rs index 904524e13ae08..b657f25f1edb1 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/_match.rs @@ -304,12 +304,11 @@ use std::iter::{FromIterator, IntoIterator}; use std::ops::RangeInclusive; crate fn expand_pattern<'a, 'tcx>(cx: &MatchCheckCtxt<'a, 'tcx>, pat: Pat<'tcx>) -> Pat<'tcx> { - LiteralExpander { tcx: cx.tcx, param_env: cx.param_env }.fold_pattern(&pat) + LiteralExpander { tcx: cx.tcx }.fold_pattern(&pat) } struct LiteralExpander<'tcx> { tcx: TyCtxt<'tcx>, - param_env: ty::ParamEnv<'tcx>, } impl<'tcx> LiteralExpander<'tcx> { @@ -328,40 +327,17 @@ impl<'tcx> LiteralExpander<'tcx> { ) -> ConstValue<'tcx> { debug!("fold_const_value_deref {:?} {:?} {:?}", val, rty, crty); match (val, &crty.kind(), &rty.kind()) { - // the easy case, deref a reference - (ConstValue::Scalar(p), x, y) if x == y => { - match p { - Scalar::Ptr(p) => { - let alloc = self.tcx.global_alloc(p.alloc_id).unwrap_memory(); - ConstValue::ByRef { alloc, offset: p.offset } - } - Scalar::Raw { .. } => { - let layout = self.tcx.layout_of(self.param_env.and(rty)).unwrap(); - if layout.is_zst() { - // Deref of a reference to a ZST is a nop. - ConstValue::Scalar(Scalar::zst()) - } else { - // FIXME(oli-obk): this is reachable for `const FOO: &&&u32 = &&&42;` - bug!("cannot deref {:#?}, {} -> {}", val, crty, rty); - } - } - } - } // unsize array to slice if pattern is array but match value or other patterns are slice (ConstValue::Scalar(Scalar::Ptr(p)), ty::Array(t, n), ty::Slice(u)) => { assert_eq!(t, u); + assert_eq!(p.offset, Size::ZERO); ConstValue::Slice { data: self.tcx.global_alloc(p.alloc_id).unwrap_memory(), - start: p.offset.bytes().try_into().unwrap(), + start: 0, end: n.eval_usize(self.tcx, ty::ParamEnv::empty()).try_into().unwrap(), } } - // fat pointers stay the same - (ConstValue::Slice { .. }, _, _) - | (_, ty::Slice(_), ty::Slice(_)) - | (_, ty::Str, ty::Str) => val, - // FIXME(oli-obk): this is reachable for `const FOO: &&&u32 = &&&42;` being used - _ => bug!("cannot deref {:#?}, {} -> {}", val, crty, rty), + _ => val, } } } diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index a203b3a142863..32b9b0a2d3b14 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -390,11 +390,14 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { ty::Slice(elem_ty) if elem_ty == tcx.types.u8 => PatKind::Constant { value: cv }, // `b"foo"` produces a `&[u8; 3]`, but you can't use constants of array type when // matching against references, you can only use byte string literals. - // FIXME: clean this up, likely by permitting array patterns when matching on slices - ty::Array(elem_ty, _) if elem_ty == tcx.types.u8 => PatKind::Constant { value: cv }, + // The typechecker has a special case for byte string literals, by treating them + // as slices. This means we turn `&[T; N]` constants into slice patterns, which + // has no negative effects on pattern matching, even if we're actually matching on + // arrays. + ty::Array(..) | // Cannot merge this with the catch all branch below, because the `const_deref` - // changes the type from slice to array, and slice patterns behave differently from - // array patterns. + // changes the type from slice to array, we need to keep the original type in the + // pattern. ty::Slice(..) => { let old = self.behind_reference.replace(true); let array = tcx.deref_const(self.param_env.and(cv)); diff --git a/src/test/ui/pattern/usefulness/match-byte-array-patterns-2.stderr b/src/test/ui/pattern/usefulness/match-byte-array-patterns-2.stderr index ffc8433403fd5..7968f9713ff22 100644 --- a/src/test/ui/pattern/usefulness/match-byte-array-patterns-2.stderr +++ b/src/test/ui/pattern/usefulness/match-byte-array-patterns-2.stderr @@ -7,11 +7,11 @@ LL | match buf { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms = note: the matched value is of type `&[u8; 4]` -error[E0004]: non-exhaustive patterns: `&[]`, `&[_]`, `&[_, _]` and 2 more not covered +error[E0004]: non-exhaustive patterns: `&[0_u8..=64_u8, _, _, _]` and `&[66_u8..=u8::MAX, _, _, _]` not covered --> $DIR/match-byte-array-patterns-2.rs:10:11 | LL | match buf { - | ^^^ patterns `&[]`, `&[_]`, `&[_, _]` and 2 more not covered + | ^^^ patterns `&[0_u8..=64_u8, _, _, _]` and `&[66_u8..=u8::MAX, _, _, _]` not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms = note: the matched value is of type `&[u8]` From c3d0445021ce6b4ee9dc96c6a4dab611402988d6 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 1 Oct 2020 09:24:44 +0200 Subject: [PATCH 03/13] Destructure byte slices and remove all the workarounds --- .../src/thir/pattern/_match.rs | 249 +----------------- .../src/thir/pattern/check_match.rs | 2 +- .../src/thir/pattern/const_to_pat.rs | 1 - 3 files changed, 9 insertions(+), 243 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/_match.rs b/compiler/rustc_mir_build/src/thir/pattern/_match.rs index b657f25f1edb1..9ca711722a307 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/_match.rs @@ -284,10 +284,9 @@ use super::{FieldPat, Pat, PatKind, PatRange}; use rustc_arena::TypedArena; use rustc_attr::{SignedInt, UnsignedInt}; -use rustc_errors::ErrorReported; use rustc_hir::def_id::DefId; use rustc_hir::{HirId, RangeEnd}; -use rustc_middle::mir::interpret::{truncate, AllocId, ConstValue, Pointer, Scalar}; +use rustc_middle::mir::interpret::{truncate, ConstValue}; use rustc_middle::mir::Field; use rustc_middle::ty::layout::IntegerExt; use rustc_middle::ty::{self, Const, Ty, TyCtxt}; @@ -296,84 +295,21 @@ use rustc_span::{Span, DUMMY_SP}; use rustc_target::abi::{Integer, Size, VariantIdx}; use smallvec::{smallvec, SmallVec}; -use std::borrow::Cow; use std::cmp::{self, max, min, Ordering}; -use std::convert::TryInto; use std::fmt; use std::iter::{FromIterator, IntoIterator}; use std::ops::RangeInclusive; -crate fn expand_pattern<'a, 'tcx>(cx: &MatchCheckCtxt<'a, 'tcx>, pat: Pat<'tcx>) -> Pat<'tcx> { - LiteralExpander { tcx: cx.tcx }.fold_pattern(&pat) +crate fn expand_pattern<'tcx>(pat: Pat<'tcx>) -> Pat<'tcx> { + LiteralExpander.fold_pattern(&pat) } -struct LiteralExpander<'tcx> { - tcx: TyCtxt<'tcx>, -} +struct LiteralExpander; -impl<'tcx> LiteralExpander<'tcx> { - /// Derefs `val` and potentially unsizes the value if `crty` is an array and `rty` a slice. - /// - /// `crty` and `rty` can differ because you can use array constants in the presence of slice - /// patterns. So the pattern may end up being a slice, but the constant is an array. We convert - /// the array to a slice in that case. - fn fold_const_value_deref( - &mut self, - val: ConstValue<'tcx>, - // the pattern's pointee type - rty: Ty<'tcx>, - // the constant's pointee type - crty: Ty<'tcx>, - ) -> ConstValue<'tcx> { - debug!("fold_const_value_deref {:?} {:?} {:?}", val, rty, crty); - match (val, &crty.kind(), &rty.kind()) { - // unsize array to slice if pattern is array but match value or other patterns are slice - (ConstValue::Scalar(Scalar::Ptr(p)), ty::Array(t, n), ty::Slice(u)) => { - assert_eq!(t, u); - assert_eq!(p.offset, Size::ZERO); - ConstValue::Slice { - data: self.tcx.global_alloc(p.alloc_id).unwrap_memory(), - start: 0, - end: n.eval_usize(self.tcx, ty::ParamEnv::empty()).try_into().unwrap(), - } - } - _ => val, - } - } -} - -impl<'tcx> PatternFolder<'tcx> for LiteralExpander<'tcx> { +impl<'tcx> PatternFolder<'tcx> for LiteralExpander { fn fold_pattern(&mut self, pat: &Pat<'tcx>) -> Pat<'tcx> { debug!("fold_pattern {:?} {:?} {:?}", pat, pat.ty.kind(), pat.kind); match (pat.ty.kind(), &*pat.kind) { - (&ty::Ref(_, rty, _), &PatKind::Constant { value: Const { val, ty: const_ty } }) - if const_ty.is_ref() => - { - let crty = - if let ty::Ref(_, crty, _) = const_ty.kind() { crty } else { unreachable!() }; - if let ty::ConstKind::Value(val) = val { - Pat { - ty: pat.ty, - span: pat.span, - kind: box PatKind::Deref { - subpattern: Pat { - ty: rty, - span: pat.span, - kind: box PatKind::Constant { - value: Const::from_value( - self.tcx, - self.fold_const_value_deref(*val, rty, crty), - rty, - ), - }, - }, - }, - } - } else { - bug!("cannot deref {:#?}, {} -> {}", val, crty, rty) - } - } - (_, &PatKind::Binding { subpattern: Some(ref s), .. }) => s.fold_with(self), (_, &PatKind::AscribeUserType { subpattern: ref s, .. }) => s.fold_with(self), _ => pat.super_fold_with(self), @@ -941,8 +877,6 @@ impl<'tcx> Constructor<'tcx> { .iter() .filter_map(|c: &Constructor<'_>| match c { Slice(slice) => Some(*slice), - // FIXME(oli-obk): implement `deref` for `ConstValue` - ConstantValue(..) => None, _ => bug!("bad slice pattern constructor {:?}", c), }) .map(Slice::value_kind); @@ -1162,12 +1096,6 @@ impl<'p, 'tcx> Fields<'p, 'tcx> { Fields::Slice(std::slice::from_ref(pat)) } - /// Construct a new `Fields` from the given patterns. You must be sure those patterns can't - /// contain fields that need to be filtered out. When in doubt, prefer `replace_fields`. - fn from_slice_unfiltered(pats: &'p [Pat<'tcx>]) -> Self { - Fields::Slice(pats) - } - /// Convenience; internal use. fn wildcards_from_tys( cx: &MatchCheckCtxt<'p, 'tcx>, @@ -2183,19 +2111,7 @@ fn pat_constructor<'tcx>( if let Some(int_range) = IntRange::from_const(tcx, param_env, value, pat.span) { Some(IntRange(int_range)) } else { - match (value.val, &value.ty.kind()) { - (_, ty::Array(_, n)) => { - let len = n.eval_usize(tcx, param_env); - Some(Slice(Slice { array_len: Some(len), kind: FixedLen(len) })) - } - (ty::ConstKind::Value(ConstValue::Slice { start, end, .. }), ty::Slice(_)) => { - let len = (end - start) as u64; - Some(Slice(Slice { array_len: None, kind: FixedLen(len) })) - } - // FIXME(oli-obk): implement `deref` for `ConstValue` - // (ty::ConstKind::Value(ConstValue::ByRef { .. }), ty::Slice(_)) => { ... } - _ => Some(ConstantValue(value)), - } + Some(ConstantValue(value)) } } PatKind::Range(PatRange { lo, hi, end }) => { @@ -2230,75 +2146,6 @@ fn pat_constructor<'tcx>( } } -// checks whether a constant is equal to a user-written slice pattern. Only supports byte slices, -// meaning all other types will compare unequal and thus equal patterns often do not cause the -// second pattern to lint about unreachable match arms. -fn slice_pat_covered_by_const<'tcx>( - tcx: TyCtxt<'tcx>, - _span: Span, - const_val: &'tcx ty::Const<'tcx>, - prefix: &[Pat<'tcx>], - slice: &Option>, - suffix: &[Pat<'tcx>], - param_env: ty::ParamEnv<'tcx>, -) -> Result { - let const_val_val = if let ty::ConstKind::Value(val) = const_val.val { - val - } else { - bug!( - "slice_pat_covered_by_const: {:#?}, {:#?}, {:#?}, {:#?}", - const_val, - prefix, - slice, - suffix, - ) - }; - - let data: &[u8] = match (const_val_val, &const_val.ty.kind()) { - (ConstValue::ByRef { offset, alloc, .. }, ty::Array(t, n)) => { - assert_eq!(*t, tcx.types.u8); - let n = n.eval_usize(tcx, param_env); - let ptr = Pointer::new(AllocId(0), offset); - alloc.get_bytes(&tcx, ptr, Size::from_bytes(n)).unwrap() - } - (ConstValue::Slice { data, start, end }, ty::Slice(t)) => { - assert_eq!(*t, tcx.types.u8); - let ptr = Pointer::new(AllocId(0), Size::from_bytes(start)); - data.get_bytes(&tcx, ptr, Size::from_bytes(end - start)).unwrap() - } - // FIXME(oli-obk): create a way to extract fat pointers from ByRef - (_, ty::Slice(_)) => return Ok(false), - _ => bug!( - "slice_pat_covered_by_const: {:#?}, {:#?}, {:#?}, {:#?}", - const_val, - prefix, - slice, - suffix, - ), - }; - - let pat_len = prefix.len() + suffix.len(); - if data.len() < pat_len || (slice.is_none() && data.len() > pat_len) { - return Ok(false); - } - - for (ch, pat) in data[..prefix.len()] - .iter() - .zip(prefix) - .chain(data[data.len() - suffix.len()..].iter().zip(suffix)) - { - if let box PatKind::Constant { value } = pat.kind { - let b = value.eval_bits(tcx, param_env, pat.ty); - assert_eq!(b as u8 as u128, b); - if b as u8 != *ch { - return Ok(false); - } - } - } - - Ok(true) -} - /// For exhaustive integer matching, some constructors are grouped within other constructors /// (namely integer typed values are grouped within ranges). However, when specialising these /// constructors, we want to be specialising for the underlying constructors (the integers), not @@ -2670,73 +2517,7 @@ fn specialize_one_pattern<'p, 'tcx>( PatKind::Deref { ref subpattern } => Some(Fields::from_single_pattern(subpattern)), PatKind::Constant { value } if constructor.is_slice() => { - // We extract an `Option` for the pointer because slices of zero - // elements don't necessarily point to memory, they are usually - // just integers. The only time they should be pointing to memory - // is when they are subslices of nonzero slices. - let (alloc, offset, n, ty) = match value.ty.kind() { - ty::Array(t, n) => { - let n = n.eval_usize(cx.tcx, cx.param_env); - // Shortcut for `n == 0` where no matter what `alloc` and `offset` we produce, - // the result would be exactly what we early return here. - if n == 0 { - if ctor_wild_subpatterns.len() as u64 != n { - return None; - } - return Some(Fields::empty()); - } - match value.val { - ty::ConstKind::Value(ConstValue::ByRef { offset, alloc, .. }) => { - (Cow::Borrowed(alloc), offset, n, t) - } - _ => span_bug!(pat.span, "array pattern is {:?}", value,), - } - } - ty::Slice(t) => { - match value.val { - ty::ConstKind::Value(ConstValue::Slice { data, start, end }) => { - let offset = Size::from_bytes(start); - let n = (end - start) as u64; - (Cow::Borrowed(data), offset, n, t) - } - ty::ConstKind::Value(ConstValue::ByRef { .. }) => { - // FIXME(oli-obk): implement `deref` for `ConstValue` - return None; - } - _ => span_bug!( - pat.span, - "slice pattern constant must be scalar pair but is {:?}", - value, - ), - } - } - _ => span_bug!( - pat.span, - "unexpected const-val {:?} with ctor {:?}", - value, - constructor, - ), - }; - if ctor_wild_subpatterns.len() as u64 != n { - return None; - } - - // Convert a constant slice/array pattern to a list of patterns. - let layout = cx.tcx.layout_of(cx.param_env.and(ty)).ok()?; - let ptr = Pointer::new(AllocId(0), offset); - let pats = cx.pattern_arena.alloc_from_iter((0..n).filter_map(|i| { - let ptr = ptr.offset(layout.size * i, &cx.tcx).ok()?; - let scalar = alloc.read_scalar(&cx.tcx, ptr, layout.size).ok()?; - let scalar = scalar.check_init().ok()?; - let value = ty::Const::from_scalar(cx.tcx, scalar, ty); - let pattern = Pat { ty, span: pat.span, kind: box PatKind::Constant { value } }; - Some(pattern) - })); - // Ensure none of the dereferences failed. - if pats.len() as u64 != n { - return None; - } - Some(Fields::from_slice_unfiltered(pats)) + span_bug!(pat.span, "unexpected const-val {:?} with ctor {:?}", value, constructor) } PatKind::Constant { .. } | PatKind::Range { .. } => { @@ -2778,21 +2559,7 @@ fn specialize_one_pattern<'p, 'tcx>( let suffix = suffix.iter().enumerate().map(|(i, p)| (arity - suffix.len() + i, p)); Some(ctor_wild_subpatterns.replace_fields_indexed(prefix.chain(suffix))) } - ConstantValue(cv) => { - match slice_pat_covered_by_const( - cx.tcx, - pat.span, - cv, - prefix, - slice, - suffix, - cx.param_env, - ) { - Ok(true) => Some(Fields::empty()), - Ok(false) => None, - Err(ErrorReported) => None, - } - } + ConstantValue(_) => None, _ => span_bug!(pat.span, "unexpected ctor {:?} for slice pat", constructor), }, 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 047bf7db4c867..f623f3f631378 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -140,7 +140,7 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { patcx.include_lint_checks(); let pattern = patcx.lower_pattern(pat); let pattern_ty = pattern.ty; - let pattern: &_ = cx.pattern_arena.alloc(expand_pattern(cx, pattern)); + let pattern: &_ = cx.pattern_arena.alloc(expand_pattern(pattern)); if !patcx.errors.is_empty() { *have_errors = true; patcx.report_inlining_errors(pat.span); diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index 32b9b0a2d3b14..6370f8c375b2a 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -387,7 +387,6 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { // `&str` and `&[u8]` are represented as `ConstValue::Slice`, let's keep using this // optimization for now. ty::Str => PatKind::Constant { value: cv }, - ty::Slice(elem_ty) if elem_ty == tcx.types.u8 => PatKind::Constant { value: cv }, // `b"foo"` produces a `&[u8; 3]`, but you can't use constants of array type when // matching against references, you can only use byte string literals. // The typechecker has a special case for byte string literals, by treating them From 99852e0db6cdb62894e87eae2a41310b8400a5bf Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 17 Oct 2020 20:07:06 +0100 Subject: [PATCH 04/13] A ConstantValue constructor with a slice pattern is an error --- compiler/rustc_mir_build/src/thir/pattern/_match.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/_match.rs b/compiler/rustc_mir_build/src/thir/pattern/_match.rs index 9ca711722a307..d16ba1916819c 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/_match.rs @@ -2559,7 +2559,6 @@ fn specialize_one_pattern<'p, 'tcx>( let suffix = suffix.iter().enumerate().map(|(i, p)| (arity - suffix.len() + i, p)); Some(ctor_wild_subpatterns.replace_fields_indexed(prefix.chain(suffix))) } - ConstantValue(_) => None, _ => span_bug!(pat.span, "unexpected ctor {:?} for slice pat", constructor), }, From 3708c86de1a77d47f53fd376659184a64f3ce706 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 2 Oct 2020 14:19:52 +0200 Subject: [PATCH 05/13] Treat booleans as integers with valid range 0..=1 --- compiler/rustc_mir_build/src/thir/pattern/_match.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/_match.rs b/compiler/rustc_mir_build/src/thir/pattern/_match.rs index d16ba1916819c..ac5e114d45bf4 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/_match.rs @@ -1485,9 +1485,7 @@ fn all_constructors<'a, 'tcx>( ) }; match *pcx.ty.kind() { - ty::Bool => { - [true, false].iter().map(|&b| ConstantValue(ty::Const::from_bool(cx.tcx, b))).collect() - } + ty::Bool => vec![make_range(0, 1)], ty::Array(ref sub_ty, len) if len.try_eval_usize(cx.tcx, cx.param_env).is_some() => { let len = len.eval_usize(cx.tcx, cx.param_env); if len != 0 && cx.is_uninhabited(sub_ty) { @@ -1600,7 +1598,7 @@ impl<'tcx> IntRange<'tcx> { #[inline] fn is_integral(ty: Ty<'_>) -> bool { match ty.kind() { - ty::Char | ty::Int(_) | ty::Uint(_) => true, + ty::Char | ty::Int(_) | ty::Uint(_) | ty::Bool => true, _ => false, } } @@ -1622,6 +1620,7 @@ impl<'tcx> IntRange<'tcx> { #[inline] fn integral_size_and_signed_bias(tcx: TyCtxt<'tcx>, ty: Ty<'_>) -> Option<(Size, u128)> { match *ty.kind() { + ty::Bool => Some((Size::from_bytes(1), 0)), ty::Char => Some((Size::from_bytes(4), 0)), ty::Int(ity) => { let size = Integer::from_attr(&tcx, SignedInt(ity)).size(); From f504e9a42500db67d3c22b07f90df4d6ebc41758 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 17 Oct 2020 20:11:30 +0100 Subject: [PATCH 06/13] Fix comment --- compiler/rustc_mir_build/src/thir/pattern/_match.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/rustc_mir_build/src/thir/pattern/_match.rs b/compiler/rustc_mir_build/src/thir/pattern/_match.rs index ac5e114d45bf4..7124cad513ecc 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/_match.rs @@ -619,6 +619,7 @@ impl<'p, 'tcx> Matrix<'p, 'tcx> { /// +++++++++++++++++++++++++++++ /// + _ + [_, _, tail @ ..] + /// +++++++++++++++++++++++++++++ +/// ``` impl<'p, 'tcx> fmt::Debug for Matrix<'p, 'tcx> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "\n")?; From aa4172076ea9b886fefadafda6e479ab6c574bff Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 17 Oct 2020 23:12:28 +0100 Subject: [PATCH 07/13] Handle ranges of float consistently This deconfuses the comparison of floats, that currently mixed ranges and non-ranges. --- .../src/thir/pattern/_match.rs | 105 ++++++++---------- 1 file changed, 48 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/_match.rs b/compiler/rustc_mir_build/src/thir/pattern/_match.rs index 7124cad513ecc..6a37dd896b6c3 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/_match.rs @@ -834,13 +834,6 @@ enum Constructor<'tcx> { } impl<'tcx> Constructor<'tcx> { - fn is_slice(&self) -> bool { - match self { - Slice(_) => true, - _ => false, - } - } - fn variant_index_for_adt<'a>( &self, cx: &MatchCheckCtxt<'a, 'tcx>, @@ -2111,7 +2104,10 @@ fn pat_constructor<'tcx>( if let Some(int_range) = IntRange::from_const(tcx, param_env, value, pat.span) { Some(IntRange(int_range)) } else { - Some(ConstantValue(value)) + match value.ty.kind() { + ty::Float(_) => Some(FloatRange(value, value, RangeEnd::Included)), + _ => Some(ConstantValue(value)), + } } } PatKind::Range(PatRange { lo, hi, end }) => { @@ -2443,35 +2439,6 @@ fn lint_overlapping_patterns<'tcx>( } } -fn constructor_covered_by_range<'tcx>( - tcx: TyCtxt<'tcx>, - param_env: ty::ParamEnv<'tcx>, - ctor: &Constructor<'tcx>, - pat: &Pat<'tcx>, -) -> Option<()> { - if let Single = ctor { - return Some(()); - } - - let (pat_from, pat_to, pat_end, ty) = match *pat.kind { - PatKind::Constant { value } => (value, value, RangeEnd::Included, value.ty), - PatKind::Range(PatRange { lo, hi, end }) => (lo, hi, end, lo.ty), - _ => bug!("`constructor_covered_by_range` called with {:?}", pat), - }; - let (ctor_from, ctor_to, ctor_end) = match *ctor { - ConstantValue(value) => (value, value, RangeEnd::Included), - FloatRange(from, to, ctor_end) => (from, to, ctor_end), - _ => bug!("`constructor_covered_by_range` called with {:?}", ctor), - }; - trace!("constructor_covered_by_range {:#?}, {:#?}, {:#?}, {}", ctor, pat_from, pat_to, ty); - - let to = compare_const_vals(tcx, ctor_to, pat_to, param_env, ty)?; - let from = compare_const_vals(tcx, ctor_from, pat_from, param_env, ty)?; - let intersects = (from == Ordering::Greater || from == Ordering::Equal) - && (to == Ordering::Less || (pat_end == ctor_end && to == Ordering::Equal)); - if intersects { Some(()) } else { None } -} - /// This is the main specialization step. It expands the pattern /// into `arity` patterns based on the constructor. For most patterns, the step is trivial, /// for instance tuple patterns are flattened and box patterns expand into their inner pattern. @@ -2516,27 +2483,51 @@ fn specialize_one_pattern<'p, 'tcx>( PatKind::Deref { ref subpattern } => Some(Fields::from_single_pattern(subpattern)), - PatKind::Constant { value } if constructor.is_slice() => { - span_bug!(pat.span, "unexpected const-val {:?} with ctor {:?}", value, constructor) - } - PatKind::Constant { .. } | PatKind::Range { .. } => { - // If the constructor is a: - // - Single value: add a row if the pattern contains the constructor. - // - Range: add a row if the constructor intersects the pattern. - if let IntRange(ctor) = constructor { - let pat = IntRange::from_pat(cx.tcx, cx.param_env, pat)?; - ctor.intersection(cx.tcx, &pat)?; - // Constructor splitting should ensure that all intersections we encounter - // are actually inclusions. - assert!(ctor.is_subrange(&pat)); - } else { - // Fallback for non-ranges and ranges that involve - // floating-point numbers, which are not conveniently handled - // by `IntRange`. For these cases, the constructor may not be a - // range so intersection actually devolves into being covered - // by the pattern. - constructor_covered_by_range(cx.tcx, cx.param_env, constructor, pat)?; + match constructor { + Single => {} + IntRange(ctor) => { + let pat = IntRange::from_pat(cx.tcx, cx.param_env, pat)?; + ctor.intersection(cx.tcx, &pat)?; + // Constructor splitting should ensure that all intersections we encounter + // are actually inclusions. + assert!(ctor.is_subrange(&pat)); + } + FloatRange(ctor_from, ctor_to, ctor_end) => { + let (pat_from, pat_to, pat_end, ty) = match *pat.kind { + PatKind::Constant { value } => (value, value, RangeEnd::Included, value.ty), + PatKind::Range(PatRange { lo, hi, end }) => (lo, hi, end, lo.ty), + _ => unreachable!(), // This is ensured by the branch we're in + }; + let to = compare_const_vals(cx.tcx, ctor_to, pat_to, cx.param_env, ty)?; + let from = compare_const_vals(cx.tcx, ctor_from, pat_from, cx.param_env, ty)?; + let intersects = (from == Ordering::Greater || from == Ordering::Equal) + && (to == Ordering::Less + || (pat_end == *ctor_end && to == Ordering::Equal)); + if !intersects { + return None; + } + } + ConstantValue(ctor_value) => { + let pat_value = match *pat.kind { + PatKind::Constant { value } => value, + _ => span_bug!( + pat.span, + "unexpected range pattern {:?} for constant value ctor", + pat + ), + }; + + // FIXME: there's probably a more direct way of comparing for equality + if compare_const_vals(cx.tcx, ctor_value, pat_value, cx.param_env, pat.ty)? + != Ordering::Equal + { + return None; + } + } + _ => { + span_bug!(pat.span, "unexpected pattern {:?} with ctor {:?}", pat, constructor) + } } Some(Fields::empty()) } From d1a784e7b9c4cfe9ce216eb89c362726ff890ea0 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 17 Oct 2020 23:18:05 +0100 Subject: [PATCH 08/13] Treat string literals separately from other constants --- compiler/rustc_mir_build/src/thir/pattern/_match.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/_match.rs b/compiler/rustc_mir_build/src/thir/pattern/_match.rs index 6a37dd896b6c3..31ec83a22baf4 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/_match.rs @@ -827,6 +827,8 @@ enum Constructor<'tcx> { IntRange(IntRange<'tcx>), /// Ranges of floating-point literal values (`2.0..=5.2`). FloatRange(&'tcx ty::Const<'tcx>, &'tcx ty::Const<'tcx>, RangeEnd), + /// String literals. Strings are not quite the same as `&[u8]` so we treat them separately. + Str(&'tcx ty::Const<'tcx>), /// Array and slice patterns. Slice(Slice), /// Fake extra constructor for enums that aren't allowed to be matched exhaustively. @@ -863,7 +865,7 @@ impl<'tcx> Constructor<'tcx> { match self { // Those constructors can only match themselves. - Single | Variant(_) | ConstantValue(..) | FloatRange(..) => { + Single | Variant(_) | ConstantValue(..) | Str(..) | FloatRange(..) => { if other_ctors.iter().any(|c| c == self) { vec![] } else { vec![self.clone()] } } &Slice(slice) => { @@ -1013,6 +1015,7 @@ impl<'tcx> Constructor<'tcx> { } }, &ConstantValue(value) => PatKind::Constant { value }, + &Str(value) => PatKind::Constant { value }, &FloatRange(lo, hi, end) => PatKind::Range(PatRange { lo, hi, end }), IntRange(range) => return range.to_pat(cx.tcx), NonExhaustive => PatKind::Wild, @@ -1167,7 +1170,9 @@ impl<'p, 'tcx> Fields<'p, 'tcx> { } _ => bug!("bad slice pattern {:?} {:?}", constructor, ty), }, - ConstantValue(..) | FloatRange(..) | IntRange(..) | NonExhaustive => Fields::empty(), + ConstantValue(..) | Str(..) | FloatRange(..) | IntRange(..) | NonExhaustive => { + Fields::empty() + } }; debug!("Fields::wildcards({:?}, {:?}) = {:#?}", constructor, ty, ret); ret @@ -2106,6 +2111,7 @@ fn pat_constructor<'tcx>( } else { match value.ty.kind() { ty::Float(_) => Some(FloatRange(value, value, RangeEnd::Included)), + ty::Ref(_, t, _) if t.is_str() => Some(Str(value)), _ => Some(ConstantValue(value)), } } @@ -2508,7 +2514,7 @@ fn specialize_one_pattern<'p, 'tcx>( return None; } } - ConstantValue(ctor_value) => { + ConstantValue(ctor_value) | Str(ctor_value) => { let pat_value = match *pat.kind { PatKind::Constant { value } => value, _ => span_bug!( From da0ba2f64574e0ecca65d0d1045af00abe1515fd Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 18 Oct 2020 13:48:54 +0100 Subject: [PATCH 09/13] The only remaining constant patterns are opaque --- .../src/thir/pattern/_match.rs | 69 +++++++++++-------- .../rustc_mir_build/src/thir/pattern/mod.rs | 7 ++ .../ui/pattern/usefulness/consts-opaque.rs | 3 +- .../pattern/usefulness/consts-opaque.stderr | 8 +-- 4 files changed, 50 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/_match.rs b/compiler/rustc_mir_build/src/thir/pattern/_match.rs index 31ec83a22baf4..fb068f21d8a69 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/_match.rs @@ -394,9 +394,15 @@ impl<'p, 'tcx> PatStack<'p, 'tcx> { cx: &mut MatchCheckCtxt<'p, 'tcx>, constructor: &Constructor<'tcx>, ctor_wild_subpatterns: &Fields<'p, 'tcx>, + is_my_head_ctor: bool, ) -> Option> { - let new_fields = - specialize_one_pattern(cx, self.head(), constructor, ctor_wild_subpatterns)?; + let new_fields = specialize_one_pattern( + cx, + self.head(), + constructor, + ctor_wild_subpatterns, + is_my_head_ctor, + )?; Some(new_fields.push_on_patstack(&self.0[1..])) } } @@ -574,6 +580,7 @@ impl<'p, 'tcx> Matrix<'p, 'tcx> { cx, constructor, ctor_wild_subpatterns, + false, ) }) .collect() @@ -599,7 +606,9 @@ impl<'p, 'tcx> Matrix<'p, 'tcx> { SpecializationCache::Incompatible => self .patterns .iter() - .filter_map(|r| r.specialize_constructor(cx, constructor, ctor_wild_subpatterns)) + .filter_map(|r| { + r.specialize_constructor(cx, constructor, ctor_wild_subpatterns, false) + }) .collect(), } } @@ -821,8 +830,6 @@ enum Constructor<'tcx> { Single, /// Enum variants. Variant(DefId), - /// Literal values. - ConstantValue(&'tcx ty::Const<'tcx>), /// Ranges of integer literal values (`2`, `2..=5` or `2..5`). IntRange(IntRange<'tcx>), /// Ranges of floating-point literal values (`2.0..=5.2`). @@ -831,27 +838,22 @@ enum Constructor<'tcx> { Str(&'tcx ty::Const<'tcx>), /// Array and slice patterns. Slice(Slice), + /// Constants that must not be matched structurally. They are treated as black + /// boxes for the purposes of exhaustiveness: we must not inspect them, and they + /// don't count towards making a match exhaustive. + Opaque, /// Fake extra constructor for enums that aren't allowed to be matched exhaustively. NonExhaustive, } impl<'tcx> Constructor<'tcx> { - fn variant_index_for_adt<'a>( - &self, - cx: &MatchCheckCtxt<'a, 'tcx>, - adt: &'tcx ty::AdtDef, - ) -> VariantIdx { + fn variant_index_for_adt(&self, adt: &'tcx ty::AdtDef) -> VariantIdx { match *self { Variant(id) => adt.variant_index_with_id(id), Single => { assert!(!adt.is_enum()); VariantIdx::new(0) } - ConstantValue(c) => cx - .tcx - .destructure_const(cx.param_env.and(c)) - .variant - .expect("destructed const of adt without variant id"), _ => bug!("bad constructor {:?} for adt {:?}", self, adt), } } @@ -865,7 +867,7 @@ impl<'tcx> Constructor<'tcx> { match self { // Those constructors can only match themselves. - Single | Variant(_) | ConstantValue(..) | Str(..) | FloatRange(..) => { + Single | Variant(_) | Str(..) | FloatRange(..) => { if other_ctors.iter().any(|c| c == self) { vec![] } else { vec![self.clone()] } } &Slice(slice) => { @@ -936,6 +938,7 @@ impl<'tcx> Constructor<'tcx> { } // This constructor is never covered by anything else NonExhaustive => vec![NonExhaustive], + Opaque => bug!("unexpected opaque ctor {:?} found in all_ctors", self), } } @@ -975,7 +978,7 @@ impl<'tcx> Constructor<'tcx> { PatKind::Variant { adt_def: adt, substs, - variant_index: self.variant_index_for_adt(cx, adt), + variant_index: self.variant_index_for_adt(adt), subpatterns, } } else { @@ -1014,11 +1017,11 @@ impl<'tcx> Constructor<'tcx> { PatKind::Slice { prefix, slice: Some(wild), suffix } } }, - &ConstantValue(value) => PatKind::Constant { value }, &Str(value) => PatKind::Constant { value }, &FloatRange(lo, hi, end) => PatKind::Range(PatRange { lo, hi, end }), IntRange(range) => return range.to_pat(cx.tcx), NonExhaustive => PatKind::Wild, + Opaque => bug!("we should not try to apply an opaque constructor {:?}", self), }; Pat { ty, span: DUMMY_SP, kind: Box::new(pat) } @@ -1122,7 +1125,7 @@ impl<'p, 'tcx> Fields<'p, 'tcx> { // Use T as the sub pattern type of Box. Fields::from_single_pattern(wildcard_from_ty(substs.type_at(0))) } else { - let variant = &adt.variants[constructor.variant_index_for_adt(cx, adt)]; + let variant = &adt.variants[constructor.variant_index_for_adt(adt)]; // Whether we must not match the fields of this variant exhaustively. let is_non_exhaustive = variant.is_field_list_non_exhaustive() && !adt.did.is_local(); @@ -1170,9 +1173,7 @@ impl<'p, 'tcx> Fields<'p, 'tcx> { } _ => bug!("bad slice pattern {:?} {:?}", constructor, ty), }, - ConstantValue(..) | Str(..) | FloatRange(..) | IntRange(..) | NonExhaustive => { - Fields::empty() - } + Str(..) | FloatRange(..) | IntRange(..) | NonExhaustive | Opaque => Fields::empty(), }; debug!("Fields::wildcards({:?}, {:?}) = {:#?}", constructor, ty, ret); ret @@ -2085,7 +2086,7 @@ fn is_useful_specialized<'p, 'tcx>( // We cache the result of `Fields::wildcards` because it is used a lot. let ctor_wild_subpatterns = Fields::wildcards(cx, &ctor, ty); let matrix = matrix.specialize_constructor(cx, &ctor, &ctor_wild_subpatterns); - v.specialize_constructor(cx, &ctor, &ctor_wild_subpatterns) + v.specialize_constructor(cx, &ctor, &ctor_wild_subpatterns, true) .map(|v| is_useful(cx, &matrix, &v, witness_preference, hir_id, is_under_guard, false)) .map(|u| u.apply_constructor(cx, &ctor, ty, &ctor_wild_subpatterns)) .unwrap_or(NotUseful) @@ -2112,7 +2113,7 @@ fn pat_constructor<'tcx>( match value.ty.kind() { ty::Float(_) => Some(FloatRange(value, value, RangeEnd::Included)), ty::Ref(_, t, _) if t.is_str() => Some(Str(value)), - _ => Some(ConstantValue(value)), + _ => Some(Opaque), } } } @@ -2461,15 +2462,26 @@ fn specialize_one_pattern<'p, 'tcx>( pat: &'p Pat<'tcx>, constructor: &Constructor<'tcx>, ctor_wild_subpatterns: &Fields<'p, 'tcx>, + is_its_own_ctor: bool, // Whether `ctor` is known to be derived from `pat` ) -> Option> { if let NonExhaustive = constructor { - // Only a wildcard pattern can match the special extra constructor + // Only a wildcard pattern can match the special extra constructor. if !pat.is_wildcard() { return None; } return Some(Fields::empty()); } + if let Opaque = constructor { + // Only a wildcard pattern can match an opaque constant, unless we're specializing the + // value against its own constructor. + if is_its_own_ctor || pat.is_wildcard() { + return Some(Fields::empty()); + } else { + return None; + } + } + let result = match *pat.kind { PatKind::AscribeUserType { .. } => bug!(), // Handled by `expand_pattern` @@ -2491,7 +2503,6 @@ fn specialize_one_pattern<'p, 'tcx>( PatKind::Constant { .. } | PatKind::Range { .. } => { match constructor { - Single => {} IntRange(ctor) => { let pat = IntRange::from_pat(cx.tcx, cx.param_env, pat)?; ctor.intersection(cx.tcx, &pat)?; @@ -2514,7 +2525,7 @@ fn specialize_one_pattern<'p, 'tcx>( return None; } } - ConstantValue(ctor_value) | Str(ctor_value) => { + Str(ctor_value) => { let pat_value = match *pat.kind { PatKind::Constant { value } => value, _ => span_bug!( @@ -2532,7 +2543,9 @@ fn specialize_one_pattern<'p, 'tcx>( } } _ => { - span_bug!(pat.span, "unexpected pattern {:?} with ctor {:?}", pat, constructor) + // If we reach here, we must be trying to inspect an opaque constant. Thus we skip + // the row. + return None; } } Some(Fields::empty()) diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs index 718ed78889f09..485e4cab87f82 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs @@ -158,6 +158,13 @@ crate enum PatKind<'tcx> { subpattern: Pat<'tcx>, }, + /// One of the following: + /// * `&str`, which will be handled as a string pattern and thus exhaustiveness + /// checking will detect if you use the same string twice in different patterns. + /// * integer, bool, char or float, which will be handled by exhaustivenes to cover exactly + /// its own value, similar to `&str`, but these values are much simpler. + /// * Opaque constants, that must not be matched structurally. So anything that does not derive + /// `PartialEq` and `Eq`. Constant { value: &'tcx ty::Const<'tcx>, }, diff --git a/src/test/ui/pattern/usefulness/consts-opaque.rs b/src/test/ui/pattern/usefulness/consts-opaque.rs index 928756723a6c9..761b293e9989b 100644 --- a/src/test/ui/pattern/usefulness/consts-opaque.rs +++ b/src/test/ui/pattern/usefulness/consts-opaque.rs @@ -106,8 +106,7 @@ fn main() { match QUUX { QUUX => {} - QUUX => {} // should not be emitting unreachable warning - //~^ ERROR unreachable pattern + QUUX => {} _ => {} } } diff --git a/src/test/ui/pattern/usefulness/consts-opaque.stderr b/src/test/ui/pattern/usefulness/consts-opaque.stderr index 07cdc1c95fad0..6d7e36e01f7f7 100644 --- a/src/test/ui/pattern/usefulness/consts-opaque.stderr +++ b/src/test/ui/pattern/usefulness/consts-opaque.stderr @@ -144,11 +144,5 @@ error: unreachable pattern LL | _ => {} // should not be emitting unreachable warning | ^ -error: unreachable pattern - --> $DIR/consts-opaque.rs:109:9 - | -LL | QUUX => {} // should not be emitting unreachable warning - | ^^^^ - -error: aborting due to 23 previous errors +error: aborting due to 22 previous errors From c4ae6c2bb95551afed4b87346cdf4ad4e4ba052c Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 18 Oct 2020 17:05:19 +0100 Subject: [PATCH 10/13] Add comment --- compiler/rustc_mir_build/src/thir/pattern/_match.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/rustc_mir_build/src/thir/pattern/_match.rs b/compiler/rustc_mir_build/src/thir/pattern/_match.rs index fb068f21d8a69..a7ddca1b6b632 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/_match.rs @@ -2113,6 +2113,9 @@ fn pat_constructor<'tcx>( match value.ty.kind() { ty::Float(_) => Some(FloatRange(value, value, RangeEnd::Included)), ty::Ref(_, t, _) if t.is_str() => Some(Str(value)), + // All constants that can be structurally matched have already been expanded + // into the corresponding `Pat`s by `const_to_pat`. Constants that remain are + // opaque. _ => Some(Opaque), } } From 5bfd3e7259caa4b94f9cfcf2af9ccc1d8420e286 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 20 Oct 2020 00:06:00 +0100 Subject: [PATCH 11/13] Accidentally fixed #78071 --- .../ui/pattern/usefulness/consts-opaque.rs | 12 +++-- .../pattern/usefulness/consts-opaque.stderr | 48 +++++++++++-------- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/test/ui/pattern/usefulness/consts-opaque.rs b/src/test/ui/pattern/usefulness/consts-opaque.rs index 761b293e9989b..f87f96e34fccd 100644 --- a/src/test/ui/pattern/usefulness/consts-opaque.rs +++ b/src/test/ui/pattern/usefulness/consts-opaque.rs @@ -44,11 +44,13 @@ fn main() { //~^ ERROR unreachable pattern } - // FIXME: this causes an ICE (https://github.com/rust-lang/rust/issues/78071) - //match FOO_REF_REF { - // FOO_REF_REF => {} - // Foo(_) => {} - //} + // This used to cause an ICE (https://github.com/rust-lang/rust/issues/78071) + match FOO_REF_REF { + FOO_REF_REF => {} + //~^ WARNING must be annotated with `#[derive(PartialEq, Eq)]` + //~| WARNING this was previously accepted by the compiler but is being phased out + Foo(_) => {} + } match BAR { Bar => {} diff --git a/src/test/ui/pattern/usefulness/consts-opaque.stderr b/src/test/ui/pattern/usefulness/consts-opaque.stderr index 6d7e36e01f7f7..f10166d5a3580 100644 --- a/src/test/ui/pattern/usefulness/consts-opaque.stderr +++ b/src/test/ui/pattern/usefulness/consts-opaque.stderr @@ -28,14 +28,24 @@ error: unreachable pattern LL | Foo(_) => {} // should not be emitting unreachable warning | ^^^^^^ +warning: to use a constant of type `Foo` in a pattern, `Foo` must be annotated with `#[derive(PartialEq, Eq)]` + --> $DIR/consts-opaque.rs:49:9 + | +LL | FOO_REF_REF => {} + | ^^^^^^^^^^^ + | + = note: `#[warn(indirect_structural_match)]` on by default + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #62411 + error: to use a constant of type `Bar` in a pattern, `Bar` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/consts-opaque.rs:55:9 + --> $DIR/consts-opaque.rs:57:9 | LL | BAR => {} // should not be emitting unreachable warning | ^^^ error: unreachable pattern - --> $DIR/consts-opaque.rs:55:9 + --> $DIR/consts-opaque.rs:57:9 | LL | Bar => {} | --- matches any value @@ -43,7 +53,7 @@ LL | BAR => {} // should not be emitting unreachable warning | ^^^ unreachable pattern error: unreachable pattern - --> $DIR/consts-opaque.rs:58:9 + --> $DIR/consts-opaque.rs:60:9 | LL | Bar => {} | --- matches any value @@ -52,19 +62,19 @@ LL | _ => {} | ^ unreachable pattern error: to use a constant of type `Bar` in a pattern, `Bar` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/consts-opaque.rs:63:9 + --> $DIR/consts-opaque.rs:65:9 | LL | BAR => {} | ^^^ error: unreachable pattern - --> $DIR/consts-opaque.rs:65:9 + --> $DIR/consts-opaque.rs:67:9 | LL | Bar => {} // should not be emitting unreachable warning | ^^^ error: unreachable pattern - --> $DIR/consts-opaque.rs:67:9 + --> $DIR/consts-opaque.rs:69:9 | LL | Bar => {} // should not be emitting unreachable warning | --- matches any value @@ -73,76 +83,76 @@ LL | _ => {} | ^ unreachable pattern error: to use a constant of type `Bar` in a pattern, `Bar` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/consts-opaque.rs:72:9 + --> $DIR/consts-opaque.rs:74:9 | LL | BAR => {} | ^^^ error: to use a constant of type `Bar` in a pattern, `Bar` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/consts-opaque.rs:74:9 + --> $DIR/consts-opaque.rs:76:9 | LL | BAR => {} // should not be emitting unreachable warning | ^^^ error: unreachable pattern - --> $DIR/consts-opaque.rs:74:9 + --> $DIR/consts-opaque.rs:76:9 | LL | BAR => {} // should not be emitting unreachable warning | ^^^ error: unreachable pattern - --> $DIR/consts-opaque.rs:77:9 + --> $DIR/consts-opaque.rs:79:9 | LL | _ => {} // should not be emitting unreachable warning | ^ error: to use a constant of type `Baz` in a pattern, `Baz` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/consts-opaque.rs:82:9 + --> $DIR/consts-opaque.rs:84:9 | LL | BAZ => {} | ^^^ error: unreachable pattern - --> $DIR/consts-opaque.rs:84:9 + --> $DIR/consts-opaque.rs:86:9 | LL | Baz::Baz1 => {} // should not be emitting unreachable warning | ^^^^^^^^^ error: unreachable pattern - --> $DIR/consts-opaque.rs:86:9 + --> $DIR/consts-opaque.rs:88:9 | LL | _ => {} | ^ error: to use a constant of type `Baz` in a pattern, `Baz` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/consts-opaque.rs:92:9 + --> $DIR/consts-opaque.rs:94:9 | LL | BAZ => {} | ^^^ error: unreachable pattern - --> $DIR/consts-opaque.rs:94:9 + --> $DIR/consts-opaque.rs:96:9 | LL | _ => {} | ^ error: to use a constant of type `Baz` in a pattern, `Baz` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/consts-opaque.rs:99:9 + --> $DIR/consts-opaque.rs:101:9 | LL | BAZ => {} | ^^^ error: unreachable pattern - --> $DIR/consts-opaque.rs:101:9 + --> $DIR/consts-opaque.rs:103:9 | LL | Baz::Baz2 => {} // should not be emitting unreachable warning | ^^^^^^^^^ error: unreachable pattern - --> $DIR/consts-opaque.rs:103:9 + --> $DIR/consts-opaque.rs:105:9 | LL | _ => {} // should not be emitting unreachable warning | ^ -error: aborting due to 22 previous errors +error: aborting due to 22 previous errors; 1 warning emitted From 35194117006d987d59dabb44ee3e4f38281c9e1b Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 21 Oct 2020 19:24:59 +0100 Subject: [PATCH 12/13] Add a test for #53708 This issue was accidentally fixed recently, probably by #70743 --- src/test/ui/consts/const_in_pattern/issue-53708.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 src/test/ui/consts/const_in_pattern/issue-53708.rs diff --git a/src/test/ui/consts/const_in_pattern/issue-53708.rs b/src/test/ui/consts/const_in_pattern/issue-53708.rs new file mode 100644 index 0000000000000..355ba63790f3b --- /dev/null +++ b/src/test/ui/consts/const_in_pattern/issue-53708.rs @@ -0,0 +1,11 @@ +// check-pass +// https://github.com/rust-lang/rust/issues/53708 +#[derive(PartialEq, Eq)] +struct S; + +fn main() { + const C: &S = &S; + match C { + C => {} + } +} From faf87105db54a399bc598765528dec818b63a54a Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 21 Oct 2020 20:15:02 +0100 Subject: [PATCH 13/13] Explain the `Opaque` special case in specialization --- .../src/thir/pattern/_match.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/_match.rs b/compiler/rustc_mir_build/src/thir/pattern/_match.rs index a7ddca1b6b632..4c69339795d40 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/_match.rs @@ -2477,7 +2477,24 @@ fn specialize_one_pattern<'p, 'tcx>( if let Opaque = constructor { // Only a wildcard pattern can match an opaque constant, unless we're specializing the - // value against its own constructor. + // value against its own constructor. That happens when we call + // `v.specialize_constructor(ctor)` with `ctor` obtained from `pat_constructor(v.head())`. + // For example, in the following match, when we are dealing with the third branch, we will + // specialize with an `Opaque` ctor. We want to ignore the second branch because opaque + // constants should not be inspected, but we don't want to ignore the current (third) + // branch, as that would cause us to always conclude that such a branch is unreachable. + // ```rust + // #[derive(PartialEq)] + // struct Foo(i32); + // impl Eq for Foo {} + // const FOO: Foo = Foo(42); + // + // match (Foo(0), true) { + // (_, true) => {} + // (FOO, true) => {} + // (FOO, false) => {} + // } + // ``` if is_its_own_ctor || pat.is_wildcard() { return Some(Fields::empty()); } else {