Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix exhaustiveness checking of strings #78553

Merged
merged 2 commits into from
Nov 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 55 additions & 32 deletions compiler/rustc_mir_build/src/thir/pattern/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,9 +327,23 @@ struct LiteralExpander;
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) {
(_, &PatKind::Binding { subpattern: Some(ref s), .. }) => s.fold_with(self),
(_, &PatKind::AscribeUserType { subpattern: ref s, .. }) => s.fold_with(self),
match (pat.ty.kind(), pat.kind.as_ref()) {
(_, PatKind::Binding { subpattern: Some(s), .. }) => s.fold_with(self),
(_, PatKind::AscribeUserType { subpattern: s, .. }) => s.fold_with(self),
(ty::Ref(_, t, _), PatKind::Constant { .. }) if t.is_str() => {
// Treat string literal patterns as deref patterns to a `str` constant, i.e.
// `&CONST`. This expands them like other const patterns. This could have been done
// in `const_to_pat`, but that causes issues with the rest of the matching code.
let mut new_pat = pat.super_fold_with(self);
// Make a fake const pattern of type `str` (instead of `&str`). That the carried
// constant value still knows it is of type `&str`.
new_pat.ty = t;
Pat {
kind: Box::new(PatKind::Deref { subpattern: new_pat }),
span: pat.span,
ty: pat.ty,
}
}
_ => pat.super_fold_with(self),
}
}
Expand Down Expand Up @@ -785,11 +799,9 @@ enum Constructor<'tcx> {
/// 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.
/// Fake extra constructor for enums that aren't allowed to be matched exhaustively. Also used
/// for those types for which we cannot list constructors explicitly, like `f64` and `str`.
NonExhaustive,
/// Fake constructor for those types for which we can't list constructors explicitly, like
/// `f64` and `&str`.
Unlistable,
/// Wildcard pattern.
Wildcard,
}
Expand Down Expand Up @@ -883,6 +895,7 @@ impl<'tcx> Constructor<'tcx> {
/// For the simple cases, this is simply checking for equality. For the "grouped" constructors,
/// this checks for inclusion.
fn is_covered_by<'p>(&self, pcx: PatCtxt<'_, 'p, 'tcx>, other: &Self) -> bool {
// This must be kept in sync with `is_covered_by_any`.
match (self, other) {
// Wildcards cover anything
(_, Wildcard) => true,
Expand Down Expand Up @@ -925,18 +938,19 @@ impl<'tcx> Constructor<'tcx> {
(Opaque, _) | (_, Opaque) => false,
// Only a wildcard pattern can match the special extra constructor.
(NonExhaustive, _) => false,
// If we encounter a `Single` here, this means there was only one constructor for this
// type after all.
(Unlistable, Single) => true,
// Otherwise, only a wildcard pattern can match the special extra constructor.
(Unlistable, _) => false,

_ => bug!("trying to compare incompatible constructors {:?} and {:?}", self, other),
_ => span_bug!(
pcx.span,
"trying to compare incompatible constructors {:?} and {:?}",
self,
other
),
}
}

/// Faster version of `is_covered_by` when applied to many constructors. `used_ctors` is
/// assumed to be built from `matrix.head_ctors()`, and `self` is assumed to have been split.
/// assumed to be built from `matrix.head_ctors()` with wildcards filtered out, and `self` is
/// assumed to have been split from a wildcard.
fn is_covered_by_any<'p>(
&self,
pcx: PatCtxt<'_, 'p, 'tcx>,
Expand All @@ -946,8 +960,9 @@ impl<'tcx> Constructor<'tcx> {
return false;
}

// This must be kept in sync with `is_covered_by`.
match self {
// `used_ctors` cannot contain anything else than `Single`s.
// If `self` is `Single`, `used_ctors` cannot contain anything else than `Single`s.
Single => !used_ctors.is_empty(),
Variant(_) => used_ctors.iter().any(|c| c == self),
IntRange(range) => used_ctors
Expand All @@ -960,8 +975,6 @@ impl<'tcx> Constructor<'tcx> {
.any(|other| slice.is_covered_by(other)),
// This constructor is never covered by anything else
NonExhaustive => false,
// This constructor is only covered by `Single`s
Unlistable => used_ctors.iter().any(|c| *c == Single),
Str(..) | FloatRange(..) | Opaque | Wildcard => {
bug!("found unexpected ctor in all_ctors: {:?}", self)
}
Expand Down Expand Up @@ -1009,6 +1022,10 @@ impl<'tcx> Constructor<'tcx> {
PatKind::Leaf { subpatterns }
}
}
// Note: given the expansion of `&str` patterns done in `expand_pattern`, we should
// be careful to reconstruct the correct constant pattern here. However a string
// literal pattern will never be reported as a non-exhaustiveness witness, so we
// can ignore this issue.
ty::Ref(..) => PatKind::Deref { subpattern: subpatterns.next().unwrap() },
ty::Slice(_) | ty::Array(..) => bug!("bad slice pattern {:?} {:?}", self, pcx.ty),
_ => PatKind::Wild,
Expand Down Expand Up @@ -1041,7 +1058,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(pcx.cx.tcx),
NonExhaustive | Unlistable => PatKind::Wild,
NonExhaustive => PatKind::Wild,
Opaque => bug!("we should not try to apply an opaque constructor"),
Wildcard => bug!(
"trying to apply a wildcard constructor; this should have been done in `apply_constructors`"
Expand Down Expand Up @@ -1190,8 +1207,9 @@ impl<'p, 'tcx> Fields<'p, 'tcx> {
}
_ => bug!("bad slice pattern {:?} {:?}", constructor, ty),
},
Str(..) | FloatRange(..) | IntRange(..) | NonExhaustive | Opaque | Unlistable
| Wildcard => Fields::empty(),
Str(..) | FloatRange(..) | IntRange(..) | NonExhaustive | Opaque | Wildcard => {
Fields::empty()
}
};
debug!("Fields::wildcards({:?}, {:?}) = {:#?}", constructor, ty, ret);
ret
Expand Down Expand Up @@ -1303,9 +1321,13 @@ impl<'p, 'tcx> Fields<'p, 'tcx> {
/// [Some(0), ..] => {}
/// }
/// ```
/// This is guaranteed to preserve the number of patterns in `self`.
fn replace_with_pattern_arguments(&self, pat: &'p Pat<'tcx>) -> Self {
match pat.kind.as_ref() {
PatKind::Deref { subpattern } => Self::from_single_pattern(subpattern),
PatKind::Deref { subpattern } => {
assert_eq!(self.len(), 1);
Fields::from_single_pattern(subpattern)
}
PatKind::Leaf { subpatterns } | PatKind::Variant { subpatterns, .. } => {
self.replace_with_fieldpats(subpatterns)
}
Expand Down Expand Up @@ -1596,10 +1618,9 @@ fn all_constructors<'p, 'tcx>(pcx: PatCtxt<'_, 'p, 'tcx>) -> Vec<Constructor<'tc
vec![make_range(0, max)]
}
_ if cx.is_uninhabited(pcx.ty) => vec![],
ty::Adt(..) | ty::Tuple(..) => vec![Single],
ty::Ref(_, t, _) if !t.is_str() => vec![Single],
// This type is one for which we don't know how to list constructors, like `&str` or `f64`.
_ => vec![Unlistable],
ty::Adt(..) | ty::Tuple(..) | ty::Ref(..) => vec![Single],
// This type is one for which we cannot list constructors, like `str` or `f64`.
_ => vec![NonExhaustive],
}
}

Expand Down Expand Up @@ -2161,28 +2182,31 @@ fn pat_constructor<'p, 'tcx>(
cx: &MatchCheckCtxt<'p, 'tcx>,
pat: &'p Pat<'tcx>,
) -> Constructor<'tcx> {
match *pat.kind {
match pat.kind.as_ref() {
PatKind::AscribeUserType { .. } => bug!(), // Handled by `expand_pattern`
PatKind::Binding { .. } | PatKind::Wild => Wildcard,
PatKind::Leaf { .. } | PatKind::Deref { .. } => Single,
PatKind::Variant { adt_def, variant_index, .. } => {
&PatKind::Variant { adt_def, variant_index, .. } => {
Variant(adt_def.variants[variant_index].def_id)
}
PatKind::Constant { value } => {
if let Some(int_range) = IntRange::from_const(cx.tcx, cx.param_env, value, pat.span) {
IntRange(int_range)
} else {
match value.ty.kind() {
match pat.ty.kind() {
ty::Float(_) => FloatRange(value, value, RangeEnd::Included),
ty::Ref(_, t, _) if t.is_str() => Str(value),
// In `expand_pattern`, we convert string literals to `&CONST` patterns with
// `CONST` a pattern of type `str`. In truth this contains a constant of type
// `&str`.
ty::Str => 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.
_ => Opaque,
}
}
}
PatKind::Range(PatRange { lo, hi, end }) => {
&PatKind::Range(PatRange { lo, hi, end }) => {
let ty = lo.ty;
if let Some(int_range) = IntRange::from_range(
cx.tcx,
Expand All @@ -2197,8 +2221,7 @@ fn pat_constructor<'p, 'tcx>(
FloatRange(lo, hi, end)
}
}
PatKind::Array { ref prefix, ref slice, ref suffix }
| PatKind::Slice { ref prefix, ref slice, ref suffix } => {
PatKind::Array { prefix, slice, suffix } | PatKind::Slice { prefix, slice, suffix } => {
let array_len = match pat.ty.kind() {
ty::Array(_, length) => Some(length.eval_usize(cx.tcx, cx.param_env)),
ty::Slice(_) => None,
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/issues/issue-30240.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
fn main() {
match "world" { //~ ERROR non-exhaustive patterns: `_`
match "world" { //~ ERROR non-exhaustive patterns: `&_`
"hello" => {}
}

match "world" { //~ ERROR non-exhaustive patterns: `_`
match "world" { //~ ERROR non-exhaustive patterns: `&_`
ref _x if false => {}
"hello" => {}
}
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/issues/issue-30240.stderr
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
error[E0004]: non-exhaustive patterns: `_` not covered
error[E0004]: non-exhaustive patterns: `&_` not covered
--> $DIR/issue-30240.rs:2:11
|
LL | match "world" {
| ^^^^^^^ pattern `_` not covered
| ^^^^^^^ pattern `&_` 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 `&str`

error[E0004]: non-exhaustive patterns: `_` not covered
error[E0004]: non-exhaustive patterns: `&_` not covered
--> $DIR/issue-30240.rs:6:11
|
LL | match "world" {
| ^^^^^^^ pattern `_` not covered
| ^^^^^^^ pattern `&_` 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 `&str`
Expand Down
25 changes: 25 additions & 0 deletions src/test/ui/pattern/usefulness/issue-78549-ref-pat-and-str.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// check-pass
// From https://github.com/rust-lang/rust/issues/78549

fn main() {
match "foo" {
"foo" => {},
&_ => {},
}

match "foo" {
&_ => {},
"foo" => {},
}

match ("foo", 0, "bar") {
(&_, 0, &_) => {},
("foo", _, "bar") => {},
(&_, _, &_) => {},
}

match (&"foo", "bar") {
(&"foo", &_) => {},
(&&_, &_) => {},
}
}