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.

Sign up for GitHub

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

Jump to bottom

Be clearer on type mismatch within a range #57389

Closed
estebank opened this issue Jan 6, 2019 · 6 comments · Fixed by #67287
Closed

Be clearer on type mismatch within a range #57389

estebank opened this issue Jan 6, 2019 · 6 comments · Fixed by #67287
Labels
A-diagnostics Area: Messages for errors, warnings, and lints 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. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

estebank commented Jan 6, 2019

When matching on a range between two values of different type we currently emit the following output:

error[E0308]: mismatched types
  --> $DIR/E0308-4.rs:4:9
   |
LL |         0u8..=3i8 => (), //~ ERROR E0308
   |         ^^^^^^^^^ expected u8, found i8

We should have a more targeted error:

error[E0308]: mismatched types
  --> $DIR/E0308-4.rs:4:9
   |
LL |         0u8..=3i8 => (), //~ ERROR E0308
   |         ---   ^^^ expected u8, found i8
   |         |
   |         expected because of this
@estebank estebank added the A-diagnostics Area: Messages for errors, warnings, and lints label Jan 6, 2019
@PramodBisht
Copy link
Contributor

@estebank Can I take this one if nobody is working on it?

@estebank
Copy link
Contributor Author

@PramodBisht go ahead!

@estebank estebank added E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 13, 2019
@estebank estebank added 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. labels Oct 18, 2019
@kevgrasso
Copy link
Contributor

@PramodBisht Are you still working on this? Or could I take this over?

@PramodBisht
Copy link
Contributor

@kevgrasso I got occupied with other things, Please take it. Thanks

@kevgrasso
Copy link
Contributor

@estebank I'm getting some weird behavior from my attempted fix. Most of the bugs I'm getting are understandable, but the one I don't understand is that the text annotating for the primary span in the error diagnostic doesn't show up if I let it display to terminal, but it does if I pipe stderr to a file, and I have no idea what I changed that caused this. Any chance you'd have any clue what might have happened? My changes are here if it helps: https://github.com/kevgrasso/rust/commit/3d37d388e40896f10d6105d80dedd2711d183067

image

@estebank
Copy link
Contributor Author

@kevgrasso

  1. Love the new output!
  2. I'm not sure what could be causing this, it seems like it should work based on the code I'm seeing.

@kevgrasso kevgrasso mentioned this issue Dec 11, 2019
Closed
@Centril Centril mentioned this issue Dec 13, 2019
Merged
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Dec 26, 2019
@Mark-Simulacrum
Rollup merge of rust-lang#67287 - Centril:mismatch-range-improve-diag…
39b2ce8 
…, r=estebank

typeck: note other end-point when checking range pats

Fixes rust-lang#57389, alternative to rust-lang#67214 that should be less invasive to type checking logic.

r? @estebank
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Dec 26, 2019
@Mark-Simulacrum
Rollup merge of rust-lang#67287 - Centril:mismatch-range-improve-diag…
2896a4c 
…, r=estebank

typeck: note other end-point when checking range pats

Fixes rust-lang#57389, alternative to rust-lang#67214 that should be less invasive to type checking logic.

r? @estebank
@bors bors closed this as completed in 2c46dd1 Dec 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-diagnostics Area: Messages for errors, warnings, and lints 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. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
3 participants
@kevgrasso @estebank @PramodBisht