From 002a1c21faf33f55a5a4c2b937531d9a941041ef Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 29 Sep 2020 18:28:39 +0200 Subject: [PATCH 1/7] 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 14a0b103fadf73bb13d3e5b7a4fcc1fc157c8971 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 1 Oct 2020 09:24:44 +0200 Subject: [PATCH 2/7] 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 e0e48ac1b2d1757453547b1494cf07e053868149 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 2 Oct 2020 14:19:52 +0200 Subject: [PATCH 3/7] 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 9ca711722a307..0ebbcd9d61239 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 523b752e55797c718f52432daaf34b7f2ba247a1 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 2 Oct 2020 14:51:15 +0200 Subject: [PATCH 4/7] Stop passing opaque constants around in the exhaustiveness checks and just treat them as nonexhaustive --- .../src/thir/pattern/_match.rs | 36 +++++++++---------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/_match.rs b/compiler/rustc_mir_build/src/thir/pattern/_match.rs index 0ebbcd9d61239..e832da97e3295 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/_match.rs @@ -820,8 +820,8 @@ enum Constructor<'tcx> { Single, /// Enum variants. Variant(DefId), - /// Literal values. - ConstantValue(&'tcx ty::Const<'tcx>), + /// String literals + Str(&'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`). @@ -840,22 +840,13 @@ 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), } } @@ -869,7 +860,7 @@ impl<'tcx> Constructor<'tcx> { match self { // Those constructors can only match themselves. - Single | Variant(_) | ConstantValue(..) | FloatRange(..) => { + Single | Variant(_) | Str(_) | FloatRange(..) => { if other_ctors.iter().any(|c| c == self) { vec![] } else { vec![self.clone()] } } &Slice(slice) => { @@ -979,7 +970,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 { @@ -1018,7 +1009,7 @@ 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, @@ -1125,7 +1116,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(); @@ -1173,7 +1164,7 @@ impl<'p, 'tcx> Fields<'p, 'tcx> { } _ => bug!("bad slice pattern {:?} {:?}", constructor, ty), }, - ConstantValue(..) | FloatRange(..) | IntRange(..) | NonExhaustive => Fields::empty(), + Str(_) | FloatRange(..) | IntRange(..) | NonExhaustive => Fields::empty(), }; debug!("Fields::wildcards({:?}, {:?}) = {:#?}", constructor, ty, ret); ret @@ -2110,7 +2101,12 @@ 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::Ref(_, t, _) if t.is_str() => Some(Str(value)), + ty::Float(_) => Some(FloatRange(value, value, RangeEnd::Included)), + // Non structural-match values are opaque. + _ => None, + } } } PatKind::Range(PatRange { lo, hi, end }) => { @@ -2458,7 +2454,7 @@ fn constructor_covered_by_range<'tcx>( _ => bug!("`constructor_covered_by_range` called with {:?}", pat), }; let (ctor_from, ctor_to, ctor_end) = match *ctor { - ConstantValue(value) => (value, value, RangeEnd::Included), + Str(value) => (value, value, RangeEnd::Included), FloatRange(from, to, ctor_end) => (from, to, ctor_end), _ => bug!("`constructor_covered_by_range` called with {:?}", ctor), }; @@ -2558,7 +2554,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(_) => None, + Str(_) => None, _ => span_bug!(pat.span, "unexpected ctor {:?} for slice pat", constructor), }, From 6553d38bcb0c9193808ae93db8bd95bcf4c4824a Mon Sep 17 00:00:00 2001 From: oli Date: Mon, 12 Oct 2020 13:09:47 +0000 Subject: [PATCH 5/7] Document new (or new uses) of enum variants --- compiler/rustc_mir_build/src/thir/pattern/_match.rs | 6 +++++- compiler/rustc_mir_build/src/thir/pattern/mod.rs | 3 +++ 2 files changed, 8 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 e832da97e3295..3ee962e4186f7 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/_match.rs @@ -820,7 +820,11 @@ enum Constructor<'tcx> { Single, /// Enum variants. Variant(DefId), - /// String literals + /// String literals. These have their own variant, because they behave essentially like + /// `&[u8]`, but there's no way to pattern match on parts of strings. So we could convert them + /// to slice constructors, but then we'd have to special case other code sites to not treat them + /// *exactly* like slice constructors. It seems simpler overall to just give them their own + /// variant. Str(&'tcx ty::Const<'tcx>), /// Ranges of integer literal values (`2`, `2..=5` or `2..5`). IntRange(IntRange<'tcx>), diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs index 718ed78889f09..272084ad66ba6 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs @@ -158,6 +158,9 @@ crate enum PatKind<'tcx> { subpattern: Pat<'tcx>, }, + /// Opaque constant. This will be treated as a single, but unknown value. + /// Except for `&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. Constant { value: &'tcx ty::Const<'tcx>, }, From d0bb77da5d7ff6666eccccbb9c29ad28a1837203 Mon Sep 17 00:00:00 2001 From: oli Date: Thu, 15 Oct 2020 14:30:59 +0000 Subject: [PATCH 6/7] Update comment --- compiler/rustc_mir_build/src/thir/pattern/mod.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs index 272084ad66ba6..3453622585b5a 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs @@ -158,9 +158,12 @@ crate enum PatKind<'tcx> { subpattern: Pat<'tcx>, }, - /// Opaque constant. This will be treated as a single, but unknown value. - /// Except for `&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. + /// 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. So anything that does not derive `PartialEq` and `Eq`. Constant { value: &'tcx ty::Const<'tcx>, }, From 091506325ddcd7ab7e59ecfa9f6338d984468434 Mon Sep 17 00:00:00 2001 From: oli Date: Thu, 15 Oct 2020 14:31:18 +0000 Subject: [PATCH 7/7] WIP: Add opaque constructor --- .../rustc_mir_build/src/thir/pattern/_match.rs | 12 ++++++++++-- .../ui/consts/const_in_pattern/ice-regression.rs | 16 ++++++++++++++++ .../const_in_pattern/ice-regression.stderr | 8 ++++++++ src/test/ui/issues/issue-44333.stderr | 10 +++++++++- 4 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/consts/const_in_pattern/ice-regression.rs create mode 100644 src/test/ui/consts/const_in_pattern/ice-regression.stderr diff --git a/compiler/rustc_mir_build/src/thir/pattern/_match.rs b/compiler/rustc_mir_build/src/thir/pattern/_match.rs index 3ee962e4186f7..4b875236b944d 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/_match.rs @@ -834,6 +834,9 @@ enum Constructor<'tcx> { Slice(Slice), /// Fake extra constructor for enums that aren't allowed to be matched exhaustively. NonExhaustive, + /// Constants of types or values that may not be used in exhaustiveness. + /// This will by itself be ok, but it will always require a wildcard pattern somewhere later. + Opaque(&'tcx ty::Const<'tcx>), } impl<'tcx> Constructor<'tcx> { @@ -935,6 +938,7 @@ impl<'tcx> Constructor<'tcx> { } // This constructor is never covered by anything else NonExhaustive => vec![NonExhaustive], + Opaque(val) => vec![Opaque(val)], } } @@ -1016,6 +1020,7 @@ impl<'tcx> Constructor<'tcx> { &Str(value) => PatKind::Constant { value }, &FloatRange(lo, hi, end) => PatKind::Range(PatRange { lo, hi, end }), IntRange(range) => return range.to_pat(cx.tcx), + Opaque(value) => PatKind::Constant { value }, NonExhaustive => PatKind::Wild, }; @@ -1168,7 +1173,7 @@ impl<'p, 'tcx> Fields<'p, 'tcx> { } _ => bug!("bad slice pattern {:?} {:?}", constructor, ty), }, - Str(_) | FloatRange(..) | IntRange(..) | NonExhaustive => Fields::empty(), + Str(_) | FloatRange(..) | IntRange(..) | NonExhaustive | Opaque(_) => Fields::empty(), }; debug!("Fields::wildcards({:?}, {:?}) = {:#?}", constructor, ty, ret); ret @@ -2109,7 +2114,7 @@ fn pat_constructor<'tcx>( ty::Ref(_, t, _) if t.is_str() => Some(Str(value)), ty::Float(_) => Some(FloatRange(value, value, RangeEnd::Included)), // Non structural-match values are opaque. - _ => None, + _ => Some(Opaque(value)), } } } @@ -2495,6 +2500,9 @@ fn specialize_one_pattern<'p, 'tcx>( } return Some(Fields::empty()); } + if let Opaque(_) = constructor { + return Some(Fields::empty()); + } let result = match *pat.kind { PatKind::AscribeUserType { .. } => bug!(), // Handled by `expand_pattern` diff --git a/src/test/ui/consts/const_in_pattern/ice-regression.rs b/src/test/ui/consts/const_in_pattern/ice-regression.rs new file mode 100644 index 0000000000000..198515847e820 --- /dev/null +++ b/src/test/ui/consts/const_in_pattern/ice-regression.rs @@ -0,0 +1,16 @@ + + +#[derive(PartialEq)] +struct Opaque(i32); + +impl Eq for Opaque {} + +const FOO: &&Opaque = &&Opaque(42); + +fn main() { + match FOO { + FOO => {}, //~ WARN must be annotated with + // The following should not lint about unreachable + Opaque(42) => {} + } +} diff --git a/src/test/ui/consts/const_in_pattern/ice-regression.stderr b/src/test/ui/consts/const_in_pattern/ice-regression.stderr new file mode 100644 index 0000000000000..4e101af458348 --- /dev/null +++ b/src/test/ui/consts/const_in_pattern/ice-regression.stderr @@ -0,0 +1,8 @@ +error: expected item, found keyword `match` + --> $DIR/ice-regression.rs:8:1 + | +LL | match FOO { + | ^^^^^ expected item + +error: aborting due to previous error + diff --git a/src/test/ui/issues/issue-44333.stderr b/src/test/ui/issues/issue-44333.stderr index 8302b09e5334d..4ab5b2e21f656 100644 --- a/src/test/ui/issues/issue-44333.stderr +++ b/src/test/ui/issues/issue-44333.stderr @@ -21,5 +21,13 @@ LL | BAR => println!("bar"), = 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 -warning: 2 warnings emitted +warning: unreachable pattern + --> $DIR/issue-44333.rs:21:9 + | +LL | BAR => println!("bar"), + | ^^^ + | + = note: `#[warn(unreachable_patterns)]` on by default + +warning: 3 warnings emitted