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

Poor diagnostics on missing semicolon after match statement #72634

Closed
moxian opened this issue May 26, 2020 · 3 comments · Fixed by #106601
Closed

Poor diagnostics on missing semicolon after match statement #72634

moxian opened this issue May 26, 2020 · 3 comments · Fixed by #106601
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-inference Area: Type inference C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@moxian
Copy link
Contributor

moxian commented May 26, 2020

I tried this code (playground):

fn stuff() -> i32 {
    match 1i32 {
        2i32 => 3i32,
        _ => panic!("wrong"),
    }
    4i32
}

I expected to see this happen:
A compiler error about missing ; between } and 4i32

Instead, this happened:

error[E0308]: `match` arms have incompatible types
 --> src/lib.rs:4:14
  |
2 | /     match 1i32 {
3 | |         2i32 => 3i32,
  | |                 ---- this is found to be of type `i32`
4 | |         _ => panic!("wrong"),
  | |              ^^^^^^^^^^^^^^^ expected `i32`, found `()`
5 | |     }
  | |_____- `match` arms have incompatible types
  |
  = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

This is very confusing, because I "know" that panic!() return type is ! which is coercible to anything else (or whatever is the technical explanation for "we can really ignore the return type of panic!"), so it should not ever be present in error messages about mistyping stuff.

Meta

rustc --version --verbose:

rustc 1.44.0-beta.3 (0f0d70055 2020-05-11)
binary: rustc
commit-hash: 0f0d70055530bdbf3f0acb7b8ad25aa4a6ad8ea9
commit-date: 2020-05-11
host: x86_64-pc-windows-msvc
release: 1.44.0-beta.3
LLVM version: 9.0

This issue is also present in 1.34.2 (from a year ago), and in two-week-old nightly (rustc 1.45.0-nightly (99cb9cc 2020-05-11))

p.s.: When deminimizing the issue, i realized that it's even more confusing when match is the last statement in a for loop (as it was in my real code), i.e.:

fn stuff2(){
    for _ in 0..5{
        match 1i32 {
            2i32 => 3i32,
            _ => panic!("wrong"),
        }
    }
}

gives the same error message about incompatible match arms.

@moxian moxian added the C-bug Category: This is a bug. label May 26, 2020
@jonas-schievink jonas-schievink added A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed C-bug Category: This is a bug. labels May 26, 2020
@estebank
Copy link
Contributor

The panic!s ! coerces to () because the whole match needs to evaluate as a statement, not an expression because its return value is ignored. Adding a trailing ; "fixes" the code.

@estebank estebank added A-inference Area: Type inference D-confusing Diagnostics: Confusing error or lint that should be reworked. D-papercut Diagnostics: An error or lint that needs small tweaks. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. labels May 28, 2020
estebank added a commit to estebank/rust that referenced this issue Jan 8, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 21, 2023
Suggest `;` after bare `match` expression E0308

Fix rust-lang#72634.
@bors bors closed this as completed in a53f280 Oct 21, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 21, 2023
Rollup merge of rust-lang#106601 - estebank:match-semi, r=cjgillot

Suggest `;` after bare `match` expression E0308

Fix rust-lang#72634.
@compiler-errors
Copy link
Member

@estebank: As far as I can tell, #106601 didn't fix this issue (you didn't add a test for it either), and the two UI tests that the PR did touch, the suggestion didn't do anything useful -- it only fires when one of the arms is actually evaluated to be type (), rather than being constrained to () due to (lack of) semicolon.

@compiler-errors
Copy link
Member

Actually, we do suggest using a semicolon, but not via #106601. I'm just gonna revert that, and add this as a test.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 15, 2024
…rms, r=estebank

Only point out non-diverging arms for match suggestions

Fixes rust-lang#121144

There is no reason to point at diverging arms, which will always coerce to whatever is the match block's evaluated type.

This also removes the suggestion from rust-lang#106601, since as I pointed out in rust-lang#72634 (comment) the added suggestion is not firing in the right cases, but instead only when one of the match arms already *actually* evaluates to `()`.

r? estebank
oli-obk added a commit to oli-obk/rust that referenced this issue Feb 15, 2024
…rms, r=estebank

Only point out non-diverging arms for match suggestions

Fixes rust-lang#121144

There is no reason to point at diverging arms, which will always coerce to whatever is the match block's evaluated type.

This also removes the suggestion from rust-lang#106601, since as I pointed out in rust-lang#72634 (comment) the added suggestion is not firing in the right cases, but instead only when one of the match arms already *actually* evaluates to `()`.

r? estebank
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 15, 2024
…rms, r=estebank

Only point out non-diverging arms for match suggestions

Fixes rust-lang#121144

There is no reason to point at diverging arms, which will always coerce to whatever is the match block's evaluated type.

This also removes the suggestion from rust-lang#106601, since as I pointed out in rust-lang#72634 (comment) the added suggestion is not firing in the right cases, but instead only when one of the match arms already *actually* evaluates to `()`.

r? estebank
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 16, 2024
Rollup merge of rust-lang#121146 - compiler-errors:ignore-diverging-arms, r=estebank

Only point out non-diverging arms for match suggestions

Fixes rust-lang#121144

There is no reason to point at diverging arms, which will always coerce to whatever is the match block's evaluated type.

This also removes the suggestion from rust-lang#106601, since as I pointed out in rust-lang#72634 (comment) the added suggestion is not firing in the right cases, but instead only when one of the match arms already *actually* evaluates to `()`.

r? estebank
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-inference Area: Type inference C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-papercut Diagnostics: An error or lint that needs small tweaks. 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.

4 participants