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

Incorrect non-exhaustive pattern error #72476

Closed
tmiasko opened this issue May 22, 2020 · 10 comments · Fixed by #72506
Closed

Incorrect non-exhaustive pattern error #72476

tmiasko opened this issue May 22, 2020 · 10 comments · Fixed by #72506
Labels
A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns C-bug Category: This is a bug. P-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tmiasko
Copy link
Contributor

tmiasko commented May 22, 2020

enum Next<I> where I: Iterator {
    Item(I::Item),
}

fn e(item: Next<&mut dyn Iterator<Item=Option<()>>>) {
    match item {
        Next::Item(None) => {}
        Next::Item(Some(())) => {}
    }
}

fn main() {}

Produces non-exhaustive pattern error (even though all patterns are covered):

error[E0004]: non-exhaustive patterns: `Item(_)` not covered
 --> main.rs:6:11
  |
1 | / enum Next<I> where I: Iterator {
2 | |     Item(I::Item),
  | |     ---- not covered
3 | | }
  | |_- `Next<&mut dyn std::iter::Iterator<Item = std::option::Option<()>>>` defined here
...
6 |       match item {
  |             ^^^^ pattern `Item(_)` 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 `Next<&mut dyn std::iter::Iterator<Item = std::option::Option<()>>>`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0004`.

Meta

rustc --version --verbose:

rustc 1.45.0-nightly (9310e3bd4 2020-05-21)
binary: rustc
commit-hash: 9310e3bd4f425f84fc27878ebf2bda1f30935a63
commit-date: 2020-05-21
host: x86_64-unknown-linux-gnu
release: 1.45.0-nightly
LLVM version: 10.0
@tmiasko tmiasko added the C-bug Category: This is a bug. label May 22, 2020
@cuviper
Copy link
Member

cuviper commented May 22, 2020

It passes on the playground with 1.43.1 and 1.44.0-beta.3

@cuviper cuviper added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label May 22, 2020
@jonas-schievink jonas-schievink added I-prioritize Issue: Indicates that prioritization has been requested for this issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns labels May 22, 2020
@lcnr
Copy link
Contributor

lcnr commented May 23, 2020

Seems to be a problem when pattern matching on projections.

trait A {
    type Projection;
}

impl A for () {
    type Projection = bool;
    // using () instead of bool here does compile though
}

struct Next<T: A>(T::Projection);

fn e(item: Next<()>) {
    match item {
        Next(true) => {}
        Next(false) => {}
    }
}

fn main() {}

@tmiasko
Copy link
Contributor Author

tmiasko commented May 23, 2020

Related to changes from #71930, cc @Nadrieril.

@Nadrieril
Copy link
Member

Ok, the problem comes from the fact that we get a pattern with type <() as A>::Projection instead of bool, and the exhaustiveness code isn't prepared to handle one of those. This type comes from listing the fields of Next<()> and calling field.ty() here. I know nothing of how type resolution is supposed to work, is there a step that was missed somewhere that should have turned this into bool? @matthewjasper you seemed to have a grasp on that.

It's a regression because of e5a2cd5. This commit keeps revealing weird quirks with types...

@matthewjasper
Copy link
Contributor

You can try adding a normalize_erasing_regions call after field.ty().

@Nadrieril
Copy link
Member

Nadrieril commented May 23, 2020

@matthewjasper This works! It feels patchy though: should I always call this before inspecting a type? Is it normal to find a non-normalized type at this stage of compilation? Do you know if there is a principled way I can be sure not to run into a similar bug later?

@LeSeulArtichaut
Copy link
Contributor

@LeSeulArtichaut LeSeulArtichaut added P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 23, 2020
@Nadrieril
Copy link
Member

I have an easy workaround, should I push that first and we can investigate further later?

@LeSeulArtichaut
Copy link
Contributor

LeSeulArtichaut commented May 23, 2020

@Nadrieril I believe you should, the technical details can be discussed as part of code review I think, and if performance tests need to be done, then you'll need a PR anyway.

@Nadrieril
Copy link
Member

Here: #72506

@bors bors closed this as completed in 9cd3f1c May 29, 2020
Manishearth added a commit to Manishearth/rust that referenced this issue Oct 1, 2021
Normalize after substituting via `field.ty()`

Back in rust-lang#72476 I hadn't understood where the problem was coming from, and only worked around the issue. What happens is that calling `field.ty()` on a field of a generic struct substitutes the appropriate generics but doesn't normalize the resulting type.
As a consumer of types I'm surprised that one would substitute without normalizing, feels like a footgun, so I added a comment.

Fixes rust-lang#89393.
mikegerwitz pushed a commit to lovullo/tame that referenced this issue Nov 23, 2021
This provides a child `raw` module that exposes a SymbolId representing the
inner value of each of the static newtypes.  This is needed in situations
where the type must match and the type of the static symbol is not
important.

In particular, when comparing against runtime-allocated symbols in `match`
expressions.

It is also worth noting that this commit managed to hit a bug in Rustc that
was fixed on 10/1/2021.  We use nightly, and it doesn't seem that this
occurred in stable, from bug reports.

  - rust-lang/rust#89393
  - rust-lang/rust@5ab1245
  - Original issue: rust-lang/rust#72476

The error was:

  compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs:1191:22:
  Unexpected type for `Single` constructor: <u32 as sym::symbol::SymbolIndexSize>::NonZero

  thread 'rustc' panicked at 'Box<dyn Any>', compiler/rustc_errors/src/lib.rs:1146:9

This occurred because we were trying to use `SymbolId` as the type, which
uses a projected type as its inner value: `SymbolId<Ix: SymbolIndexSize>(Ix::NonZero)`.
This was not a problem with the static newtypes because their inner type was
simply `SymbolId<Ix>`, which is not projected.

This is one of the risks of using nightly.

But, the point is: if you receive this error, upgrade your toolchain.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns C-bug Category: This is a bug. P-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants