Skip to content

Commit

Permalink
Rollup merge of #78072 - Nadrieril:cleanup-constant-matching, r=varkor
Browse files Browse the repository at this point in the history
Cleanup constant matching in exhaustiveness checking

This supercedes #77390. I made the `Opaque` constructor work.
I have opened two issues #78071 and #78057 from the discussion we had on the previous PR. They are not regressions nor directly related to the current PR so I thought we'd deal with them separately.

I left a FIXME somewhere because I didn't know how to compare string constants for equality. There might even be some unicode things that need to happen there. In the meantime I preserved previous behavior.

EDIT: I accidentally fixed #78071
  • Loading branch information
jonas-schievink authored Oct 24, 2020
2 parents 0a06d73 + faf8710 commit e12e972
Show file tree
Hide file tree
Showing 12 changed files with 448 additions and 347 deletions.
450 changes: 111 additions & 339 deletions compiler/rustc_mir_build/src/thir/pattern/_match.rs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,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);
Expand Down
12 changes: 7 additions & 5 deletions compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,14 +387,16 @@ 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.
// 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));
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_mir_build/src/thir/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
},
Expand Down
File renamed without changes.
File renamed without changes.
11 changes: 11 additions & 0 deletions src/test/ui/consts/const_in_pattern/issue-53708.rs
Original file line number Diff line number Diff line change
@@ -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 => {}
}
}
17 changes: 17 additions & 0 deletions src/test/ui/consts/const_in_pattern/issue-78057.rs
Original file line number Diff line number Diff line change
@@ -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
}
}
20 changes: 20 additions & 0 deletions src/test/ui/consts/const_in_pattern/issue-78057.stderr
Original file line number Diff line number Diff line change
@@ -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

114 changes: 114 additions & 0 deletions src/test/ui/pattern/usefulness/consts-opaque.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
// 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
}

// 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 => {}
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 => {}
_ => {}
}
}
158 changes: 158 additions & 0 deletions src/test/ui/pattern/usefulness/consts-opaque.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
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
| ^^^^^^

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 <https://github.com/rust-lang/rust/issues/62411>

error: to use a constant of type `Bar` in a pattern, `Bar` must be annotated with `#[derive(PartialEq, Eq)]`
--> $DIR/consts-opaque.rs:57:9
|
LL | BAR => {} // should not be emitting unreachable warning
| ^^^

error: unreachable pattern
--> $DIR/consts-opaque.rs:57:9
|
LL | Bar => {}
| --- matches any value
LL | BAR => {} // should not be emitting unreachable warning
| ^^^ unreachable pattern

error: unreachable pattern
--> $DIR/consts-opaque.rs:60: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:65:9
|
LL | BAR => {}
| ^^^

error: unreachable pattern
--> $DIR/consts-opaque.rs:67:9
|
LL | Bar => {} // should not be emitting unreachable warning
| ^^^

error: unreachable pattern
--> $DIR/consts-opaque.rs:69: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: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:76:9
|
LL | BAR => {} // should not be emitting unreachable warning
| ^^^

error: unreachable pattern
--> $DIR/consts-opaque.rs:76:9
|
LL | BAR => {} // should not be emitting unreachable warning
| ^^^

error: unreachable pattern
--> $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:84:9
|
LL | BAZ => {}
| ^^^

error: unreachable pattern
--> $DIR/consts-opaque.rs:86:9
|
LL | Baz::Baz1 => {} // should not be emitting unreachable warning
| ^^^^^^^^^

error: unreachable pattern
--> $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:94:9
|
LL | BAZ => {}
| ^^^

error: unreachable pattern
--> $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:101:9
|
LL | BAZ => {}
| ^^^

error: unreachable pattern
--> $DIR/consts-opaque.rs:103:9
|
LL | Baz::Baz2 => {} // should not be emitting unreachable warning
| ^^^^^^^^^

error: unreachable pattern
--> $DIR/consts-opaque.rs:105:9
|
LL | _ => {} // should not be emitting unreachable warning
| ^

error: aborting due to 22 previous errors; 1 warning emitted

Original file line number Diff line number Diff line change
Expand Up @@ -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]`
Expand Down

0 comments on commit e12e972

Please sign in to comment.